Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add stream idle timeout #317

Open
bjosv opened this issue Aug 15, 2023 · 2 comments · May be fixed by #330
Open

Add stream idle timeout #317

bjosv opened this issue Aug 15, 2023 · 2 comments · May be fixed by #330

Comments

@bjosv
Copy link
Contributor

bjosv commented Aug 15, 2023

When we ran the client towards a http2 server that misbehaved we saw that we got an ever increasing map of unfinished streams (http2_state.streams, but also http2_state.stream_refs). We noticed that the server did not always respond to our requests, even when using a single TLS connection.

I have seen that some clients for other languages [1] provides a configuration, a stream idle timeout, which would terminate a stream after not receiving any data or response within configured time.

Would it make sense to have a feature like this in Gun or is it up to the user to make sure that non-finished streams are cancelled/terminated using own timeouts? What is your view on this?

Could it make sense to add some guidance in
https://ninenines.eu/docs/en/gun/2.0/guide/http/#_processing_responses

that gun:cancel/2 could be used to terminate a stream that has not yet received any response or a message with the atom fin.
WDYT?

[1]
https://nodejs.org/dist//v9.0.0/docs/api/http2.html#http2_http2stream_settimeout_msecs_callback
https://eclipse.dev/jetty/javadoc/jetty-12/org/eclipse/jetty/http2/client/HTTP2Client.html#setStreamIdleTimeout(long)

@essen
Copy link
Member

essen commented Aug 16, 2023

I think Gun cannot know whether a stream is long polling or not. I therefore would not favor a feature where Gun ends streams based on a single per-connection option. On the other hand, a per-request (ReqOpts) option sounds fine, especially since you can tweak the value depending on the request (if some responses take longer to be generated than others). Gun will have to send a gun_error with an adequate error reason to identify why the request was terminated.

The guide can of course be updated but do that after sending a PR for the timeout feature preferrably.

@zuiderkwast
Copy link
Contributor

I can implement this, if you're willing to review and merge it.

Maybe first you can merge or comment on my other PRs that have been waiting for several months: #280 and #312.

@zuiderkwast zuiderkwast linked a pull request Jan 29, 2024 that will close this issue
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants