Open NoahAndrews opened 6 years ago
Actually, a better solution would be to throw the IOException after putting the WebSocket back in its previous state (a forceClose parameter could be added to override this behavior and finish shutting down the websocket)
Actually, the best approach would be to understand why the server would fail to send a close frame and only then think of the correct way of dealing with each scenario.
Tell me, would you be interested in helping rewrite the entire library from the ground up? Patching on top of patches is kinda not cutting it anymore, especially if you take into account how half assed the initial implementations were...
I'm afraid that's not likely something I'm going to be able to help you with.
Well if things do go that way, I intend to do most of the hard work anyways, but having people like you pointing out inconsistencies does help a lot.
Either way, thank you for your contributions so far.
Back to business, since you've been looking into it, how and why do you think it fails to send close frames?
You're welcome, I hope I'll end up being of further help in the future (I've already got an actual fix for #328 that I need to submit). I'll try to keep track of the code being submitted here going forward.
I've never actually seen it fail to send a close frame. I was just implementing some code that would close all of the open sockets, and of course I had to handle the IOException. Naturally, I went back to look at the code that would throw that exception, and I noticed that if it did get thrown for whatever reason, you'd end up in an incorrect state. I imagine that you could trigger this by disconnecting from the network at just the right split second. So it's not about an underlying issue that needs fixing, just about keeping things sane if an error were to happen.
Note: This is all based on reading the code for the latest 2.x release, not observation. I could be wrong. I did a quick check of the master branch, and it looks like it works the same way.
Right now, if
close()
is called on aWebSocket
instance, and the close frame fails to send, then anIOException
is thrown, and theWebSocket
is left in theCLOSING
state. If you were to then callclose()
again, it would finish closing down the socket, without even trying to re-send.It seems like the correct behavior in this instance would be to catch the
IOException
, attempt to resend the close frame, and then immediately calldoClose()
.Maybe my specific idea isn't the best, but I think the code as it exists needs a little reworking.