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

Suppress Read() error on closing websocket-proxy connection #22

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rbott
Copy link
Member

@rbott rbott commented Oct 7, 2020

When one end of the websocket<->socket proxy closes the connection (e.g. due to a browser page reload) a net.OpError 'use of closed network connection' error will be triggered in the blocking Read()/ReadMessage() call of the other direction.

This is a rather hacky attempt to detect this situation and not trigger a log.Warning(). TCPConn (as opposed to Conn returned by net.Dial()) comes with aCloseRead() method which gracefully terminates all open reads while closing a connection. We could therefore replace net.Dial() with net.TCPDial(). However, that only helps in one direction, as the other one builds upon the websocket handler and there is no such thing as CloseRead() there.

When one end of the websocket proxy closes the connection (e.g.
a browser reload) a net.OpError 'use of closed network connection'
error will be triggered in the blocking Read()/ReadMessage() call
of the other direction. This is a rather hacky attempt to detect
this situation and not trigger a log.Warning().

Signed-off-by: Rudolph Bott <[email protected]>
@paulschwoerer
Copy link
Collaborator

I might make sense to use https://github.com/nhooyr/websocket, which supports graceful connection closing

@geigi
Copy link
Collaborator

geigi commented Nov 25, 2022

Golang now has an official websocket package: https://pkg.go.dev/golang.org/x/net/websocket#example-Handler

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 this pull request may close these issues.

3 participants