cohttp-eio: Don't blow up in Server.callback on client disconnections.
#1015
Conversation
There was a problem hiding this comment.
Looks fine to me.
It might be nice to add a test for connection reset errors. The old cohttp-eio had some tests where you could inject errors (though it only tried with End_of_file), but it looks like they got removed in the rewrite: https://github.com/mirage/ocaml-cohttp/blob/b7182a17e9769c7d08c5b05e2512e0eca605f74e/cohttp-eio/tests/server.md#tests
|
@mefyl do you think you have time to add some tests or we should better merge and open an issue about that? |
Not right now unfortunately, I might be able to look into this in a week or two. I think we should merge this in the meantime as it is a fairly crippling bug on busy servers. |
|
Thanks |
CHANGES: - bump minimum dune version to 3.8 (@avsm) - cohttp-eio: Use system authenticator in example. - http, cohttp: remove the scheme field from requests. This means that [Request.uri] no longer returns the same URI as was to create the request with [Request.make] (@rgrinberg 1086) - cohttp-eio: Remove unused `Client_intf` module (talex5 mirage/ocaml-cohttp#1081) - cohttp-eio: Make server response type abstract and allow streaming in cohttp-eio (talex5 mirage/ocaml-cohttp#1024) - cohttp-{lwt,eio}: server: add connection header to response if not present (ushitora-anqou mirage/ocaml-cohttp#1025) - cohttp-curl: Curl no longer prepends the first HTTP request header to the output. (jonahbeckford mirage/ocaml-cohttp#1030, mirage/ocaml-cohttp#987) - cohttp-eio: client: use permissive argument type for make_generic - cohttp-eio: Improve error handling in example server (talex5 mirage/ocaml-cohttp#1023) - cohttp-eio: Don't blow up `Server.callback` on client disconnections. (mefyl mirage/ocaml-cohttp#1015) - http: Fix assertion in `Source.to_string_trim` when `pos <> 0` (mefyl mirage/ocaml-cohttp#1017) - cohttp: `Cohttp.Request.make_for_client` no longer allows setting both `~chunked:true` and `~body_length`. - cohttp-lwt-unix: Don't blow up when certificates are not available and no-network requests are made. (akuhlens mirage/ocaml-cohttp#1027) + Makes `cohttp-lwt.S.default_ctx` lazy.
My previous fix on this fixed the issue for
Server.run, but notServer.callback.