From 80beb6d6f602d80c6a17cb8c660540d959fa9528 Mon Sep 17 00:00:00 2001 From: Evgeny Khramtsov Date: Sun, 7 Jul 2019 22:12:14 +0300 Subject: [PATCH] Improve formatting of exceptions --- src/ejabberd_local.erl | 35 +++++-------- src/ejabberd_mnesia.erl | 10 ++-- src/ejabberd_router.erl | 15 ++---- src/ejabberd_router_sql.erl | 11 +++-- src/ejabberd_s2s.erl | 22 ++++----- src/ejabberd_sm.erl | 31 ++++++------ src/ejabberd_sql.erl | 12 ++--- src/gen_iq_handler.erl | 7 +-- src/gen_mod.erl | 9 ++-- src/mod_muc_log.erl | 2 +- src/mod_muc_room.erl | 11 +++-- src/mod_roster.erl | 99 +++++++++++++++++-------------------- 12 files changed, 122 insertions(+), 142 deletions(-) diff --git a/src/ejabberd_local.erl b/src/ejabberd_local.erl index 3f8aa749b..91a9264a5 100644 --- a/src/ejabberd_local.erl +++ b/src/ejabberd_local.erl @@ -69,13 +69,20 @@ start_link() -> gen_server:start_link({local, ?MODULE}, ?MODULE, [], []). --spec route(stanza()) -> any(). +-spec route(stanza()) -> ok. route(Packet) -> - try do_route(Packet) - catch ?EX_RULE(E, R, St) -> - StackTrace = ?EX_STACK(St), - ?ERROR_MSG("Failed to route packet:~n~s~nReason = ~p", - [xmpp:pp(Packet), {E, {R, StackTrace}}]) + ?DEBUG("Local route:~n~s", [xmpp:pp(Packet)]), + Type = xmpp:get_type(Packet), + To = xmpp:get_to(Packet), + if To#jid.luser /= <<"">> -> + ejabberd_sm:route(Packet); + is_record(Packet, iq), To#jid.lresource == <<"">> -> + gen_iq_handler:handle(?MODULE, Packet); + Type == result; Type == error -> + ok; + true -> + ejabberd_hooks:run(local_send_to_resource_hook, + To#jid.lserver, [Packet]) end. -spec route_iq(iq(), function()) -> ok. @@ -139,22 +146,6 @@ code_change(_OldVsn, State, _Extra) -> %%-------------------------------------------------------------------- %%% Internal functions %%-------------------------------------------------------------------- --spec do_route(stanza()) -> any(). -do_route(Packet) -> - ?DEBUG("Local route:~n~s", [xmpp:pp(Packet)]), - Type = xmpp:get_type(Packet), - To = xmpp:get_to(Packet), - if To#jid.luser /= <<"">> -> - ejabberd_sm:route(Packet); - is_record(Packet, iq), To#jid.lresource == <<"">> -> - gen_iq_handler:handle(?MODULE, Packet); - Type == result; Type == error -> - ok; - true -> - ejabberd_hooks:run(local_send_to_resource_hook, - To#jid.lserver, [Packet]) - end. - -spec update_table() -> ok. update_table() -> catch mnesia:delete_table(iq_response), diff --git a/src/ejabberd_mnesia.erl b/src/ejabberd_mnesia.erl index 6f27231cb..3df981124 100644 --- a/src/ejabberd_mnesia.erl +++ b/src/ejabberd_mnesia.erl @@ -365,14 +365,14 @@ do_transform(OldAttrs, Attrs, Old) -> transform_fun(Module, Name) -> fun(Obj) -> try Module:transform(Obj) - catch ?EX_RULE(E, R, St) -> + catch ?EX_RULE(Class, Reason, St) -> StackTrace = ?EX_STACK(St), ?ERROR_MSG("Failed to transform Mnesia table ~s:~n" "** Record: ~p~n" - "** Reason: ~p~n" - "** StackTrace: ~p", - [Name, Obj, R, StackTrace]), - erlang:raise(E, R, StackTrace) + "** ~s", + [Name, Obj, + misc:format_exception(2, Class, Reason, StackTrace)]), + erlang:raise(Class, Reason, StackTrace) end end. diff --git a/src/ejabberd_router.erl b/src/ejabberd_router.erl index 9c5c4e505..fe2db0ce0 100644 --- a/src/ejabberd_router.erl +++ b/src/ejabberd_router.erl @@ -90,10 +90,11 @@ start_link() -> -spec route(stanza()) -> ok. route(Packet) -> try do_route(Packet) - catch ?EX_RULE(E, R, St) -> + catch ?EX_RULE(Class, Reason, St) -> StackTrace = ?EX_STACK(St), - ?ERROR_MSG("Failed to route packet:~n~s~nReason = ~p", - [xmpp:pp(Packet), {E, {R, StackTrace}}]) + ?ERROR_MSG("Failed to route packet:~n~s~n** ~s", + [xmpp:pp(Packet), + misc:format_exception(2, Class, Reason, StackTrace)]) end. -spec route(jid(), jid(), xmlel() | stanza()) -> ok. @@ -107,13 +108,7 @@ route(#jid{} = From, #jid{} = To, #xmlel{} = El) -> xmpp:format_error(Why)]) end; route(#jid{} = From, #jid{} = To, Packet) -> - case catch do_route(xmpp:set_from_to(Packet, From, To)) of - {'EXIT', Reason} -> - ?ERROR_MSG("~p~nwhen processing: ~p", - [Reason, {From, To, Packet}]); - _ -> - ok - end. + route(xmpp:set_from_to(Packet, From, To)). -spec route_error(stanza(), stanza_error()) -> ok. route_error(Packet, Err) -> diff --git a/src/ejabberd_router_sql.erl b/src/ejabberd_router_sql.erl index a64288fd9..92fbefdf5 100644 --- a/src/ejabberd_router_sql.erl +++ b/src/ejabberd_router_sql.erl @@ -121,12 +121,13 @@ row_to_route(Domain, {ServerHost, NodeS, PidS, LocalHintS} = Row) -> local_hint = dec_local_hint(LocalHintS)}] catch _:{bad_node, _} -> []; - ?EX_RULE(E, R, St) -> + ?EX_RULE(Class, Reason, St) -> StackTrace = ?EX_STACK(St), ?ERROR_MSG("Failed to decode row from 'route' table:~n" - "Row = ~p~n" - "Domain = ~s~n" - "Reason = ~p", - [Row, Domain, {E, {R, StackTrace}}]), + "** Row = ~p~n" + "** Domain = ~s~n" + "** ~s", + [Row, Domain, + misc:format_exception(2, Class, Reason, StackTrace)]), [] end. diff --git a/src/ejabberd_s2s.erl b/src/ejabberd_s2s.erl index 5e793ec36..2128d6b6a 100644 --- a/src/ejabberd_s2s.erl +++ b/src/ejabberd_s2s.erl @@ -86,16 +86,6 @@ start_link() -> gen_server:start_link({local, ?MODULE}, ?MODULE, [], []). --spec route(stanza()) -> ok. - -route(Packet) -> - try do_route(Packet) - catch ?EX_RULE(E, R, St) -> - StackTrace = ?EX_STACK(St), - ?ERROR_MSG("Failed to route packet:~n~s~nReason = ~p", - [xmpp:pp(Packet), {E, {R, StackTrace}}]) - end. - clean_temporarily_blocked_table() -> mnesia:clear_table(temporarily_blocked). @@ -296,7 +286,13 @@ handle_info({mnesia_system_event, {mnesia_down, Node}}, State) -> clean_table_from_bad_node(Node), {noreply, State}; handle_info({route, Packet}, State) -> - route(Packet), + try route(Packet) + catch ?EX_RULE(Class, Reason, St) -> + StackTrace = ?EX_STACK(St), + ?ERROR_MSG("Failed to route packet:~n~s~n** ~s", + [xmpp:pp(Packet), + misc:format_exception(2, Class, Reason, StackTrace)]) + end, {noreply, State}; handle_info(_Info, State) -> {noreply, State}. @@ -347,8 +343,8 @@ clean_table_from_bad_node(Node) -> end, mnesia:async_dirty(F). --spec do_route(stanza()) -> ok. -do_route(Packet) -> +-spec route(stanza()) -> ok. +route(Packet) -> ?DEBUG("Local route:~n~s", [xmpp:pp(Packet)]), From = xmpp:get_from(Packet), To = xmpp:get_to(Packet), diff --git a/src/ejabberd_sm.erl b/src/ejabberd_sm.erl index 03218dc3e..13ffa1af0 100644 --- a/src/ejabberd_sm.erl +++ b/src/ejabberd_sm.erl @@ -125,12 +125,14 @@ stop() -> -spec route(jid(), term()) -> ok. %% @doc route arbitrary term to c2s process(es) route(To, Term) -> - case catch do_route(To, Term) of - {'EXIT', Reason} -> - ?ERROR_MSG("Route ~p to ~p failed: ~p", - [Term, To, Reason]); - _ -> - ok + try do_route(To, Term), ok + catch ?EX_RULE(E, R, St) -> + StackTrace = ?EX_STACK(St), + ?ERROR_MSG("Failed to route term to ~s:~n" + "** Term = ~p~n" + "** ~s", + [jid:encode(To), Term, + misc:format_exception(2, E, R, StackTrace)]) end. -spec route(stanza()) -> ok. @@ -140,13 +142,8 @@ route(Packet) -> drop -> ?DEBUG("Hook dropped stanza:~n~s", [xmpp:pp(Packet)]); Packet1 -> - try do_route(Packet1), ok - catch ?EX_RULE(E, R, St) -> - StackTrace = ?EX_STACK(St), - ?ERROR_MSG("Failed to route packet:~n~s~nReason = ~p", - [xmpp:pp(Packet1), - {E, {R, StackTrace}}]) - end + do_route(Packet1), + ok end. -spec open_session(sid(), binary(), binary(), binary(), prio(), info()) -> ok. @@ -500,7 +497,13 @@ handle_call(_Request, _From, State) -> handle_cast(_Msg, State) -> {noreply, State}. handle_info({route, Packet}, State) -> - route(Packet), + try route(Packet) + catch ?EX_RULE(E, R, St) -> + StackTrace = ?EX_STACK(St), + ?ERROR_MSG("Failed to route packet:~n~s~n** ~s", + [xmpp:pp(Packet), + misc:format_exception(2, E, R, StackTrace)]) + end, {noreply, State}; handle_info(Info, State) -> ?WARNING_MSG("Unexpected info: ~p", [Info]), diff --git a/src/ejabberd_sql.erl b/src/ejabberd_sql.erl index 029c22602..5a94dcf8e 100644 --- a/src/ejabberd_sql.erl +++ b/src/ejabberd_sql.erl @@ -618,8 +618,8 @@ sql_query_internal(#sql_query{} = Query) -> {error, <<"terminated unexpectedly">>}; ?EX_RULE(Class, Reason, Stack) -> StackTrace = ?EX_STACK(Stack), - ?ERROR_MSG("Internal error while processing SQL query: ~p", - [{Class, Reason, StackTrace}]), + ?ERROR_MSG("Internal error while processing SQL query:~n** ~s", + [misc:format_exception(2, Class, Reason, StackTrace)]), {error, <<"internal error">>} end, check_error(Res, Query); @@ -760,10 +760,10 @@ sql_query_format_res({selected, _, Rows}, SQLQuery) -> catch ?EX_RULE(Class, Reason, Stack) -> StackTrace = ?EX_STACK(Stack), - ?ERROR_MSG("Error while processing " - "SQL query result: ~p~n" - "row: ~p", - [{Class, Reason, StackTrace}, Row]), + ?ERROR_MSG("Error while processing SQL query result:~n" + "** Row: ~p~n** ~s", + [Row, + misc:format_exception(2, Class, Reason, StackTrace)]), [] end end, Rows), diff --git a/src/gen_iq_handler.erl b/src/gen_iq_handler.erl index c00ecefe3..a5fe4cd43 100644 --- a/src/gen_iq_handler.erl +++ b/src/gen_iq_handler.erl @@ -111,10 +111,11 @@ process_iq(_Host, Module, Function, IQ) -> ejabberd_router:route(ResIQ); ignore -> ok - catch ?EX_RULE(E, R, St) -> + catch ?EX_RULE(Class, Reason, St) -> StackTrace = ?EX_STACK(St), - ?ERROR_MSG("Failed to process iq:~n~s~nReason = ~p", - [xmpp:pp(IQ), {E, {R, StackTrace}}]), + ?ERROR_MSG("Failed to process iq:~n~s~n** ~s", + [xmpp:pp(IQ), + misc:format_exception(2, Class, Reason, StackTrace)]), Txt = ?T("Module failed to handle the query"), Err = xmpp:err_internal_server_error(Txt, IQ#iq.lang), ejabberd_router:route_error(IQ, Err) diff --git a/src/gen_mod.erl b/src/gen_mod.erl index 5e04c5dbb..f4d2323c4 100644 --- a/src/gen_mod.erl +++ b/src/gen_mod.erl @@ -451,16 +451,15 @@ format_module_error(Module, Fun, Arity, Opts, Class, Reason, St) -> io_lib:format("Module ~s returned unexpected value from ~s/~B:~n" "** Error: ~p~n" "** Hint: this is either not an ejabberd module " - "or it implements ejabbed API incorrectly", + "or it implements ejabberd API incorrectly", [Module, Fun, Arity, Ret]); _ -> io_lib:format("Internal error of module ~s has " "occured during ~s:~n" "** Options: ~p~n" - "** Class: ~p~n" - "** Reason: ~p~n" - "** Stacktrace: ~p", - [Module, Fun, Opts, Class, Reason, St]) + "** ~s", + [Module, Fun, Opts, + misc:format_exception(2, Class, Reason, St)]) end. -spec format_hosts_list([binary()]) -> iolist(). diff --git a/src/mod_muc_log.erl b/src/mod_muc_log.erl index 2ad8475fc..79d31c360 100644 --- a/src/mod_muc_log.erl +++ b/src/mod_muc_log.erl @@ -503,7 +503,7 @@ create_image_files(Images_dir) -> case file:copy(Src, Dst) of {ok, _} -> ok; {error, Why} -> - ?ERROR_MSG("Failed to copy ~s to ~s", + ?ERROR_MSG("Failed to copy ~s to ~s: ~s", [Src, Dst, file:format_error(Why)]) end end, Filenames). diff --git a/src/mod_muc_room.erl b/src/mod_muc_room.erl index b0d9da276..f83210606 100644 --- a/src/mod_muc_room.erl +++ b/src/mod_muc_room.erl @@ -759,9 +759,9 @@ terminate(Reason, _StateName, catch ?EX_RULE(E, R, St) -> StackTrace = ?EX_STACK(St), mod_muc:room_destroyed(Host, Room, self(), LServer), - ?ERROR_MSG("Got exception on room termination: ~p", [{E, {R, StackTrace}}]) - end, - ok. + ?ERROR_MSG("Got exception on room termination:~n** ~s", + [misc:format_exception(2, E, R, StackTrace)]) + end. %%%---------------------------------------------------------------------- %%% Internal functions @@ -2803,8 +2803,9 @@ process_item_change(Item, SD, UJID) -> undefined -> <<"">> end, - ?ERROR_MSG("Failed to set item ~p~s: ~p", - [Item, FromSuffix, {E, {R, StackTrace}}]), + ?ERROR_MSG("Failed to set item ~p~s:~n** ~s", + [Item, FromSuffix, + misc:format_exception(2, E, R, StackTrace)]), {error, xmpp:err_internal_server_error()} end. diff --git a/src/mod_roster.erl b/src/mod_roster.erl index e3adce046..5c880019f 100644 --- a/src/mod_roster.erl +++ b/src/mod_roster.erl @@ -270,63 +270,56 @@ write_roster_version(LUser, LServer, InTransaction) -> %% - roster versioning is not used by the client OR %% - roster versioning is used by server and client, BUT the server isn't storing versions on db OR %% - the roster version from client don't match current version. -process_iq_get(#iq{to = To, lang = Lang, +process_iq_get(#iq{to = To, sub_els = [#roster_query{ver = RequestedVersion}]} = IQ) -> LUser = To#jid.luser, LServer = To#jid.lserver, US = {LUser, LServer}, - try {ItemsToSend, VersionToSend} = - case {roster_versioning_enabled(LServer), - roster_version_on_db(LServer)} of - {true, true} when RequestedVersion /= undefined -> - case read_roster_version(LUser, LServer) of - error -> - RosterVersion = write_roster_version(LUser, LServer), - {lists:map(fun encode_item/1, - ejabberd_hooks:run_fold( - roster_get, To#jid.lserver, [], [US])), - RosterVersion}; - {ok, RequestedVersion} -> - {false, false}; - {ok, NewVersion} -> - {lists:map(fun encode_item/1, - ejabberd_hooks:run_fold( - roster_get, To#jid.lserver, [], [US])), - NewVersion} - end; - {true, false} when RequestedVersion /= undefined -> - RosterItems = ejabberd_hooks:run_fold( - roster_get, To#jid.lserver, [], [US]), - case roster_hash(RosterItems) of - RequestedVersion -> - {false, false}; - New -> - {lists:map(fun encode_item/1, RosterItems), New} - end; - _ -> - {lists:map(fun encode_item/1, - ejabberd_hooks:run_fold( - roster_get, To#jid.lserver, [], [US])), - false} - end, - xmpp:make_iq_result( - IQ, - case {ItemsToSend, VersionToSend} of - {false, false} -> - undefined; - {Items, false} -> - #roster_query{items = Items}; - {Items, Version} -> - #roster_query{items = Items, - ver = Version} - end) - catch ?EX_RULE(E, R, St) -> - StackTrace = ?EX_STACK(St), - ?ERROR_MSG("Failed to process roster get for ~s: ~p", - [jid:encode(To), {E, {R, StackTrace}}]), - Txt = ?T("Roster module has failed"), - xmpp:make_error(IQ, xmpp:err_internal_server_error(Txt, Lang)) - end. + {ItemsToSend, VersionToSend} = + case {roster_versioning_enabled(LServer), + roster_version_on_db(LServer)} of + {true, true} when RequestedVersion /= undefined -> + case read_roster_version(LUser, LServer) of + error -> + RosterVersion = write_roster_version(LUser, LServer), + {lists:map(fun encode_item/1, + ejabberd_hooks:run_fold( + roster_get, To#jid.lserver, [], [US])), + RosterVersion}; + {ok, RequestedVersion} -> + {false, false}; + {ok, NewVersion} -> + {lists:map(fun encode_item/1, + ejabberd_hooks:run_fold( + roster_get, To#jid.lserver, [], [US])), + NewVersion} + end; + {true, false} when RequestedVersion /= undefined -> + RosterItems = ejabberd_hooks:run_fold( + roster_get, To#jid.lserver, [], [US]), + case roster_hash(RosterItems) of + RequestedVersion -> + {false, false}; + New -> + {lists:map(fun encode_item/1, RosterItems), New} + end; + _ -> + {lists:map(fun encode_item/1, + ejabberd_hooks:run_fold( + roster_get, To#jid.lserver, [], [US])), + false} + end, + xmpp:make_iq_result( + IQ, + case {ItemsToSend, VersionToSend} of + {false, false} -> + undefined; + {Items, false} -> + #roster_query{items = Items}; + {Items, Version} -> + #roster_query{items = Items, + ver = Version} + end). -spec get_user_roster([#roster{}], {binary(), binary()}) -> [#roster{}]. get_user_roster(Acc, {LUser, LServer}) ->