search

stomp-js / stomp-websocket

Stomp client for Web browsers and node.js apps
https://stomp-js.github.io/stomp-websocket/
Apache License 2.0
141 stars 36 forks source link

Disconnect throws an Exception when client not connected #21

Closed aalbericio closed 6 years ago

aalbericio commented 6 years ago

I'm trying to "dispose" all Stomp resources in my application at logout time.

If my Stomp.Client did not make it to connect to the server, I also want to ensure that a possible reconnection mechanism is also disposed and stopped.

What happens it that, client.disconnect() throws an Exception when the client is not connected (client.connected is false)

Workaround: I've successfully disposed the reconnection mechanism by doing:

this.client.ws = null; this.client = null;

Should the library provide this feature/method natively?

kum-deepak commented 6 years ago

I think I understand the issue. The disconnect code currently does quite a few cleanups. It however does not handle the case if the client was not connected at that time. I need to review the code carefully to fix the issue.

aalbericio commented 6 years ago

I believe the disconnect method is doing lots of things, which should be correct to dispose the client, but it's also attempting to send (don't know in which context) something that it shouldn't.

That send operation is causing issues. You should review and secure that important process of disposing the client.

Let me know when you want me to test it again cause I'm quite busy right now with a release.

Thanks so much for your work!

kum-deepak commented 6 years ago

When the client is actually connected, as per standard it needs to send a DISCONNECT frame to the server, so that the server can cleanup and send a RECEIPT. Only then as per standards, client is supposed to consider the the disconnection completed.

While that case works as expected, the disconnect function does not consider the case that the server may actually not be connected.

kum-deepak commented 6 years ago

Committed branch disconnect, it has cleaned up code for handling disconnect. Please see if this resolves your issue.

kum-deepak commented 6 years ago

Please check if the current code resolves your issue. Accordingly I will close the issue.

kum-deepak commented 6 years ago

Planning for next version of stompjs is on the way. Your participation will be really appreciated, please join at https://github.com/stomp-js/stompjs/issues/1