Address ECONNRESET in issue #118

dune-project

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

src/dream.mli

@@ -2120,6 +2120,8 @@ val run :
     - [~interface] is the network interface to listen on. Defaults to
       ["localhost"]. Use ["0.0.0.0"] to listen on all interfaces.
     - [~port] is the port to listen on. Defaults to [8080].
+    - If [~socket_path] is specified, Dream will listen on a Unix domain socket
+      at the given path, and ignore [~interface] and [~port].
     - [~stop] is a promise that causes the server to stop accepting new
       requests, and {!Dream.run} to return. Requests that have already entered
       the Web application continue to be processed. The default value is a

src/http/error_handler.ml

@@ -120,40 +120,48 @@ let customize template (error : Catch.error) =
 
   (* First, log the error. *)
 
+  let log_handler condition (error : Catch.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 |> Log.iter_backtrace (fun line ->
+       select_log error.severity (fun log ->
+         log ?request:error.request "%s" line))
+  in
+
   begin match error.condition with
   | `Response _ -> ()
+  (* 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 ->
-
-    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 |> Log.iter_backtrace (fun line ->
-      select_log error.severity (fun log ->
-        log ?request:error.request "%s" line))
+    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