aboutsummaryrefslogtreecommitdiffstats
path: root/src
diff options
context:
space:
mode:
authorLoïc Hoguin <[email protected]>2017-09-27 14:17:27 +0200
committerLoïc Hoguin <[email protected]>2017-09-27 14:17:27 +0200
commit11ae407eed92002339fc6cde8acd767e7be953c1 (patch)
treee375c9fa324cd4b9fa988e9a83978064374be54c /src
parentd47e22edaa1a876081c07bf49c79587c3c2d21d5 (diff)
downloadcowboy-11ae407eed92002339fc6cde8acd767e7be953c1.tar.gz
cowboy-11ae407eed92002339fc6cde8acd767e7be953c1.tar.bz2
cowboy-11ae407eed92002339fc6cde8acd767e7be953c1.zip
Ensure the behavior on stream handler crash is consistent
Also corrects the lack of error response when HTTP/1.1 is used.
Diffstat (limited to 'src')
-rw-r--r--src/cowboy_http.erl40
-rw-r--r--src/cowboy_http2.erl12
2 files changed, 29 insertions, 23 deletions
diff --git a/src/cowboy_http.erl b/src/cowboy_http.erl
index a7c9b4d..0be949e 100644
--- a/src/cowboy_http.erl
+++ b/src/cowboy_http.erl
@@ -252,10 +252,9 @@ after_parse({request, Req=#{streamid := StreamID, headers := Headers, version :=
cowboy_stream:report_error(init,
[StreamID, Req, Opts],
Class, Exception, erlang:get_stacktrace()),
- ok %% @todo send a proper response, etc. note that terminate must NOT be called
- %% @todo Status code.
-% stream_reset(State, StreamID, {internal_error, {Class, Reason},
-% 'Exception occurred in StreamHandler:init/10 call.'}) %% @todo Check final arity.
+ early_error(500, State0, {internal_error, {Class, Exception},
+ 'Unhandled exception in cowboy_stream:init/3.'}, Req),
+ parse(Buffer, State0)
end;
%% Streams are sequential so the body is always about the last stream created
%% unless that stream has terminated.
@@ -270,11 +269,8 @@ after_parse({data, StreamID, IsFin, Data, State=#state{
cowboy_stream:report_error(data,
[StreamID, IsFin, Data, StreamState0],
Class, Exception, erlang:get_stacktrace()),
- %% @todo Bad value returned here. Crashes.
- ok
- %% @todo
-% stream_reset(State, StreamID, {internal_error, {Class, Reason},
-% 'Exception occurred in StreamHandler:data/4 call.'})
+ stream_reset(State, StreamID, {internal_error, {Class, Exception},
+ 'Unhandled exception in cowboy_stream:data/4.'})
end;
%% No corresponding stream, skip.
after_parse({data, _, _, _, State, Buffer}) ->
@@ -733,10 +729,8 @@ info(State=#state{streams=Streams0}, StreamID, Msg) ->
cowboy_stream:report_error(info,
[StreamID, Msg, StreamState0],
Class, Exception, erlang:get_stacktrace()),
- ok
-%% @todo
-% stream_reset(State, StreamID, {internal_error, {Class, Reason},
-% 'Exception occurred in StreamHandler:info/3 call.'})
+ stream_reset(State, StreamID, {internal_error, {Class, Exception},
+ 'Unhandled exception in cowboy_stream:info/3.'})
end;
false ->
error_logger:error_msg("Received message ~p for unknown stream ~p.~n", [Msg, StreamID]),
@@ -935,6 +929,8 @@ stream_terminate(State0=#state{socket=Socket, transport=Transport,
{value, #stream{state=StreamState, version=Version}, Streams}
= lists:keytake(StreamID, #stream.id, Streams0),
State1 = case OutState of
+ wait when element(1, Reason) =:= internal_error ->
+ info(State0, StreamID, {response, 500, #{<<"content-length">> => <<"0">>}, <<>>});
wait ->
info(State0, StreamID, {response, 204, #{}, <<>>});
chunked when Version =:= 'HTTP/1.1' ->
@@ -1020,8 +1016,7 @@ connection_hd_is_close(Conn) ->
%% This function is only called when an error occurs on a new stream.
-spec error_terminate(cowboy:http_status(), #state{}, _) -> no_return().
-error_terminate(StatusCode0, State=#state{ref=Ref, socket=Socket, transport=Transport,
- opts=Opts, peer=Peer, in_streamid=StreamID, in_state=StreamState}, Reason) ->
+error_terminate(StatusCode, State=#state{ref=Ref, peer=Peer, in_state=StreamState}, Reason) ->
PartialReq = case StreamState of
#ps_request_line{} ->
#{};
@@ -1039,7 +1034,12 @@ error_terminate(StatusCode0, State=#state{ref=Ref, socket=Socket, transport=Tran
end
}
end,
- Resp = {response, StatusCode0, #{<<"content-length">> => <<"0">>}, <<>>},
+ early_error(StatusCode, State, Reason, PartialReq),
+ terminate(State, Reason).
+
+early_error(StatusCode0, #state{socket=Socket, transport=Transport,
+ opts=Opts, in_streamid=StreamID}, Reason, PartialReq) ->
+ Resp = {response, StatusCode0, RespHeaders0=#{<<"content-length">> => <<"0">>}, <<>>},
try cowboy_stream:early_error(StreamID, Reason, PartialReq, Resp, Opts) of
{response, StatusCode, RespHeaders, RespBody} ->
Transport:send(Socket, [
@@ -1049,9 +1049,13 @@ error_terminate(StatusCode0, State=#state{ref=Ref, socket=Socket, transport=Tran
catch Class:Exception ->
cowboy_stream:report_error(early_error,
[StreamID, Reason, PartialReq, Resp, Opts],
- Class, Exception, erlang:get_stacktrace())
+ Class, Exception, erlang:get_stacktrace()),
+ %% We still need to send an error response, so send what we initially
+ %% wanted to send. It's better than nothing.
+ Transport:send(Socket, cow_http:response(StatusCode0,
+ 'HTTP/1.1', maps:to_list(RespHeaders0)))
end,
- terminate(State, Reason).
+ ok.
-spec terminate(_, _) -> no_return().
terminate(undefined, Reason) ->
diff --git a/src/cowboy_http2.erl b/src/cowboy_http2.erl
index 8c6a0d3..dbd67cb 100644
--- a/src/cowboy_http2.erl
+++ b/src/cowboy_http2.erl
@@ -305,7 +305,7 @@ frame(State0=#state{remote_window=ConnWindow, streams=Streams},
[StreamID, IsFin, Data, StreamState0],
Class, Exception, erlang:get_stacktrace()),
stream_reset(State, StreamID, {internal_error, {Class, Exception},
- 'Exception occurred in cowboy_stream:data/4.'})
+ 'Unhandled exception in cowboy_stream:data/4.'})
end;
#stream{remote=fin} ->
stream_reset(State, StreamID, {stream_error, stream_closed,
@@ -444,7 +444,7 @@ info(State=#state{streams=Streams}, StreamID, Msg) ->
[StreamID, Msg, StreamState0],
Class, Exception, erlang:get_stacktrace()),
stream_reset(State, StreamID, {internal_error, {Class, Exception},
- 'Exception occurred in cowboy_stream:info/3.'})
+ 'Unhandled exception in cowboy_stream:info/3.'})
end;
false ->
error_logger:error_msg("Received message ~p for unknown stream ~p.", [Msg, StreamID]),
@@ -759,7 +759,7 @@ stream_handler_init(State=#state{opts=Opts,
[StreamID, Req, Opts],
Class, Exception, erlang:get_stacktrace()),
stream_reset(State, StreamID, {internal_error, {Class, Exception},
- 'Exception occurred in cowboy_stream:init/3.'})
+ 'Unhandled exception in cowboy_stream:init/3.'})
end.
%% @todo We might need to keep track of which stream has been reset so we don't send lots of them.
@@ -801,9 +801,11 @@ stream_terminate(State=#state{socket=Socket, transport=Transport,
stream_call_terminate(StreamID, Reason, StreamState),
Children = cowboy_children:shutdown(Children0, StreamID),
State#state{streams=Streams, children=Children};
+ %% The stream doesn't exist. This can occur for various reasons.
+ %% It can happen before the stream has been created, or because
+ %% the cowboy_stream:init call failed, in which case doing nothing
+ %% is correct.
false ->
- %% @todo Unknown stream. Not sure what to do here. Check again once all
- %% terminate calls have been written.
State
end.