search

elixir-mint / mint

Functional HTTP client for Elixir with support for HTTP/1 and HTTP/2 🌱
Apache License 2.0
1.36k stars 112 forks source link

Fix connection closed errors not propagating, closes #279 #280

Closed josevalim closed 4 years ago

josevalim commented 4 years ago

There is one failing test but I believe the test is wrong. We are transferring the test to another process and then trying to stream its response, which fails when trying to set it back to active mode. I want to change the test but I would like confirmation before.

josevalim commented 4 years ago

@ericmj helped me fixed the test. I have also added a test for the functionality that we need.

sneako commented 4 years ago

FWIW: I deployed this change to an application which uses Finch and makes a ton of http1 requests (no http2). I am not observing any new errors or regressions and the rate of connection closed errors that we have been trying to track down here is practically the same as before.

josevalim commented 4 years ago

@sneako are you sure the server does not support http2? Because due to https://github.com/keathley/finch/pull/87, we may be accidentally using http2 and that could have been the root cause. :)

sneako commented 4 years ago

@josevalim We have a dozen pools, and one pool in particular emits almost all of the connection closed errors, however I just took a packet capture to confirm that we are in fact using HTTP/1.1 for this host.

keathley commented 4 years ago

Tested locally, and this PR resolves the issues in the Finch tests that I was seeing.