xmppjs / xmpp.js

XMPP for JavaScript
ISC License
2.18k stars 371 forks source link

Calling 'stop()' sometimes leads to the client being 'disconnected' #956

Open CamilleDrapier opened 1 year ago

CamilleDrapier commented 1 year ago

Describe the bug

When calling stop(), we expect the client to end up in the 'offline' status, but sometimes (somewhat extreme network environment?) the 'offline' event is triggered before the 'disconnected' one, which will then keep the 'disconnected' until other actions are triggered. If the reconnect package/plugin is used, it will identify this state as necessitating re-connection and will try to perform a re-connection (which will usually not succeed)

Logs

LogCollectBrowserConsole.js:54 status closing 
LogCollectBrowserConsole.js:54 IN
<presence xmlns="jabber:client" type="unavailable" from="b5gse3ps9g8ibyriggbz0mqy0@XXX/ahim9ls5lrj0v5e3g8rc8mc69_c1537340-1df8-11ed-93ad-2d00ab542b96" to="b5gse3ps9g8ibyriggbz0mqy0@XXX/ahim9ls5lrj0v5e3g8rc8mc69_c52a44d0-1df8-11ed-ba8c-e9862a556d50"/>
LogCollectBrowserConsole.js:54 IN
<presence xmlns="jabber:client" from="b5gse3ps9g8ibyriggbz0mqy0@XXX/ahim9ls5lrj0v5e3g8rc8mc69_c52a44d0-1df8-11ed-ba8c-e9862a556d50" to="b5gse3ps9g8ibyriggbz0mqy0@XXX/ahim9ls5lrj0v5e3g8rc8mc69_c52a44d0-1df8-11ed-ba8c-e9862a556d50"/>
LogCollectBrowserConsole.js:54 status disconnecting 
LogCollectBrowserConsole.js:54 status offline 
LogCollectBrowserConsole.js:54 status disconnect [object Object]
LogCollectBrowserConsole.js:54 status connecting wss://XXX/ws/
LogCollectBrowserConsole.js:54 status connect 
LogCollectBrowserConsole.js:54 status opening 
LogCollectBrowserConsole.js:54 status open <open from="XXX" id="a6wknjnx16" xmlns="urn:ietf:params:xml:ns:xmpp-framing" xml:lang="en" version="1.0"/>

Environment

This is obtained by running our application through some integration testing (cypress 10.3.1 + Chrome 104.0.5112.79, observed on both CI (ubuntu), Mac, and Linux (Arch) computers)

Possible Solutions and workaround

One possible solution would be to not mark the 'disconnected' state if we are currently in offline state, maybe on the connection package we could do something such as:

      if (this.status !== "offline") this._status("disconnect", { clean: !dirty, event });

Otherwise for now, as a workaround, we will disable the reconnect mechanism after explicitly calling the client close method to avoid the client trying to reconnect (but it will still be left in an unexpected state/status):

      await xmppClient?.stop();
      xmppClient?.reconnect.stop();
sonnyp commented 1 year ago

Would be nice to have a way to reproduce. Which server / version? What is the code that reproduce the problem ?

Anyhow - could you check if the following fixes the problem: https://github.com/xmppjs/xmpp.js/pull/967

sonnyp commented 1 year ago

after explicitly calling the client close method

Did you mean the stop method?

CamilleDrapier commented 1 year ago

Thank you for your reply!

I do not think I can provide a way to reproduce, as this happens only sometimes with our quite complex setup that I cannot share publicly, sorry. I noticed this problem with the 0.13.1 client at least. I'm not sure about the server though!

Anyhow - could you check if the following fixes the problem: https://github.com/xmppjs/xmpp.js/pull/967

Thanks! I think the changes looks nice, but I do not think it would solve the problem I described:

Of course, I'm not very proficient with the code base here, so I might have missed some side effect of your proposed changes that would solve the problem :bow:

Did you mean the stop method?

Yes, sorry for the confusion!