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

Cannot read properties of null (reading 'once') in 'promise.js' #974

Open
CamilleDrapier opened this issue Feb 7, 2023 · 4 comments
Open

Comments

@CamilleDrapier
Copy link

Describe the bug
Upon calling client.disconnect(); I notice I sometimes having the following problem raised in promise.js

Sadly, I'm not completely sure about how to reproduce this problem as it might be related to the reconnection mechanism being triggered, or because of multiple disconnection attempts being called in parallel?

Logs

TypeError

Cannot read properties of null (reading 'once')

Crashed in non-app:
./node_modules/@xmpp/events/lib/promise.js in <anonymous>
<anonymous> in new Promise
Called from:
./node_modules/@xmpp/events/lib/promise.js in module.exports

It seems that the problem is that the socket has been reset to null (maybe while the promise was awaited?), probably by the _detachSocket method. but I'm not sure of the order of events that allow this to happen. It seems this happens while handling a timeout error from a ping we send from clients to the server in order to check if the client is indeed still connected; so maybe there was some concurrent event happing at that time leading to the socket being reset.

Environment
As much as possible information about the environment running xmpp.js

browser: Chrome 109.0.0
Xmpp.js version: 0.13.1

@sonnyp
Copy link
Member

sonnyp commented Jan 5, 2025

You shouldn't use client.disconnect.

Use client.stop

@sonnyp sonnyp closed this as completed Jan 5, 2025
@CamilleDrapier
Copy link
Author

Thanks @sonnyp for the reply and insight!

The reason why I'm sometimes using client.disconnect so far is because I am manually checking for disconnections with a manual PING (on Chrome, notably, this seems necessary, when the network is simply disabled from the OS interface, or the "cable plugged out"), and in that case, I want to disconnect (so that my application behaves such as XMPP is disconnected and attempting to reconnect) but not to stop the reconnection attempts. This is somewhat what is discussed in #993

Using stop seems to sometimes stop the reconnection, sometimes not (in Chrome at least I have observed different behaviors, this being called when the network connection is actually disabled from the OS, which might explain why it ends up in reconnecting or in offline sometimes~): cf: #956

If I want to achieve the same without disconnect, should I try the solution you advised here? Later in that issue, you might have mentioned that stop might not cancel/stop the reconnect. In the current doc, the ".stop also stops reconnection" is what is stated, and I believe that would be logical that it does. 💡

Sorry for replying on an issue that you closed, just wanted to add additional information in case it is of any use 🙇

@sonnyp sonnyp reopened this Jan 7, 2025
@sonnyp
Copy link
Member

sonnyp commented Jan 7, 2025

Using stop seems to sometimes stop the reconnection

That's the expected behavior. It's a bug otherwise.

just wanted to add additional information in case it is of any use 🙇

Honestly, I would much rather receive PRs with tests and documentation / explanations. I don't use xmpp.js in a professional setup currently as such all of my work here is 100% volunteer. I could use some help :)

@sonnyp
Copy link
Member

sonnyp commented Jan 7, 2025

Regarding network reliability, I think our best bet is to have a built-in/upstream solution.

Something like #1005 . I believe @singpolyma will rebase it so we could have it in the next release of xmpp.js

sonnyp added a commit that referenced this issue Jan 8, 2025
Users keep using it expecting it to be a public method even though it is not documented

* #1013
* b25e478
* #974

I guess that's fair, I'll create a new `disconnect` method that is safe to use.
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