-
Notifications
You must be signed in to change notification settings - Fork 296
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
Context expiration on Read
shuts down WebSocket
#242
Comments
100% agree this is unfortunate and definitely not a design decision but the API exposed to us by
I agree but there isn't a good way yet to bound a context to a IO operation so unfortunately this is the best that can be done.
Yes, it's unfortunate but that is your best shot at the moment, shouldn't be much boilerplate. In my experience such patterns are uncommon compared to just closing the connection if there hasn't been a message sent in X amount of time. If you really need that, I'd go with gorilla/websocket or gobwas/ws, I know gobwas/ws is particularly optimized for this. I'll document the decisions here better for future users. |
* fix handling of error in nhooyr websocket wrapper (mentioned in andersfylling#298) * closing context given to g.c.Read(ctx) leads to WebSocket being closed coder/websocket#242
webSocketConnection Read and Write had separate timeouts which, when expired, mutually closed the websocket. In case only one party of the connection was sending, the websocket was closed after the timeout despite the fact that the connection was alive. This comes from misunderstanding the impact of the Read/Write context parameter. See coder/websocket#242
If the context I pass to
(*Conn).Read
expires, this shuts down the entire websocket.conn_notjs:160
is at fault here.This is confusing and severely constraints the ways in which this library can be used. For example, the following is not possible:
It is also extremely un-idiomatic since contexts communicate expectations/bounds by the caller only, and the effects of their expiration should be local to the call.
The only way to work around this would be to move all reads in a goroutine and communicate the messages read back via a channel, which is.. meh.
The text was updated successfully, but these errors were encountered: