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

document http client must use context timeout and not timeout on struct #341

Closed
yihexi opened this issue Jun 2, 2022 · 8 comments
Closed
Labels
Milestone

Comments

@yihexi
Copy link

yihexi commented Jun 2, 2022

when I set http.DefaultClient.Timeout = time.Second * 10, websocket.Dial will get an error "failed to WebSocket dial: response body is not a io.ReadWriteCloser: *http.cancelTimerBody"

verson: nhooyr.io/websocket v1.8.7

this is because in the dial.go line 128: rwc, ok := respBody.(io.ReadWriteCloser), it convert respBody to an io.ReadWriteCloser, I think it should be rwc, ok := respBody.(io.ReadCloser).

in general, respBody is a *http.readWriteCloserBody, but if http.DefaultClient.Timeout is set, it will be a *http.cancelTimerBody, which is just an io.ReadCloser.

@nhooyr
Copy link
Owner

nhooyr commented Mar 5, 2023

Ah right, I'll document this.

@nhooyr nhooyr changed the title Dial got error "failed to WebSocket dial: response body is not a io.ReadWriteCloser: *http.cancelTimerBody" when set http.DefaultClient.Timeout document http client must use context timeout and not timeout on struct Mar 7, 2023
@mateusfmello
Copy link

mateusfmello commented Sep 12, 2023

@nhooyr

Was this implemented in the project?

I'm having the same problem here, but i use context.WithCancel(context.Background())

@nhooyr
Copy link
Owner

nhooyr commented Sep 12, 2023

Nothing to implement, just an unfortunate quirk of *http.Client. Please use the context to pass in a timeout.

@mateusfmello
Copy link

Looking at the readme example, I switched to context.WithTimeout, but the error continues

@nhooyr
Copy link
Owner

nhooyr commented Sep 12, 2023

Ah right yea so I'll change the code here to remove the Timeout from *http.Client but right now you have to make sure Timeout is 0 on whatever *http.Client you use with websocket.Dial.

@mateusfmello
Copy link

mateusfmello commented Sep 13, 2023

I believe that nothing needs to be done, as context.WithCancel(context.Background()) works perfectly too, I believe the problem in my code is something else, I'm rewriting it.

The bug occurred after closing the connection and reopening.

Thank you for creating this library and being active in the project

@nhooyr nhooyr added the bug label Sep 28, 2023
@nhooyr nhooyr added this to the v1.8.8 milestone Sep 28, 2023
@nhooyr
Copy link
Owner

nhooyr commented Sep 28, 2023

Oh interesting, I'll definitely test to make sure.

nhooyr added a commit that referenced this issue Oct 13, 2023
@nhooyr
Copy link
Owner

nhooyr commented Oct 13, 2023

Fixed in dev.

@nhooyr nhooyr closed this as completed Oct 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants