Skip to content

Error should not be returned on non-zero http client timeout #260

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

Closed
swithek opened this issue Sep 19, 2020 · 2 comments · Fixed by #261
Closed

Error should not be returned on non-zero http client timeout #260

swithek opened this issue Sep 19, 2020 · 2 comments · Fixed by #261
Milestone

Comments

@swithek
Copy link
Contributor

swithek commented Sep 19, 2020

Having a warning about non-zero http client timeout is useful, but it should be a documentation-level warning not an actual error. With the current implementation it's not possible to use the same http client for both standard http requests and websocket events, even when a context with timeout is provided during dialing.

Related to: #67

@nhooyr
Copy link
Contributor

nhooyr commented Sep 19, 2020

Hi @swithek

I guess we can copy the http client, remove the timeout and use that instead.

Would that work for your use case?

The reason it errors out is that a timeout on the client causes response bodies to not be writable, and so we can't use it for the WebSocket protocol.

@swithek
Copy link
Contributor Author

swithek commented Sep 20, 2020

Thank you for your swift response.

Yes, copying the client would work. Would this be an internal change or should this be done before passing it as an option? Having it as an internal feature would be a bit more convenient when working on multiple projects.

@nhooyr nhooyr added this to the v1.8.7 milestone Oct 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants