connection: Fix race condition when socket closes after timeout (#1048)

packages/connection/index.js

@@ -57,16 +57,18 @@ class Connection extends EventEmitter {
     this.emit("error", error);
   }
 
+  #onSocketClosed(dirty, event) {
+    this._reset();
+    this._status("disconnect", { clean: !dirty, event });
+  }
+
   _attachSocket(socket) {
     this.socket = socket;
     const listeners = this.socketListeners;
 
     listeners.data = this._onData.bind(this);
 
-    listeners.close = (dirty, event) => {
-      this._reset();
-      this._status("disconnect", { clean: !dirty, event });
-    };
+    listeners.close = this.#onSocketClosed.bind(this);
 
     listeners.connect = () => {
       this._status("connect");
@@ -178,6 +180,22 @@ class Connection extends EventEmitter {
     return this.jid;
   }
 
+  /*
+  [
+    "offline",
+    // "disconnect",
+    "connecting",
+    "connected",
+    "opening",
+    "open",
+    "online",
+    "closing",
+    "close",
+    "disconnecting",
+    "disconnect",
+    "offline",
+  ];
+  */
   _status(status, ...args) {
     if (this.status === status) return;
     this.status = status;
@@ -201,7 +219,9 @@ class Connection extends EventEmitter {
 
     try {
       await this.disconnect();
-    } catch {}
+    } catch (err) {
+      this.#onSocketClosed(true, err);
+    }
 
     return el;
   }

packages/connection/test/stop.js

@@ -1,4 +1,5 @@
 import Connection from "../index.js";
+import { EventEmitter } from "@xmpp/events";
 
 test("resolves if socket property is undefined", async () => {
   const conn = new Connection();
@@ -38,3 +39,32 @@ test("does not throw if connection is not established", async () => {
   await conn.stop();
   expect().pass();
 });
+
+// https://github.com/xmppjs/xmpp.js/issues/956
+test("socket closes after timeout", (done) => {
+  const conn = new Connection();
+  conn.timeout = 100;
+
+  const socket = new EventEmitter();
+  socket.end = jest.fn(async () => {
+    // Mock receiving "close" event after timeout
+    setTimeout(() => {
+      socket.emit("close");
+    }, conn.timeout * 2);
+  });
+  conn._attachSocket(socket);
+
+  const statuses = [conn.status];
+  conn.on("status", (status) => {
+    statuses.push(status);
+  });
+
+  conn.stop();
+
+  // Wait a bit and assert that status is correct
+  setTimeout(() => {
+    expect(conn.status).toBe("offline");
+    expect(conn.status).not.toBe("disconnect");
+    done();
+  }, conn.timeout * 3);
+});

test/client.js

@@ -1,3 +1,5 @@
+// eslint-disable-next-line n/no-extraneous-import
+import { promise } from "@xmpp/events";
 import { client, xml, jid } from "../packages/client/index.js";
 import debug from "../packages/debug/index.js";
 import server from "../server/index.js";
@@ -143,6 +145,44 @@ test("does not reconnect when stop is called", (done) => {
   xmpp.start();
 });
 
+test("statuses", async () => {
+  xmpp = client({ credentials, service: domain });
+  debug(xmpp);
+
+  let statuses = [xmpp.status];
+
+  xmpp.on("status", (status) => {
+    statuses.push(status);
+  });
+
+  xmpp.on("error", () => {});
+
+  await xmpp.start();
+
+  expect(statuses).toEqual([
+    "offline",
+    "connecting",
+    "connect",
+    "opening",
+    "open",
+    "online",
+  ]);
+
+  // trigger reconnect
+  await xmpp.disconnect();
+
+  statuses = [xmpp.status];
+  await promise(xmpp, "open");
+
+  expect(statuses).toEqual([
+    "disconnect",
+    "connecting",
+    "connect",
+    "opening",
+    "open",
+  ]);
+});
+
 test("anonymous authentication", (done) => {
   expect.assertions(2);