Merge pull request #13 from basho/adt-fix-term-printing

Fix term printing in errors to properly show embedded lists

src/error_logger_lager_h.erl

@@ -109,8 +109,8 @@ handle_event(Event, State) ->
             Details = lists:sort(D),
             case Details of
                 [{application, App}, {exited, Reason}, {type, _Type}] ->
-                    ?LOG(info, Pid, "Application ~w exited with reason: ~w",
-                        [App, Reason]);
+                    ?LOG(info, Pid, "Application ~w exited with reason: ~s",
+                        [App, format_reason(Reason)]);
                 _ ->
                     ?LOG(info, Pid, print_silly_list(D))
             end;
@@ -171,19 +171,21 @@ format_reason({'function not exported', [{M, F, A},MFA|_]}) ->
 format_reason({undef, [MFA|_]}) ->
     ["call to undefined function ", format_mfa(MFA)];
 format_reason({bad_return_value, Val}) ->
-    io_lib:format("bad return value: ~w", [Val]);
+    ["bad return value: ", print_val(Val)];
+format_reason({{bad_return_value, Val}, MFA}) ->
+    ["bad return value: ", print_val(Val), " in ", format_mfa(MFA)];
 format_reason({{case_clause, Val}, [MFA|_]}) ->
-    [io_lib:format("no case clause matching ~w in ", [Val]), format_mfa(MFA)];
+    ["no case clause matching ", print_val(Val), " in ", format_mfa(MFA)];
 format_reason({function_clause, [MFA|_]}) ->
     ["no function clause matching ", format_mfa(MFA)];
 format_reason({if_clause, [MFA|_]}) ->
     ["no true branch found while evaluating if expression in ", format_mfa(MFA)];
 format_reason({{try_clause, Val}, [MFA|_]}) ->
-    [io_lib:format("no try clause matching ~w in ", [Val]), format_mfa(MFA)]; 
+    ["no try clause matching ", print_val(Val), " in ", format_mfa(MFA)]; 
 format_reason({badarith, [MFA|_]}) ->
     ["bad arithmetic expression in ", format_mfa(MFA)];
 format_reason({{badmatch, Val}, [MFA|_]}) ->
-    [io_lib:format("no match of right hand value ~w in ", [Val]), format_mfa(MFA)];
+    ["no match of right hand value ", print_val(Val), " in ", format_mfa(MFA)];
 format_reason({emfile, _Trace}) ->
     "maximum number of file descriptors exhausted, check ulimit -n";
 format_reason({system_limit, [{M, F, _}|_] = Trace}) ->
@@ -218,29 +220,24 @@ format_reason({{badarity, {Fun, Args}}, [MFA|_]}) ->
 format_reason({noproc, MFA}) ->
     ["no such process or port in call to ", format_mfa(MFA)];
 format_reason({{badfun, Term}, [MFA|_]}) ->
-    [io_lib:format("bad function ~w in ", [Term]), format_mfa(MFA)];
+    ["bad function ", print_val(Term), " in ", format_mfa(MFA)];
 format_reason(Reason) ->
     {Str, _} = lager_trunc_io:print(Reason, 500),
     Str.
 
 format_mfa({M, F, A}) when is_list(A) ->
-    io_lib:format("~w:~w("++format_args(A, [])++")", [M, F | A]);
+    {FmtStr, Args} = format_args(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(Other) ->
     io_lib:format("~w", [Other]).
 
-format_args([], Acc) ->
-    string:join(lists:reverse(Acc), ", ");
-format_args([H|T], Acc) when is_list(H) ->
-    case lager_stdlib:string_p(H) of
-        true ->
-            format_args(T, ["\"~s\""|Acc]);
-        _ ->
-            format_args(T, ["~w"|Acc])
-    end;
-format_args([_|T], Acc) ->
-    format_args(T, ["~w"|Acc]).
+format_args([], FmtAcc, ArgsAcc) ->
+    {string:join(lists:reverse(FmtAcc), ", "), lists:reverse(ArgsAcc)};
+format_args([H|T], FmtAcc, ArgsAcc) ->
+    {Str, _} = lager_trunc_io:print(H, 100),
+    format_args(T, ["~s"|FmtAcc], [Str|ArgsAcc]).
 
 print_silly_list(L) when is_list(L) ->
     case lager_stdlib:string_p(L) of
@@ -260,3 +257,7 @@ print_silly_list([{K,V}|T], Fmt, Acc) ->
     print_silly_list(T, ["~w: ~w" | Fmt], [V, K | Acc]);
 print_silly_list([H|T], Fmt, Acc) ->
     print_silly_list(T, ["~w" | Fmt], [H | Acc]).
+
+print_val(Val) ->
+    {Str, _} = lager_trunc_io:print(Val, 500),
+    Str.

test/crash.erl

@@ -22,11 +22,19 @@ handle_call(badfun, _, State) ->
 	{reply, M(), State};
 handle_call(bad_return, _, _) ->
 	bleh;
+handle_call(bad_return_string, _, _) ->
+	{tuple, {tuple, "string"}};
 handle_call(case_clause, _, State) ->
-		case State of
-			goober ->
-				{reply, ok, State}
-		end;
+	case State of
+		goober ->
+			{reply, ok, State}
+	end;
+handle_call(case_clause_string, _, State) ->
+	Foo = atom_to_list(?MODULE),
+	case Foo of
+		State ->
+			{reply, ok, State}
+	end;
 handle_call(if_clause, _, State) ->
 	if State == 1 ->
 			{reply, ok, State}
@@ -56,12 +64,12 @@ handle_call(system_limit, _, State) ->
 	Res = list_to_atom(lists:flatten(lists:duplicate(256, "a"))),
 	{reply, Res, State};
 handle_call(process_limit, _, State) ->
-    %% run with +P 300 to make this crash
-    [erlang:spawn(fun() -> timer:sleep(5000) end) || _ <- lists:seq(0, 500)],
-    {reply, ok, State};
+	%% run with +P 300 to make this crash
+	[erlang:spawn(fun() -> timer:sleep(5000) end) || _ <- lists:seq(0, 500)],
+	{reply, ok, State};
 handle_call(port_limit, _, State) ->
-    [erlang:open_port({spawn, "ls"}, []) || _ <- lists:seq(0, 1024)],
-    {reply, ok, State};
+	[erlang:open_port({spawn, "ls"}, []) || _ <- lists:seq(0, 1024)],
+	{reply, ok, State};
 handle_call(noproc, _, State) ->
 	Res = gen_event:call(foo, bar, baz),
 	{reply, Res, State};

test/lager_test_backend.erl

@@ -237,6 +237,15 @@ error_logger_redirect_crash_test_() ->
                         ?assertEqual(Expected, lists:flatten(Msg))
                 end
             },
+            {"bad return value with string",
+                fun() ->
+                        Pid = whereis(crash),
+                        crash(bad_return_string),
+                        {_, _, Msg} = pop(),
+                        Expected = lists:flatten(io_lib:format("[error] ~w gen_server crash terminated with reason: bad return value: {tuple,{tuple,\"string\"}}", [Pid])),
+                        ?assertEqual(Expected, lists:flatten(Msg))
+                end
+            },
             {"case clause",
                 fun() ->
                         Pid = whereis(crash),
@@ -246,6 +255,15 @@ error_logger_redirect_crash_test_() ->
                         ?assertEqual(Expected, lists:flatten(Msg))
                 end
             },
+            {"case clause string",
+                fun() ->
+                        Pid = whereis(crash),
+                        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))
+                end
+            },
             {"function clause",
                 fun() ->
                         Pid = whereis(crash),
@@ -323,7 +341,7 @@ error_logger_redirect_crash_test_() ->
                         Pid = whereis(crash),
                         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([[102,111,111],bar]) in crash:handle_call/3", [Pid])),
+                        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))
                 end
             },