knutin · potomak · Apr 2, 2013 · Apr 4, 2013 · Apr 4, 2013
diff --git a/src/elli_http.erl b/src/elli_http.erl
@@ -226,27 +226,15 @@ execute_callback(Req, {Mod, Args}) ->
             {response, ResponseCode, Headers, Body};
         throw:Exc ->
             handle_event(Mod, request_throw, [Req, Exc, erlang:get_stacktrace()], Args),
-            internal_error_response(Req);
+            {response, 500, [], <<"Internal server error">>};
         error:Error ->
             handle_event(Mod, request_error, [Req, Error, erlang:get_stacktrace()], Args),
-            internal_error_response(Req);
+            {response, 500, [], <<"Internal server error">>};
         exit:Exit ->
             handle_event(Mod, request_exit, [Req, Exit, erlang:get_stacktrace()], Args),
-            internal_error_response(Req)
+            {response, 500, [], <<"Internal server error">>}
     end.
 
-internal_error_response(Req) ->
-    Body = <<"<html>"
-             "<head><title>Internal server error</title></head>"
-             "<body>"
-             "<h1>Internal server error</h1>"
-             "<p>Request id: #", (list_to_binary(
-                                    integer_to_list(
-                                      elli_request:id(Req))))/binary, "</p>"
-             "</body>"
-             "</html>">>,
-
-    {response, 500, [], Body}.
 
 handle_event(Mod, Name, EventArgs, ElliArgs) ->
     try

diff --git a/src/elli_middleware.erl b/src/elli_middleware.erl
@@ -39,8 +39,21 @@
 handle(CleanReq, Config) ->
     Mods   = mods(Config),
     PreReq = preprocess(CleanReq, Mods),
-    Res    = process(PreReq, Mods),
-    postprocess(PreReq, Res, lists:reverse(Mods)).
+    try process(PreReq, Mods) of
+        Res -> postprocess(PreReq, Res, lists:reverse(Mods))
+    catch
+        throw:{ResponseCode, Headers, Body} when is_integer(ResponseCode) ->
+            postprocess(PreReq, {ResponseCode, Headers, Body}, lists:reverse(Mods));
+        throw:Exc ->
+            handle_event(request_throw, [PreReq, Exc, erlang:get_stacktrace()], Config),
+            postprocess(PreReq, {500, [], <<"Internal server error">>}, lists:reverse(Mods));
+        error:Error ->
+            handle_event(request_error, [PreReq, Error, erlang:get_stacktrace()], Config),
+            postprocess(PreReq, {500, [], <<"Internal server error">>}, lists:reverse(Mods));
+        exit:Exit ->
+            handle_event(request_exit, [PreReq, Exit, erlang:get_stacktrace()], Config),
+            postprocess(PreReq, {500, [], <<"Internal server error">>}, lists:reverse(Mods))
+    end.
 
 
 handle_event(elli_startup, Args, Config) ->

diff --git a/src/elli_middleware_error_responses.erl b/src/elli_middleware_error_responses.erl
@@ -0,0 +1,41 @@
+%% @doc: Add request reference to error response body as Elli middleware.
+%% Postprocesses all requests and add request's unique reference to response
+%% body.
+
+-module(elli_middleware_error_responses).
+-export([postprocess/3]).
+
+%%
+%% Postprocess handler
+%%
+
+postprocess(Req, {ResponseCode, Headers, Body} = Res, _Args)
+  when is_integer(ResponseCode) ->
+    case is_error(ResponseCode) of
+        false ->
+            Res;
+        true ->
+            error_response(Req, Res)
+    end;
+postprocess(_, Res, _) ->
+    Res.
+
+%%
+%% INTERNALS
+%%
+
+is_error(ResponseCode) ->
+    ResponseCode >= 400 andalso ResponseCode < 600.
+
+error_response(Req, {ResponseCode, Headers, Body} = Res) ->
+    NewBody = <<"<html>"
+             "<head><title>", Body/binary, "</title></head>"
+             "<body>"
+             "<h1>", Body/binary, "</h1>"
+             "<p>Request id: #", (list_to_binary(
+                                    integer_to_list(
+                                      elli_request:id(Req))))/binary, "</p>"
+             "</body>"
+             "</html>">>,
+
+    {ResponseCode, Headers, NewBody}.
diff --git a/test/elli_middleware_tests.erl b/test/elli_middleware_tests.erl
@@ -9,7 +9,8 @@ elli_test_() ->
      [
       ?_test(hello_world()),
       ?_test(short_circuit()),
-      ?_test(compress())
+      ?_test(compress()),
+      ?_test(error_responses())
      ]}.
 
 %%
@@ -58,6 +59,28 @@ compress() ->
     ?assertEqual(lists:flatten(lists:duplicate(86, "Hello World!")), body(Response3)).
 
 
+error_responses() ->
+    {ok, Response} = httpc:request("http://localhost:3002/foobarbaz"),
+    ?assertEqual(404, status(Response)),
+    ?assertNotMatch(nomatch, binary:match(list_to_binary(body(Response)),
+                                          <<"Not Found">>)),
+    ?assertNotMatch(nomatch, binary:match(list_to_binary(body(Response)),
+                                          <<"Request id">>)),
+
+    {ok, Response1} = httpc:request("http://localhost:3002/crash"),
+    ?assertEqual(500, status(Response1)),
+    ?assertNotMatch(nomatch, binary:match(list_to_binary(body(Response1)),
+                                          <<"Internal server error">>)),
+    ?assertNotMatch(nomatch, binary:match(list_to_binary(body(Response1)),
+                                          <<"Request id">>)),
+    ?assert(lists:member(request_throw, helper_events_server:get())),
+
+    {ok, Response2} = httpc:request("http://localhost:3002/403"),
+    ?assertEqual(403, status(Response2)),
+    ?assertNotMatch(nomatch, binary:match(list_to_binary(body(Response2)),
+                                          <<"Forbidden">>)),
+    ?assertNotMatch(nomatch, binary:match(list_to_binary(body(Response2)),
+                                          <<"Request id">>)).
 
 
 
@@ -80,6 +103,7 @@ setup() ->
     application:start(public_key),
     application:start(ssl),
     inets:start(),
+    helper_events_server:start(),
 
     Config = [
               {mods, [
@@ -88,7 +112,9 @@ setup() ->
                                          {port, 514}]},
                       {elli_example_middleware, []},
                       {elli_middleware_compress, []},
-                      {elli_example_callback, []}
+                      {elli_middleware_error_responses, []},
+                      {elli_example_callback, []},
+                      {helper_event_handler, []}
                      ]}
              ],
 
@@ -99,6 +125,5 @@ setup() ->
     [P].
 
 teardown(Pids) ->
+    helper_events_server:stop(),
     [elli:stop(P) || P <- Pids].
-
-
diff --git a/test/elli_tests.erl b/test/elli_tests.erl
@@ -76,12 +76,8 @@ crash() ->
     {ok, Response} = httpc:request("http://localhost:3001/crash"),
     ?assertEqual(500, status(Response)),
     ?assertEqual([{"connection", "Keep-Alive"},
-                  {"content-length", "142"}], headers(Response)),
-    ?assertNotEqual(nomatch, binary:match(list_to_binary(body(Response)),
-                                          <<"Internal server error">>)),
-    ?assertNotEqual(nomatch, binary:match(list_to_binary(body(Response)),
-                                          <<"Request id">>)).
-
+                  {"content-length", "21"}], headers(Response)),
+    ?assertEqual("Internal server error", body(Response)).
 
 no_compress() ->
     {ok, Response} = httpc:request(get, {"http://localhost:3001/compressed",

diff --git a/test/helper_event_handler.erl b/test/helper_event_handler.erl
@@ -0,0 +1,10 @@
+-module(helper_event_handler).
+-behaviour(elli_handler).
+-export([handle/2, handle_event/3]).
+
+handle(_Req, _Args) -> ignore.
+
+handle_event(request_throw, [_Request, _Exception, _Stacktrace], _) ->
+    helper_events_server:add(request_throw),
+    ok;
+handle_event(_, _, _) -> ok.
diff --git a/test/helper_events_server.erl b/test/helper_events_server.erl
@@ -0,0 +1,35 @@
+-module(helper_events_server).
+-export([start/0, stop/0, add/1, get/0, init/0]).
+
+start() ->
+    Pid = spawn_link(?MODULE, init, []),
+    register(?MODULE, Pid),
+    ok.
+
+stop() ->
+    ?MODULE ! stop,
+    unregister(?MODULE).
+
+add(Event) ->
+    ?MODULE ! {add, Event},
+    ok.
+
+get() ->
+    ?MODULE ! {events, self()},
+    receive
+        X -> X
+    end.
+
+init() ->
+    loop([]).
+
+loop(Events) ->
+    receive
+        stop ->
+            ok;
+        {events, From} ->
+            From ! Events,
+            loop(Events);
+        {add, Event} ->
+            loop(lists:append([Event], Events))
+    end.