Address ECONNRESET in issue #118

dream.opam

@@ -55,6 +55,7 @@ depends: [
   "conf-libev" {os != "win32"}
   "cstruct" {>= "6.0.0"}
   "dune" {>= "2.7.0"}  # --instrument-with.
+  "emile"
   "fmt" {>= "0.8.7"}  # `Italic.
   "graphql_parser"
   "graphql-lwt"
@@ -64,9 +65,12 @@ depends: [
   "lwt_ssl"
   "logs" {>= "0.5.0"}
   "magic-mime"
+  "mimic"
   "mirage-clock"
   "mirage-crypto" {>= "0.8.1"}  # AES-256-GCM.
   "mirage-crypto-rng" {>= "0.8.0"}  # Signature of initialize.
+  "mirage-time"
+  "mirage-stack"
   "multipart_form" {>= "0.3.0"}
   "ocaml" {>= "4.08.0"}
   "ssl" {>= "0.5.8"}  # Ssl.get_negotiated_alpn_protocol.

dune-project

@@ -1 +1,2 @@
 (lang dune 2.7)
+(cram enable)

src/http/error_handler.ml

@@ -117,40 +117,46 @@ let customize template (error : Dream.error) =
 
   (* First, log the error. *)
 
+  let log_handler condition (error : Dream.error)  =
+     let client =
+       match error.client with
+       | None -> ""
+       | Some client ->  " (" ^ client ^ ")"
+     in
+
+     let layer =
+       match error.layer with
+       | `TLS -> ["TLS" ^ client]
+       | `HTTP -> ["HTTP" ^ client]
+       | `HTTP2 -> ["HTTP/2" ^ client]
+       | `WebSocket -> ["WebSocket" ^ client]
+       | `App -> []
+     in
+
+     let description, backtrace =
+       match condition with
+       | `String string -> string, ""
+       | `Exn exn ->
+         let backtrace = Printexc.get_backtrace () in
+         Printexc.to_string exn, backtrace
+     in
+
+     let message = String.concat ": " (layer @ [description]) in
+
+     select_log error.severity (fun log ->
+       log ?request:error.request "%s" message);
+     backtrace |> Dream__middleware.Log.iter_backtrace (fun line ->
+       select_log error.severity (fun log ->
+         log ?request:error.request "%s" line))
+  in
   begin match error.condition with
   | `Response _ -> ()
-  | `String _ | `Exn _ as condition ->
-
-    let client =
-      match error.client with
-      | None -> ""
-      | Some client ->  " (" ^ client ^ ")"
-    in
-
-    let layer =
-      match error.layer with
-      | `TLS -> ["TLS" ^ client]
-      | `HTTP -> ["HTTP" ^ client]
-      | `HTTP2 -> ["HTTP/2" ^ client]
-      | `WebSocket -> ["WebSocket" ^ client]
-      | `App -> []
-    in
-
-    let description, backtrace =
-      match condition with
-      | `String string -> string, ""
-      | `Exn exn ->
-        let backtrace = Printexc.get_backtrace () in
-        Printexc.to_string exn, backtrace
-    in
-
-    let message = String.concat ": " (layer @ [description]) in
-
-    select_log error.severity (fun log ->
-      log ?request:error.request "%s" message);
-    backtrace |> Dream__middleware.Log.iter_backtrace (fun line ->
-      select_log error.severity (fun log ->
-        log ?request:error.request "%s" line))
+  (* TODO is this the right place to handle lower level connection resets? *)
+  | `Exn (Unix.Unix_error (Unix.ECONNRESET, _, _)) ->
+     let condition = `String "Connection Reset at Client" in
+     let severity = `Info in
+     log_handler condition {error with condition; severity}
+  | `String _ | `Exn _ as condition -> log_handler condition error
   end;
 
   (* If Dream will not send a response for this error, we are done after

test/cram/dune

@@ -0,0 +1,2 @@
+(cram
+ (applies_to :whole_subtree))

test/cram/http/dune

@@ -0,0 +1,6 @@
+(executable
+  (name econnreset)
+  (libraries dream))
+
+(cram
+  (deps %{exe:econnreset.exe}))

test/cram/http/econnreset.ml

@@ -0,0 +1,49 @@
+(* This file is part of Dream, released under the MIT license. See LICENSE.md
+   for details, or visit https://github.com/aantron/dream.
+
+   Copyright 2021 Anton Bachin *)
+
+let serve port =
+  print_endline "server mode";
+  Dream.run ~greeting:false ~port (fun _ -> Unix.sleepf 10.0; Dream.html "Hello")
+
+let client port =
+  print_endline "client mode";
+  let open Unix in
+  let fd = socket PF_INET6 SOCK_STREAM 0 in
+  (* force the client to send a TCP RST packet if it fails during connection *)
+  setsockopt_optint fd SO_LINGER (Some 0);
+  let _ = connect fd (ADDR_INET (inet6_addr_loopback, port)) in
+  ignore @@ failwith "sending RST"
+
+let print_open_port () =
+  let open Unix in
+  let fd = socket PF_INET6 SOCK_STREAM 0 in
+  bind fd (ADDR_INET (inet6_addr_loopback, 0));
+
+  begin match getsockname fd with
+  | ADDR_INET (_, port) -> Printf.printf "%d\n" port
+  | _ -> failwith "Invalid Socket response"
+  end;
+
+  exit 0
+
+let () =
+  let server = ref(false) in
+  let port = ref(-1) in
+  let usage = "Test for ECONNRESET errors being reported" in
+  Arg.parse [
+    "-p", Set_int port, "sets the port to listen on or connect to, if not specified, prints an available TCP port and exits";
+    "-s", Set server, "enables the server on port [port], if not set sends a TCP RST on [port]"
+  ] (fun _ -> ()) usage;
+
+  let port = !port in
+
+  (* see if we need to print an open port or validate the port *)
+  if port = -1 then print_open_port ()
+  else if port > 65535 || port < 1025
+  then failwith "Port argument (-p) must set and be between 1025-65535";
+
+  if !server then serve port
+  else client port
+

test/cram/http/issue_118_econnreset.t

@@ -0,0 +1,19 @@
+Start a Dream server
+
+  $ export PORT=$(./econnreset.exe)
+  $ ./econnreset.exe -s -p "${PORT}" &> test.log &
+  $ export PID=$!
+  $ sleep 1
+
+Force a connection reset - will log a few errors
+
+  $ ./econnreset.exe -p "${PORT}"
+
+Does the log contain an error line for the ECONNRESET? An error code of [1] is "good", meaning no line was found.
+
+  $ kill "${PID}"
+  $ cat test.log | grep 'ERROR' | grep 'ECONNRESET'
+
+Does the log contain an info line with custom string for the ECONNRESET?
+
+  $ cat test.log | grep 'INFO' | grep 'Connection Reset at Client' | wc -l