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.

src/cowboy_clear.erl

@@ -16,26 +16,16 @@
 -behavior(ranch_protocol).
 
 -export([start_link/4]).
--export([proc_lib_hack/5]).
+-export([connection_process/5]).
 
 -spec start_link(ranch:ref(), inet:socket(), module(), cowboy:opts()) -> {ok, pid()}.
 start_link(Ref, Socket, Transport, Opts) ->
-	Pid = proc_lib:spawn_link(?MODULE, proc_lib_hack, [self(), Ref, Socket, Transport, Opts]),
+	Pid = proc_lib:spawn_link(?MODULE, connection_process,
+		[self(), Ref, Socket, Transport, Opts]),
 	{ok, Pid}.
 
--spec proc_lib_hack(pid(), ranch:ref(), inet:socket(), module(), cowboy:opts()) -> ok.
-proc_lib_hack(Parent, Ref, Socket, Transport, Opts) ->
-	try
-		init(Parent, Ref, Socket, Transport, Opts)
-	catch
-		_:normal -> exit(normal);
-		_:shutdown -> exit(shutdown);
-		_:Reason = {shutdown, _} -> exit(Reason);
-		_:Reason -> exit({Reason, erlang:get_stacktrace()})
-	end.
-
--spec init(pid(), ranch:ref(), inet:socket(), module(), cowboy:opts()) -> ok.
-init(Parent, Ref, Socket, Transport, Opts) ->
+-spec connection_process(pid(), ranch:ref(), inet:socket(), module(), cowboy:opts()) -> ok.
+connection_process(Parent, Ref, Socket, Transport, Opts) ->
 	ok = ranch:accept_ack(Ref),
 	init(Parent, Ref, Socket, Transport, Opts, cowboy_http).
 

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.
 %%
-%% @todo Remove whenever proc_lib propagates stacktraces.
--spec proc_lib_hack(_, _, _) -> _.
-proc_lib_hack(Req, Env, Middlewares) ->
+%% 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 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

src/cowboy_tls.erl

@@ -16,26 +16,16 @@
 -behavior(ranch_protocol).
 
 -export([start_link/4]).
--export([proc_lib_hack/5]).
+-export([connection_process/5]).
 
 -spec start_link(ranch:ref(), ssl:sslsocket(), module(), cowboy:opts()) -> {ok, pid()}.
 start_link(Ref, Socket, Transport, Opts) ->
-	Pid = proc_lib:spawn_link(?MODULE, proc_lib_hack, [self(), Ref, Socket, Transport, Opts]),
+	Pid = proc_lib:spawn_link(?MODULE, connection_process,
+		[self(), Ref, Socket, Transport, Opts]),
 	{ok, Pid}.
 
--spec proc_lib_hack(pid(), ranch:ref(), ssl:sslsocket(), module(), cowboy:opts()) -> ok.
-proc_lib_hack(Parent, Ref, Socket, Transport, Opts) ->
-	try
-		init(Parent, Ref, Socket, Transport, Opts)
-	catch
-		_:normal -> exit(normal);
-		_:shutdown -> exit(shutdown);
-		_:Reason = {shutdown, _} -> exit(Reason);
-		_:Reason -> exit({Reason, erlang:get_stacktrace()})
-	end.
-
--spec init(pid(), ranch:ref(), ssl:sslsocket(), module(), cowboy:opts()) -> ok.
-init(Parent, Ref, Socket, Transport, Opts) ->
+-spec connection_process(pid(), ranch:ref(), ssl:sslsocket(), module(), cowboy:opts()) -> ok.
+connection_process(Parent, Ref, Socket, Transport, Opts) ->
 	ok = ranch:accept_ack(Ref),
 	case ssl:negotiated_protocol(Socket) of
 		{ok, <<"h2">>} ->