From dbc02ed92ec22096b10aad5906b30d586def0259 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hubert=20=C5=81=C4=99picki?= Date: Fri, 23 Apr 2021 11:22:44 +0200 Subject: [PATCH] [1551] Handle external exits of processes Adds handler to capture external exits of processes, and tests to verify this is indeed happening. Problem: I don't know how to silence crash reports in this case. When I run the tests, I can see the crash reports I added, but I would like to silence them in tests. ct_helper:ignore() doesn't seem to cut it the way I use it, which is probably wrong. I see on the console: Testing ninenines.cowboy.metrics_SUITE: Starting test, 96 test cases =ERROR REPORT==== 23-Apr-2021::11:22:09 === Ranch listener http, connection process <0.266.0>, stream 1 had its request process <0.275.0> exit with reason external_exit which we probably want to silence but I'm not sure how. Help? --- src/cowboy_stream_h.erl | 16 ++++++ test/handlers/crash_h.erl | 8 ++- test/metrics_SUITE.erl | 117 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 140 insertions(+), 1 deletion(-) diff --git a/src/cowboy_stream_h.erl b/src/cowboy_stream_h.erl index f516f3d21..6a6405e3f 100644 --- a/src/cowboy_stream_h.erl +++ b/src/cowboy_stream_h.erl @@ -154,6 +154,22 @@ info(StreamID, Exit={'EXIT', Pid, {Reason, Stacktrace}}, State=#state{ref=Ref, p do_info(StreamID, Exit, [ {error_response, 500, #{<<"content-length">> => <<"0">>}, <<>>} |Commands], State); +info(StreamID, Exit={'EXIT', Pid, Reason}, State=#state{ref=Ref, pid=Pid}) -> + Commands0 = [{internal_error, Exit, 'Stream process crashed.'}], + Commands = case Reason of + normal -> Commands0; + shutdown -> Commands0; + {shutdown, _} -> Commands0; + _ -> [{log, error, + "Ranch listener ~p, connection process ~p, stream ~p " + "had its request process ~p exit with reason " + "~999999p~n", + [Ref, self(), StreamID, Pid, Reason]} + |Commands0] + end, + do_info(StreamID, Exit, [ + {error_response, 500, #{<<"content-length">> => <<"0">>}, <<>>} + |Commands], State); %% Request body, auto mode, no body buffered. info(StreamID, Info={read_body, Pid, Ref, auto, infinity}, State=#state{read_body_buffer= <<>>}) -> do_info(StreamID, Info, [], State#state{ diff --git a/test/handlers/crash_h.erl b/test/handlers/crash_h.erl index b687ababf..ed9b549c6 100644 --- a/test/handlers/crash_h.erl +++ b/test/handlers/crash_h.erl @@ -13,4 +13,10 @@ init(_, no_reply) -> init(Req, reply) -> _ = cowboy_req:reply(200, Req), ct_helper:ignore(?MODULE, init, 2), - error(crash). + error(crash); +init(_, internal_exit) -> + ct_helper:ignore(?MODULE, init, 2), + exit(internal_exit); +init(_, external_exit) -> + ct_helper:ignore(?MODULE, init, 2), + exit(self(), external_exit). diff --git a/test/metrics_SUITE.erl b/test/metrics_SUITE.erl index 74a259f20..5ca5c801f 100644 --- a/test/metrics_SUITE.erl +++ b/test/metrics_SUITE.erl @@ -76,6 +76,8 @@ init_routes(_) -> [ {"/", hello_h, []}, {"/crash/no_reply", crash_h, no_reply}, {"/crash/reply", crash_h, reply}, + {"/crash/internal_exit", crash_h, internal_exit}, + {"/crash/external_exit", crash_h, external_exit}, {"/default", default_h, []}, {"/full/:key", echo_h, []}, {"/resp/:key[/:arg]", resp_h, []}, @@ -542,3 +544,118 @@ error_response_after_reply(Config) -> %% All good! gun:close(ConnPid) end. + +error_response_after_internal_exit(Config) -> + doc("Confirm metrics are correct when an error_response command is returned " + "after internal exit."), + %% Perform a GET request. + ConnPid = gun_open(Config), + Ref = gun:get(ConnPid, "/crash/internal_exit", [ + {<<"accept-encoding">>, <<"gzip">>}, + {<<"x-test-pid">>, pid_to_list(self())} + ]), + {response, fin, 500, RespHeaders} = gun:await(ConnPid, Ref, infinity), + timer:sleep(100), + %% Receive the metrics and validate them. + receive + {metrics, From, Metrics} -> + %% Ensure the timestamps are in the expected order. + #{ + req_start := ReqStart, req_end := ReqEnd, + resp_start := RespStart, resp_end := RespEnd + } = Metrics, + true = (ReqStart =< RespStart) + and (RespStart =< RespEnd) + and (RespEnd =< ReqEnd), + %% We didn't send a body. + #{ + req_body_start := undefined, + req_body_end := undefined, + req_body_length := 0 + } = Metrics, + %% We got a 500 response without a body. + #{ + resp_status := 500, + resp_headers := ExpectedRespHeaders, + resp_body_length := 0 + } = Metrics, + ExpectedRespHeaders = maps:from_list(RespHeaders), + %% The request process executed normally. + #{procs := Procs} = Metrics, + [{_, #{ + spawn := ProcSpawn, + exit := ProcExit, + reason := {internal_exit, _StackTrace} + }}] = maps:to_list(Procs), + true = ProcSpawn =< ProcExit, + %% Confirm other metadata are as expected. + #{ + ref := _, + pid := From, + streamid := 1, + reason := {internal_error, {'EXIT', _Pid, {internal_exit, _StackTrace}}, 'Stream process crashed.'}, + req := #{}, + informational := [], + user_data := #{} + } = Metrics, + %% All good! + gun:close(ConnPid) + end. + +error_response_after_external_exit(Config) -> + doc("Confirm metrics are correct when an error_response command is returned " + "after external exit."), + %% Perform a GET request. + ConnPid = gun_open(Config), + Ref = gun:get(ConnPid, "/crash/external_exit", [ + {<<"accept-encoding">>, <<"gzip">>}, + {<<"x-test-pid">>, pid_to_list(self())} + ]), + {response, fin, 500, RespHeaders} = gun:await(ConnPid, Ref, infinity), + timer:sleep(100), + %% Receive the metrics and validate them. + receive + {metrics, From, Metrics} -> + %% Ensure the timestamps are in the expected order. + #{ + req_start := ReqStart, req_end := ReqEnd, + resp_start := RespStart, resp_end := RespEnd + } = Metrics, + true = (ReqStart =< RespStart) + and (RespStart =< RespEnd) + and (RespEnd =< ReqEnd), + %% We didn't send a body. + #{ + req_body_start := undefined, + req_body_end := undefined, + req_body_length := 0 + } = Metrics, + %% We got a 500 response without a body. + #{ + resp_status := 500, + resp_headers := ExpectedRespHeaders, + resp_body_length := 0 + } = Metrics, + ExpectedRespHeaders = maps:from_list(RespHeaders), + %% The request process executed normally. + #{procs := Procs} = Metrics, + [{_, #{ + spawn := ProcSpawn, + exit := ProcExit, + reason := external_exit + }}] = maps:to_list(Procs), + true = ProcSpawn =< ProcExit, + %% Confirm other metadata are as expected. + #{ + ref := _, + pid := From, + streamid := 1, + reason := {internal_error, {'EXIT', _Pid, external_exit}, 'Stream process crashed.'}, + req := #{}, + informational := [], + user_data := #{} + } = Metrics, + %% All good! + gun:close(ConnPid) + end. +