connection: Do not throw on socket data if stream parser is null (#1100)

Author: sonnyp

package-lock.json

@@ -27,7 +27,7 @@
     "husky": "^9.1.7",
     "jest": "^30.2.0",
     "jest-extended": "^7.0.0",
-    "jsdom": "^27.2.0",
+    "jsdom": "^27.3.0",
     "lerna": "^9.0.3",
     "lint-staged": "^16.2.7",
     "prettier": "^3.7.4",

package.json

@@ -46,6 +46,10 @@ class Connection extends EventEmitter {
   }
 
   _onData(data) {
+    // In some case it is possible for the socket to still have buffered data (therefor emit data events)
+    // even after _detachParser has been called.
+    // This is intentional so we just drop the data in that case; for example stream restart or stream error.
+    if (!this.parser) return;
     const str = data.toString("utf8");
     this.parser.write(str);
   }

packages/connection/index.js

@@ -12,3 +12,13 @@ test("#_onData", () => {
 
   conn._onData(foo);
 });
+
+test("#_onData does not throw after _detachParser has been called", () => {
+  expect.assertions(1);
+  const foo = "<foo>";
+  const conn = new Connection();
+  conn._detachParser();
+  expect(() => {
+    conn._onData(foo);
+  }).not.toThrow();
+});

packages/connection/test/onData.js

@@ -11,15 +11,20 @@ const domain = "localhost";
 
 let xmpp;
 
-afterEach(async () => {
+beforeAll(async () => {
+  await server.enableModules(["smacks"]);
+});
+
+beforeEach(async () => {
+  await server.restart();
+});
+
+afterAll(async () => {
   await xmpp?.stop();
   await server.reset();
 });
 
 test("client ack stanzas", async () => {
-  await server.enableModules(["smacks"]);
-  await server.restart();
-
   xmpp = client({ credentials, service: domain });
   debug(xmpp);
 
@@ -36,9 +41,6 @@ test("client ack stanzas", async () => {
 });
 
 test("client fail stanzas", async () => {
-  await server.enableModules(["smacks"]);
-  await server.restart();
-
   xmpp = client({ credentials, service: domain });
   debug(xmpp);
 
@@ -60,9 +62,6 @@ test("client fail stanzas", async () => {
 });
 
 test("client retry stanzas", async () => {
-  await server.enableModules(["smacks"]);
-  await server.restart();
-
   xmpp = client({ credentials, service: domain });
   debug(xmpp);
 
@@ -86,9 +85,6 @@ test("client retry stanzas", async () => {
 });
 
 test("client reconnects when server fails to ack stanza", async () => {
-  await server.enableModules(["smacks"]);
-  await server.restart();
-
   xmpp = client({ credentials, service: domain });
   xmpp.streamManagement.timeout = 10;
   xmpp.streamManagement.requestAckInterval = 5;
@@ -110,39 +106,3 @@ test("client reconnects when server fails to ack stanza", async () => {
   await promise_resumed;
   expect().pass();
 });
-
-test("pings do not prevent timeout", async () => {
-  await server.enableModules(["smacks"]);
-  await server.restart();
-
-  xmpp = client({ credentials, service: domain });
-  xmpp.streamManagement.timeout = 10;
-  xmpp.streamManagement.requestAckInterval = 5;
-
-  // Make sure an ack request is sent right away after a stanza
-  xmpp.streamManagement.debounceAckRequest = 1;
-  debug(xmpp);
-
-  let promise_disconnect = new Promise((resolve) => {
-    xmpp.disconnect = () => {
-      resolve();
-      return Promise.resolve(null);
-    };
-  });
-  await xmpp.start();
-
-  // Send just any stanza to trigger the act request send
-  xmpp.send(
-    <iq to={domain} id="ping" type="get">
-      <ping xmlns="urn:xmppp:ping" />
-    </iq>,
-  );
-
-  // Pretend we don't receive the ack by removing event listeners
-  // on the socket, so that the timeout can fire
-  xmpp._detachSocket();
-
-  // Timeout fires, and disconnect happens
-  await promise_disconnect;
-  expect().pass();
-});