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

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

Open
CamilleDrapier opened this issue Aug 17, 2022 · 3 comments
Open

Comments

@CamilleDrapier
Copy link

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
Copy link
Member

sonnyp commented Dec 1, 2022

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: #967

@sonnyp
Copy link
Member

sonnyp commented Dec 1, 2022

after explicitly calling the client close method

Did you mean the stop method?

@CamilleDrapier
Copy link
Author

CamilleDrapier commented Dec 14, 2022

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: #967

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

  • I do not think it would prevent for a closing event to be handled before a disconnecting event.
  • In the case that a closing get processed before a disconnecting, if would still end up in a diconnected state wouldn't it?

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 🙇

Did you mean the stop method?

Yes, sorry for the confusion!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants