search

sneako / finch

Elixir HTTP client, focused on performance
MIT License
1.23k stars 114 forks source link

receive_timeout regarding stream/5 & async_request/3 #243

Closed jswanner closed 7 months ago

jswanner commented 10 months ago

Thanks for the amazing library!

Finch.stream/5 seems to treat the receive_timeout option differently between v0.16.0 & main (9609c40), and I believe it stems from 823cc65.

On main, Finch.stream/5 (and Finch.async_request/3) timeout after the receive_timeout threshold has elapsed, even if data are being received. While on v0.16.0, the connection times out after the receive_timeout threshold has elapsed since the last chunk of data (rather than from the start of the connection).

Maybe I'm wanting to do something that's not intended to be supported, which is to leave the streaming connection open as long as it's still receiving data (the API I'm calling should send pings every 5 seconds). I can work around the problem by setting a very high receive_timeout value and then reconnect on timeout, but it would be nice to somehow have the receive_timeout behavior of v0.16.0.

sneako commented 10 months ago

Thank you for the kind words!

I see your point.

I guess, the value that is passed to the recv calls should always be the :receive_timeout and something similar to the new behaviour that was introduced could be implemented as a new :request_timeout option for the overall duration of request.

jswanner commented 10 months ago

I guess, the value that is passed to the recv calls should always be the :receive_timeout and something similar to the new behaviour that was introduced could be implemented as a new :request_timeout option for the overall duration of request.

@sneako, what you proposed sounds good to me. I thought I understood what needed to happen well enough to implement it myself, but turns out I don't know what I'm doing. Mint seems to be pipelining requests, and the request ref is unexpectedly changing.

Would you like me to push a broken PR? Maybe you'll have some insight into what I'm doing wrong.

zachallaun commented 10 months ago

Would you like me to push a broken PR? Maybe you'll have some insight into what I'm doing wrong.

I'm not @sneako, but it seems totally reasonable to push it as a Draft PR!

jswanner commented 10 months ago

Thanks for the prompt @zachallaun, I had been intending to make the PR, but had been focusing on other things.

sneako commented 7 months ago

Closed via #244

jswanner commented 7 months ago

Thank you, @sneako! ❤️