Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Don't assume the supervisor is a registered process. #115

Merged
merged 3 commits into from

2 participants

@mworrell

Lager crashes in error_logger_lager_h.erl when reporting about a non-named supervisor.

This patch removes the assumption that the supervisor name is always something like {local, name}, as it can also be a pid.

@Vagabond
Collaborator

Isn't line 141 in error_logger_lager_h also bad, then?

@Vagabond
Collaborator

Eunit tests for supervisor messages that look like this would be good, as well. Look at lines 722 and 754 in lager_test_backend.

@mworrell

Indeed, line 141 needed some changes as well.
Added two tests.

@Vagabond
Collaborator

Reading supervisor.erl, it looks like the supervisor name can also be {global, Name}. The specs are a mess, but that appears to be the only other allowed variation. Might as well handle {global, ...} while we're here, no?

@mworrell

I though to leave {global, ...}, as it will make it clear that it is a global name. Which is probably not used that often anyway...

@Vagabond
Collaborator

Fair enough.

@Vagabond Vagabond merged commit c72a761 into from
@mworrell

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 28 additions and 2 deletions.
  1. +6 −2 src/error_logger_lager_h.erl
  2. +22 −0 test/lager_test_backend.erl
View
8 src/error_logger_lager_h.erl
@@ -100,7 +100,7 @@ handle_event(Event, State) ->
Offender = format_offender(Off),
?LOGFMT(error, Pid,
"Supervisor ~w had child ~s exit with reason ~s in context ~w",
- [element(2, Name), Offender, format_reason(Reason), Ctx]);
+ [supervisor_name(Name), Offender, format_reason(Reason), Ctx]);
_ ->
?LOGMSG(error, Pid, "SUPERVISOR REPORT " ++ print_silly_list(D))
end;
@@ -134,7 +134,7 @@ handle_event(Event, State) ->
MFA = format_mfa(proplists:get_value(mfargs, Started)),
Pid = proplists:get_value(pid, Started),
?LOGFMT(debug, P, "Supervisor ~w started ~s at pid ~w",
- [element(2, Name), MFA, Pid]);
+ [supervisor_name(Name), MFA, Pid]);
_ ->
?LOGMSG(info, P, "PROGRESS REPORT " ++ print_silly_list(D))
end;
@@ -311,3 +311,7 @@ print_silly_list([H|T], Fmt, Acc) ->
print_val(Val) ->
{Str, _} = lager_trunc_io:print(Val, 500),
Str.
+
+
+supervisor_name({local, Name}) -> Name;
+supervisor_name(Name) -> Name.
View
22 test/lager_test_backend.erl
@@ -730,6 +730,17 @@ error_logger_redirect_test_() ->
end
},
+ {"supervisor reports with real error and pid",
+ fun() ->
+ sync_error_logger:error_report(supervisor_report, [{errorContext, france}, {offender, [{name, mini_steve}, {mfargs, {a, b, [c]}}, {pid, bleh}]}, {reason, {function_clause,[{crash,handle_info,[foo]}]}}, {supervisor, somepid}]),
+ _ = gen_event:which_handlers(error_logger),
+ {Level, _, Msg,Metadata} = pop(),
+ ?assertEqual(lager_util:level_to_num(error),Level),
+ ?assertEqual(self(),proplists:get_value(pid,Metadata)),
+ ?assertEqual("Supervisor somepid had child mini_steve started with a:b(c) at bleh exit with reason no function clause matching crash:handle_info(foo) in context france", lists:flatten(Msg))
+ end
+ },
+
{"supervisor_bridge reports",
fun() ->
sync_error_logger:error_report(supervisor_report, [{errorContext, france}, {offender, [{mod, mini_steve}, {pid, bleh}]}, {reason, fired}, {supervisor, {local, steve}}]),
@@ -762,6 +773,17 @@ error_logger_redirect_test_() ->
?assertEqual("Supervisor foo started foo:bar/1 at pid baz", lists:flatten(Msg))
end
},
+ {"supervisor progress report with pid",
+ fun() ->
+ lager:set_loglevel(?MODULE, debug),
+ sync_error_logger:info_report(progress, [{supervisor, somepid}, {started, [{mfargs, {foo, bar, 1}}, {pid, baz}]}]),
+ _ = gen_event:which_handlers(error_logger),
+ {Level, _, Msg,Metadata} = pop(),
+ ?assertEqual(lager_util:level_to_num(debug),Level),
+ ?assertEqual(self(),proplists:get_value(pid,Metadata)),
+ ?assertEqual("Supervisor somepid started foo:bar/1 at pid baz", lists:flatten(Msg))
+ end
+ },
{"crash report for emfile",
fun() ->
sync_error_logger:error_report(crash_report, [[{pid, self()}, {registered_name, []}, {error_info, {error, emfile, [{stack, trace, 1}]}}], []]),
Something went wrong with that request. Please try again.