From 68065c166b031ac6418362eeab1fa452e789e9d1 Mon Sep 17 00:00:00 2001 From: Andrew Thompson Date: Wed, 26 Oct 2011 10:50:06 -0400 Subject: [PATCH 1/4] Intial support for R15 line numbers in errors --- src/error_logger_lager_h.erl | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/error_logger_lager_h.erl b/src/error_logger_lager_h.erl index 3bbe468..0b36326 100644 --- a/src/error_logger_lager_h.erl +++ b/src/error_logger_lager_h.erl @@ -250,6 +250,13 @@ format_mfa({M, F, A}) when is_list(A) -> io_lib:format("~w:~w("++FmtStr++")", [M, F | Args]); format_mfa({M, F, A}) when is_integer(A) -> io_lib:format("~w:~w/~w", [M, F, A]); +format_mfa({M, F, A, Props}) when is_list(Props) -> + case {proplists:get_value(file, Props), proplists:get_value(line, Props)} of + {undefined, undefined} -> + format_mfa({M, F, A}); + {File, Line} -> + [format_mfa({M, F, A}), io_lib:format(" (~s:~w)", [File, Line])] + end; format_mfa(Other) -> io_lib:format("~w", [Other]). From 807a8476388347ea8d414b44aa971e00ba9f97eb Mon Sep 17 00:00:00 2001 From: Andrew Thompson Date: Sun, 1 Apr 2012 15:10:44 -0400 Subject: [PATCH 2/4] Fix tests. and 2 places MFAs werent handling the new 4th parameter Tests now pass on both R15B and R14B03. --- src/error_logger_lager_h.erl | 7 ++++++ test/lager_test_backend.erl | 45 +++++++++++++++++++++++++----------- 2 files changed, 39 insertions(+), 13 deletions(-) diff --git a/src/error_logger_lager_h.erl b/src/error_logger_lager_h.erl index 0b36326..1d62241 100644 --- a/src/error_logger_lager_h.erl +++ b/src/error_logger_lager_h.erl @@ -188,6 +188,10 @@ format_offender(Off) -> format_reason({'function not exported', [{M, F, A},MFA|_]}) -> ["call to undefined function ", format_mfa({M, F, length(A)}), " from ", format_mfa(MFA)]; +format_reason({'function not exported', [{M, F, A, _Props},MFA|_]}) -> + %% R15 line numbers + ["call to undefined function ", format_mfa({M, F, length(A)}), + " from ", format_mfa(MFA)]; format_reason({undef, [MFA|_]}) -> ["call to undefined function ", format_mfa(MFA)]; format_reason({bad_return_value, Val}) -> @@ -227,6 +231,9 @@ format_reason({system_limit, [{M, F, _}|_] = Trace}) -> ["system limit: ", Limit]; format_reason({badarg, [MFA,MFA2|_]}) -> case MFA of + {_M, _F, A, _Props} when is_list(A) -> + %% R15 line numbers + ["bad argument in call to ", format_mfa(MFA), " in ", format_mfa(MFA2)]; {_M, _F, A} when is_list(A) -> ["bad argument in call to ", format_mfa(MFA), " in ", format_mfa(MFA2)]; _ -> diff --git a/test/lager_test_backend.erl b/test/lager_test_backend.erl index b6bc71c..63d5457 100644 --- a/test/lager_test_backend.erl +++ b/test/lager_test_backend.erl @@ -87,6 +87,12 @@ count_ignored() -> flush() -> gen_event:call(lager_event, ?MODULE, flush). +has_line_numbers() -> + %% are we R15 or greater + Rel = erlang:system_info(otp_release), + {match, [Major]} = re:run(Rel, "^R(\\d+)[A|B](|0(\\d))$", [{capture, [1], list}]), + list_to_integer(Major) >= 15. + not_running_test() -> ?assertEqual({error, lager_not_running}, lager:log(info, self(), "not running")). @@ -241,6 +247,18 @@ crash(Type) -> _ = gen_event:which_handlers(error_logger), ok. +test_body(Expected, Actual, File) -> + case has_line_numbers() of + true -> + FileLine = string:substr(Actual, length(Expected)+1), + Body = string:substr(Actual, 1, length(Expected)), + ?assertEqual(Expected, Body), + ?assertEqual(File, string:substr(FileLine, 3, length(File))); + false -> + ?assertEqual(Expected, Actual) + end. + + error_logger_redirect_crash_test_() -> {foreach, fun() -> @@ -276,6 +294,7 @@ error_logger_redirect_crash_test_() -> {_, _, Msg} = pop(), Expected = lists:flatten(io_lib:format("[error] ~w gen_server crash terminated with reason: bad return value: bleh", [Pid])), ?assertEqual(Expected, lists:flatten(Msg)) + %test_body(Expected, lists:flatten(msg), "test/crash.erl") end }, {"bad return value with string", @@ -293,7 +312,7 @@ error_logger_redirect_crash_test_() -> crash(case_clause), {_, _, Msg} = pop(), Expected = lists:flatten(io_lib:format("[error] ~w gen_server crash terminated with reason: no case clause matching {} in crash:handle_call/3", [Pid])), - ?assertEqual(Expected, lists:flatten(Msg)) + test_body(Expected, lists:flatten(Msg), "test/crash.erl") end }, {"case clause string", @@ -302,7 +321,7 @@ error_logger_redirect_crash_test_() -> crash(case_clause_string), {_, _, Msg} = pop(), Expected = lists:flatten(io_lib:format("[error] ~w gen_server crash terminated with reason: no case clause matching \"crash\" in crash:handle_call/3", [Pid])), - ?assertEqual(Expected, lists:flatten(Msg)) + test_body(Expected, lists:flatten(Msg), "test/crash.erl") end }, {"function clause", @@ -311,7 +330,7 @@ error_logger_redirect_crash_test_() -> crash(function_clause), {_, _, Msg} = pop(), Expected = lists:flatten(io_lib:format("[error] ~w gen_server crash terminated with reason: no function clause matching crash:function({})", [Pid])), - ?assertEqual(Expected, lists:flatten(Msg)) + test_body(Expected, lists:flatten(Msg), "test/crash.erl") end }, {"if clause", @@ -320,7 +339,7 @@ error_logger_redirect_crash_test_() -> crash(if_clause), {_, _, Msg} = pop(), Expected = lists:flatten(io_lib:format("[error] ~w gen_server crash terminated with reason: no true branch found while evaluating if expression in crash:handle_call/3", [Pid])), - ?assertEqual(Expected, lists:flatten(Msg)) + test_body(Expected, lists:flatten(Msg), "test/crash.erl") end }, {"try clause", @@ -329,7 +348,7 @@ error_logger_redirect_crash_test_() -> crash(try_clause), {_, _, Msg} = pop(), Expected = lists:flatten(io_lib:format("[error] ~w gen_server crash terminated with reason: no try clause matching [] in crash:handle_call/3", [Pid])), - ?assertEqual(Expected, lists:flatten(Msg)) + test_body(Expected, lists:flatten(Msg), "test/crash.erl") end }, {"undefined function", @@ -338,7 +357,7 @@ error_logger_redirect_crash_test_() -> crash(undef), {_, _, Msg} = pop(), Expected = lists:flatten(io_lib:format("[error] ~w gen_server crash terminated with reason: call to undefined function crash:booger/0 from crash:handle_call/3", [Pid])), - ?assertEqual(Expected, lists:flatten(Msg)) + test_body(Expected, lists:flatten(Msg), "test/crash.erl") end }, {"bad math", @@ -347,7 +366,7 @@ error_logger_redirect_crash_test_() -> crash(badarith), {_, _, Msg} = pop(), Expected = lists:flatten(io_lib:format("[error] ~w gen_server crash terminated with reason: bad arithmetic expression in crash:handle_call/3", [Pid])), - ?assertEqual(Expected, lists:flatten(Msg)) + test_body(Expected, lists:flatten(Msg), "test/crash.erl") end }, {"bad match", @@ -356,7 +375,7 @@ error_logger_redirect_crash_test_() -> crash(badmatch), {_, _, Msg} = pop(), Expected = lists:flatten(io_lib:format("[error] ~w gen_server crash terminated with reason: no match of right hand value {} in crash:handle_call/3", [Pid])), - ?assertEqual(Expected, lists:flatten(Msg)) + test_body(Expected, lists:flatten(Msg), "test/crash.erl") end }, {"bad arity", @@ -365,7 +384,7 @@ error_logger_redirect_crash_test_() -> crash(badarity), {_, _, Msg} = pop(), Expected = lists:flatten(io_lib:format("[error] ~w gen_server crash terminated with reason: fun called with wrong arity of 1 instead of 3 in crash:handle_call/3", [Pid])), - ?assertEqual(Expected, lists:flatten(Msg)) + test_body(Expected, lists:flatten(Msg), "test/crash.erl") end }, {"bad arg1", @@ -374,7 +393,7 @@ error_logger_redirect_crash_test_() -> crash(badarg1), {_, _, Msg} = pop(), Expected = lists:flatten(io_lib:format("[error] ~w gen_server crash terminated with reason: bad argument in crash:handle_call/3", [Pid])), - ?assertEqual(Expected, lists:flatten(Msg)) + test_body(Expected, lists:flatten(Msg), "test/crash.erl") end }, {"bad arg2", @@ -383,7 +402,7 @@ error_logger_redirect_crash_test_() -> crash(badarg2), {_, _, Msg} = pop(), Expected = lists:flatten(io_lib:format("[error] ~w gen_server crash terminated with reason: bad argument in call to erlang:iolist_to_binary([\"foo\",bar]) in crash:handle_call/3", [Pid])), - ?assertEqual(Expected, lists:flatten(Msg)) + test_body(Expected, lists:flatten(Msg), "test/crash.erl") end }, {"noproc", @@ -401,7 +420,7 @@ error_logger_redirect_crash_test_() -> crash(badfun), {_, _, Msg} = pop(), Expected = lists:flatten(io_lib:format("[error] ~w gen_server crash terminated with reason: bad function booger in crash:handle_call/3", [Pid])), - ?assertEqual(Expected, lists:flatten(Msg)) + test_body(Expected, lists:flatten(Msg), "test/crash.erl") end } @@ -699,7 +718,7 @@ error_logger_redirect_test_() -> {_, _, Msg} = pop(), Expected = lists:flatten(io_lib:format("[error] ~p CRASH REPORT Process ~p with 0 neighbours crashed with reason: no function clause matching special_process:foo(bar)", [Pid, Pid])), - ?assertEqual(Expected, lists:flatten(Msg)) + test_body(Expected, lists:flatten(Msg), "test/special_process.erl") end }, {"messages should not be generated if they don't satisfy the threshold", From d68f4d855fb923587a54672ba0ab93acba0474aa Mon Sep 17 00:00:00 2001 From: Andrew Thompson Date: Sun, 1 Apr 2012 16:01:19 -0400 Subject: [PATCH 3/4] Add support for bad_return errors from application start --- src/error_logger_lager_h.erl | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/error_logger_lager_h.erl b/src/error_logger_lager_h.erl index 1d62241..a67bb91 100644 --- a/src/error_logger_lager_h.erl +++ b/src/error_logger_lager_h.erl @@ -194,6 +194,10 @@ format_reason({'function not exported', [{M, F, A, _Props},MFA|_]}) -> " from ", format_mfa(MFA)]; format_reason({undef, [MFA|_]}) -> ["call to undefined function ", format_mfa(MFA)]; +format_reason({bad_return, {_MFA, {'EXIT', Reason}}}) -> + format_reason(Reason); +format_reason({bad_return, {MFA, Val}}) -> + ["bad return value ", print_val(Val), " from ", format_mfa(MFA)]; format_reason({bad_return_value, Val}) -> ["bad return value: ", print_val(Val)]; format_reason({{bad_return_value, Val}, MFA}) -> From b52e204f24c07302d5188448b7e1c2bf35080425 Mon Sep 17 00:00:00 2001 From: Andrew Thompson Date: Sun, 1 Apr 2012 16:01:41 -0400 Subject: [PATCH 4/4] Don't bother printing the filename in error messages, it is always redundant Whenever we have the filename we ALWAYS have the module name, which is effectively just as useful and already printed. --- src/error_logger_lager_h.erl | 8 ++++---- test/lager_test_backend.erl | 31 +++++++++++++++---------------- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/src/error_logger_lager_h.erl b/src/error_logger_lager_h.erl index a67bb91..cfe2667 100644 --- a/src/error_logger_lager_h.erl +++ b/src/error_logger_lager_h.erl @@ -262,11 +262,11 @@ format_mfa({M, F, A}) when is_list(A) -> format_mfa({M, F, A}) when is_integer(A) -> io_lib:format("~w:~w/~w", [M, F, A]); format_mfa({M, F, A, Props}) when is_list(Props) -> - case {proplists:get_value(file, Props), proplists:get_value(line, Props)} of - {undefined, undefined} -> + case proplists:get_value(line, Props) of + undefined -> format_mfa({M, F, A}); - {File, Line} -> - [format_mfa({M, F, A}), io_lib:format(" (~s:~w)", [File, Line])] + Line -> + [format_mfa({M, F, A}), io_lib:format(" line ~w", [Line])] end; format_mfa(Other) -> io_lib:format("~w", [Other]). diff --git a/test/lager_test_backend.erl b/test/lager_test_backend.erl index 63d5457..b25e03f 100644 --- a/test/lager_test_backend.erl +++ b/test/lager_test_backend.erl @@ -247,13 +247,13 @@ crash(Type) -> _ = gen_event:which_handlers(error_logger), ok. -test_body(Expected, Actual, File) -> +test_body(Expected, Actual) -> case has_line_numbers() of true -> FileLine = string:substr(Actual, length(Expected)+1), Body = string:substr(Actual, 1, length(Expected)), ?assertEqual(Expected, Body), - ?assertEqual(File, string:substr(FileLine, 3, length(File))); + ?assertEqual(" line ", string:substr(FileLine, 1, 6)); false -> ?assertEqual(Expected, Actual) end. @@ -294,7 +294,6 @@ error_logger_redirect_crash_test_() -> {_, _, Msg} = pop(), Expected = lists:flatten(io_lib:format("[error] ~w gen_server crash terminated with reason: bad return value: bleh", [Pid])), ?assertEqual(Expected, lists:flatten(Msg)) - %test_body(Expected, lists:flatten(msg), "test/crash.erl") end }, {"bad return value with string", @@ -312,7 +311,7 @@ error_logger_redirect_crash_test_() -> crash(case_clause), {_, _, Msg} = pop(), Expected = lists:flatten(io_lib:format("[error] ~w gen_server crash terminated with reason: no case clause matching {} in crash:handle_call/3", [Pid])), - test_body(Expected, lists:flatten(Msg), "test/crash.erl") + test_body(Expected, lists:flatten(Msg)) end }, {"case clause string", @@ -321,7 +320,7 @@ error_logger_redirect_crash_test_() -> crash(case_clause_string), {_, _, Msg} = pop(), Expected = lists:flatten(io_lib:format("[error] ~w gen_server crash terminated with reason: no case clause matching \"crash\" in crash:handle_call/3", [Pid])), - test_body(Expected, lists:flatten(Msg), "test/crash.erl") + test_body(Expected, lists:flatten(Msg)) end }, {"function clause", @@ -330,7 +329,7 @@ error_logger_redirect_crash_test_() -> crash(function_clause), {_, _, Msg} = pop(), Expected = lists:flatten(io_lib:format("[error] ~w gen_server crash terminated with reason: no function clause matching crash:function({})", [Pid])), - test_body(Expected, lists:flatten(Msg), "test/crash.erl") + test_body(Expected, lists:flatten(Msg)) end }, {"if clause", @@ -339,7 +338,7 @@ error_logger_redirect_crash_test_() -> crash(if_clause), {_, _, Msg} = pop(), Expected = lists:flatten(io_lib:format("[error] ~w gen_server crash terminated with reason: no true branch found while evaluating if expression in crash:handle_call/3", [Pid])), - test_body(Expected, lists:flatten(Msg), "test/crash.erl") + test_body(Expected, lists:flatten(Msg)) end }, {"try clause", @@ -348,7 +347,7 @@ error_logger_redirect_crash_test_() -> crash(try_clause), {_, _, Msg} = pop(), Expected = lists:flatten(io_lib:format("[error] ~w gen_server crash terminated with reason: no try clause matching [] in crash:handle_call/3", [Pid])), - test_body(Expected, lists:flatten(Msg), "test/crash.erl") + test_body(Expected, lists:flatten(Msg)) end }, {"undefined function", @@ -357,7 +356,7 @@ error_logger_redirect_crash_test_() -> crash(undef), {_, _, Msg} = pop(), Expected = lists:flatten(io_lib:format("[error] ~w gen_server crash terminated with reason: call to undefined function crash:booger/0 from crash:handle_call/3", [Pid])), - test_body(Expected, lists:flatten(Msg), "test/crash.erl") + test_body(Expected, lists:flatten(Msg)) end }, {"bad math", @@ -366,7 +365,7 @@ error_logger_redirect_crash_test_() -> crash(badarith), {_, _, Msg} = pop(), Expected = lists:flatten(io_lib:format("[error] ~w gen_server crash terminated with reason: bad arithmetic expression in crash:handle_call/3", [Pid])), - test_body(Expected, lists:flatten(Msg), "test/crash.erl") + test_body(Expected, lists:flatten(Msg)) end }, {"bad match", @@ -375,7 +374,7 @@ error_logger_redirect_crash_test_() -> crash(badmatch), {_, _, Msg} = pop(), Expected = lists:flatten(io_lib:format("[error] ~w gen_server crash terminated with reason: no match of right hand value {} in crash:handle_call/3", [Pid])), - test_body(Expected, lists:flatten(Msg), "test/crash.erl") + test_body(Expected, lists:flatten(Msg)) end }, {"bad arity", @@ -384,7 +383,7 @@ error_logger_redirect_crash_test_() -> crash(badarity), {_, _, Msg} = pop(), Expected = lists:flatten(io_lib:format("[error] ~w gen_server crash terminated with reason: fun called with wrong arity of 1 instead of 3 in crash:handle_call/3", [Pid])), - test_body(Expected, lists:flatten(Msg), "test/crash.erl") + test_body(Expected, lists:flatten(Msg)) end }, {"bad arg1", @@ -393,7 +392,7 @@ error_logger_redirect_crash_test_() -> crash(badarg1), {_, _, Msg} = pop(), Expected = lists:flatten(io_lib:format("[error] ~w gen_server crash terminated with reason: bad argument in crash:handle_call/3", [Pid])), - test_body(Expected, lists:flatten(Msg), "test/crash.erl") + test_body(Expected, lists:flatten(Msg)) end }, {"bad arg2", @@ -402,7 +401,7 @@ error_logger_redirect_crash_test_() -> crash(badarg2), {_, _, Msg} = pop(), Expected = lists:flatten(io_lib:format("[error] ~w gen_server crash terminated with reason: bad argument in call to erlang:iolist_to_binary([\"foo\",bar]) in crash:handle_call/3", [Pid])), - test_body(Expected, lists:flatten(Msg), "test/crash.erl") + test_body(Expected, lists:flatten(Msg)) end }, {"noproc", @@ -420,7 +419,7 @@ error_logger_redirect_crash_test_() -> crash(badfun), {_, _, Msg} = pop(), Expected = lists:flatten(io_lib:format("[error] ~w gen_server crash terminated with reason: bad function booger in crash:handle_call/3", [Pid])), - test_body(Expected, lists:flatten(Msg), "test/crash.erl") + test_body(Expected, lists:flatten(Msg)) end } @@ -718,7 +717,7 @@ error_logger_redirect_test_() -> {_, _, Msg} = pop(), Expected = lists:flatten(io_lib:format("[error] ~p CRASH REPORT Process ~p with 0 neighbours crashed with reason: no function clause matching special_process:foo(bar)", [Pid, Pid])), - test_body(Expected, lists:flatten(Msg), "test/special_process.erl") + test_body(Expected, lists:flatten(Msg)) end }, {"messages should not be generated if they don't satisfy the threshold",