xmppjs / xmpp.js

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

Node JS crash: Uncaught exception "Connection is closing" #961

Closed moufmouf closed 1 year ago

moufmouf commented 1 year ago

Describe the bug

We use XMPP.js in a NodeJS application to connect to an XMPP server (Ejabberd).

The problem happens randomly and rarely (a few times a day on a server with ~500 people). The NodeJS server crashes.

Logs

In the logs, we see:

/usr/src/node_modules/@xmpp/connection/index.js:325
    return new Promise((resolve, reject) => {
           ^
Error: Connection is closing
    at /usr/src/node_modules/@xmpp/connection/index.js:329:16
    at new Promise (<anonymous>)
    at Client.write (/usr/src/node_modules/@xmpp/connection/index.js:325:12)
    at Client.send (/usr/src/node_modules/@xmpp/connection/index.js:313:16)
    at Client.send (/usr/src/node_modules/@xmpp/websocket/lib/Connection.js:22:18)
    at Client.send (/usr/src/node_modules/@xmpp/client-core/lib/Client.js:12:42)
    at enable (/usr/src/node_modules/@xmpp/stream-management/index.js:10:10)
    at /usr/src/node_modules/@xmpp/stream-management/index.js:114:27
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
error Command failed with exit code 1.

So the error is linked to closing connection. Looking at the stacktrace, I cannot detect what triggers it from my own application (because the whole code is triggered by a runMicrotasks). This is also why I cannot properly catch and handle the error.

The error happens when sending the <enable/> stanza (see stream-management/index.js:114)

My guess (but I may be wrong) is that if a network issue happens just after the connection (and before the <enable/> stanza is sent), the exception is not caught properly and goes back to the top level without anyone having a change to properly catch it.

Environment

The problem occurs in the "pusher" container of WorkAdventure, available at https://github.com/thecodingmachine/workadventure/ We use the latest version of xmpp/client (0.13.1)

sonnyp commented 1 year ago

This is also why I cannot properly catch and handle the error.

You shouldn't have to anyway. This looks like a bug in xmpp.js

The error happens when sending the stanza (see stream-management/index.js:114)

https://github.com/xmppjs/xmpp.js/blob/50d58ec3c1723350b074bb48b5780abbb8889cab/packages/stream-management/index.js#L10

Looks like an await is missing here :hand_over_mouth: – could you send a PR with a test covering entity.send rejecting?