aboutsummaryrefslogtreecommitdiffstats
path: root/src/cowboy_stream_h.erl
diff options
context:
space:
mode:
authorLoïc Hoguin <[email protected]>2017-09-14 15:34:37 +0200
committerLoïc Hoguin <[email protected]>2017-09-14 15:34:37 +0200
commit5027d1335d1ca39e614f56bea199aacac4d3d5a7 (patch)
tree9aa59982acf82958d9547224e46e506e966608e9 /src/cowboy_stream_h.erl
parent3cbf885961dc93df1b39d2de89f2a871402acbd5 (diff)
downloadcowboy-5027d1335d1ca39e614f56bea199aacac4d3d5a7.tar.gz
cowboy-5027d1335d1ca39e614f56bea199aacac4d3d5a7.tar.bz2
cowboy-5027d1335d1ca39e614f56bea199aacac4d3d5a7.zip
Rework the proc_lib_hack
It is completely removed for connection processes, because assuming Cowboy is written properly this should bring us nothing anymore in 2.0. It is reworked for request processes, there we want to always propagate the stacktrace (including for exits) because we will print a report to help with debugging and proc_lib doesn't propagate it for exits. At the same time the initial callback for connection and request processes has been changed to connection_process and request_process, which should help with identifying processes when inspecting.
Diffstat (limited to 'src/cowboy_stream_h.erl')
-rw-r--r--src/cowboy_stream_h.erl37
1 files changed, 19 insertions, 18 deletions
diff --git a/src/cowboy_stream_h.erl b/src/cowboy_stream_h.erl
index 4459679..cf1d38a 100644
--- a/src/cowboy_stream_h.erl
+++ b/src/cowboy_stream_h.erl
@@ -21,7 +21,7 @@
-export([terminate/3]).
-export([early_error/5]).
--export([proc_lib_hack/3]).
+-export([request_process/3]).
-export([execute/3]).
-export([resume/5]).
@@ -47,7 +47,7 @@ init(_StreamID, Req=#{ref := Ref}, Opts) ->
Env = maps:get(env, Opts, #{}),
Middlewares = maps:get(middlewares, Opts, [cowboy_router, cowboy_handler]),
Shutdown = maps:get(shutdown_timeout, Opts, 5000),
- Pid = proc_lib:spawn_link(?MODULE, proc_lib_hack, [Req, Env, Middlewares]),
+ Pid = proc_lib:spawn_link(?MODULE, request_process, [Req, Env, Middlewares]),
{[{spawn, Pid, Shutdown}], #state{ref=Ref, pid=Pid}}.
%% If we receive data and stream is waiting for data:
@@ -120,7 +120,6 @@ info(_StreamID, SwitchProtocol = {switch_protocol, _, _, _}, State) ->
{[SwitchProtocol], State};
%% Stray message.
info(_StreamID, _Info, State) ->
- %% @todo Cleanup if no reply was sent when stream ends.
{[], State}.
-spec terminate(cowboy_stream:streamid(), cowboy_stream:reason(), #state{}) -> ok.
@@ -150,24 +149,26 @@ report_crash(Ref, StreamID, Pid, Reason, Stacktrace) ->
%% Request process.
-%% @todo This should wrap with try/catch to get the full error
-%% in the stream handler. Only then can we decide what to do
-%% about it. This means that we should remove any other try/catch
-%% in the request process.
-
-%% This hack is necessary because proc_lib does not propagate
-%% stacktraces by default. This is ugly because we end up
-%% having two try/catch instead of one (the one in proc_lib),
-%% just to add the stacktrace information.
+%% We catch all exceptions in order to add the stacktrace to
+%% the exit reason as it is not propagated by proc_lib otherwise
+%% and therefore not present in the 'EXIT' message. We want
+%% the stacktrace in order to simplify debugging of errors.
+%%
+%% This + the behavior in proc_lib means that we will get a
+%% {Reason, Stacktrace} tuple for every exceptions, instead of
+%% just for errors and throws.
%%
-%% @todo Remove whenever proc_lib propagates stacktraces.
--spec proc_lib_hack(_, _, _) -> _.
-proc_lib_hack(Req, Env, Middlewares) ->
+%% @todo Better spec.
+-spec request_process(_, _, _) -> _.
+request_process(Req, Env, Middlewares) ->
try
execute(Req, Env, Middlewares)
- catch _:Reason ->
- %% @todo Have a way to identify OTP 20 to not do this twice?
- exit({Reason, erlang:get_stacktrace()})
+ catch
+ exit:Reason ->
+ Stacktrace = erlang:get_stacktrace(),
+ erlang:raise(exit, {Reason, Stacktrace}, Stacktrace);
+ Class:Reason ->
+ erlang:raise(Class, Reason, erlang:get_stacktrace())
end.
%% @todo