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

Should be able to force close the WebSocket #90

Open
lmcd opened this issue May 18, 2021 · 2 comments
Open

Should be able to force close the WebSocket #90

lmcd opened this issue May 18, 2021 · 2 comments
Labels
enhancement New feature or request

Comments

@lmcd
Copy link

lmcd commented May 18, 2021

Is your feature request related to a problem? Please describe.

The close method waits for acknowledgement from the server. In some situations we already know the connection is closed (e.g. if the reachability state to the server has changed), and would rather just force a disconnected state immediately.

Describe the solution you'd like

Introduce forceClose method to WebSocket, that just terminates the channel.

@lmcd lmcd added the enhancement New feature or request label May 18, 2021
@lmcd
Copy link
Author

lmcd commented May 19, 2021

Actually, I looked a bit further into this, and you're still forced to incur a 5 second penalty between closing the channel and onClose being triggered. This is a shutdown timeout for TLS connections built into NIO.

This is really frustrating, 5 seconds is an unacceptably long time for a WebSocket to be stranded in a closing state for my use case. So for example, if you set pingInterval to 1 second, you'll actually be waiting at least 6 seconds until onClose is triggered and you can attempt a connection restart.

Perhaps an onClosing future would be useful in responding to these situations?

@lmcd
Copy link
Author

lmcd commented May 19, 2021

On further thought, since it might difficult to provide a reliable onClosing that covers all possible closing scenarios, perhaps we should just deliver a onPingTimeout that covers just this specific use case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant