From c4d4bb825451a564e5d32a12ba4a0f270a3f7c3e Mon Sep 17 00:00:00 2001 From: Stephen Paul Weber Date: Tue, 14 Nov 2023 16:31:15 -0500 Subject: [PATCH 01/42] Implement stream management requesting ACKs Queue outgoing stanzas and periodically request ACK. Remove from the queue anything ack'd and notify of the ack so apps can know the stanza has for sure sent. On resume, anything not ack'd is re-sent. On reconnect, anything not ack'd notify of the failure to send this stanza so apps can know the stanza failed. Even when there is no traffic, send an at least every 5 minutes to check the connection. If there is no inbound traffic (such as an ) within timeout (default 60 seconds) then consider the connection disconnected. --- package-lock.json | 1 + packages/client-core/src/bind2/bind2.test.js | 2 - packages/stream-management/index.js | 86 ++++++++++++++++-- packages/stream-management/package.json | 3 +- .../stream-management/stream-features.test.js | 87 ++++++++++++++++++- packages/xml/index.js | 3 + 6 files changed, 171 insertions(+), 11 deletions(-) diff --git a/package-lock.json b/package-lock.json index 13d18a4d..de35cabd 100644 --- a/package-lock.json +++ b/package-lock.json @@ -14407,6 +14407,7 @@ "dependencies": { "@xmpp/error": "^0.14.0", "@xmpp/events": "^0.14.0", + "@xmpp/time": "^0.14.0", "@xmpp/xml": "^0.14.0" }, "engines": { diff --git a/packages/client-core/src/bind2/bind2.test.js b/packages/client-core/src/bind2/bind2.test.js index b0aac358..23c5fddb 100644 --- a/packages/client-core/src/bind2/bind2.test.js +++ b/packages/client-core/src/bind2/bind2.test.js @@ -66,7 +66,6 @@ test("with function resource returning string", async () => { test("with function resource throwing", async () => { const error = new Error("foo"); - function resource() { throw error; } @@ -102,7 +101,6 @@ test("with function resource returning resolved promise", async () => { test("with function resource returning rejected promise", async () => { const error = new Error("foo"); - async function resource() { throw error; } diff --git a/packages/stream-management/index.js b/packages/stream-management/index.js index 9f162ce3..dc5dc7d1 100644 --- a/packages/stream-management/index.js +++ b/packages/stream-management/index.js @@ -1,6 +1,7 @@ import XMPPError from "@xmpp/error"; import { procedure } from "@xmpp/events"; import xml from "@xmpp/xml"; +import { datetime } from "@xmpp/time"; // https://xmpp.org/extensions/xep-0198.html @@ -45,24 +46,49 @@ export default function streamManagement({ bind2, sasl2, }) { + let timeoutTimeout = null; + let requestAckTimeout = null; + const sm = { allowResume: true, preferredMaximum: null, enabled: false, id: "", + outbound_q: [], outbound: 0, inbound: 0, max: null, + timeout: 60_000, + _teardown: () => { + if (timeoutTimeout) clearTimeout(timeoutTimeout); + if (requestAckTimeout) clearTimeout(requestAckTimeout); + }, }; - function resumed() { + async function resumed(resumed) { sm.enabled = true; + const oldOutbound = sm.outbound; + for (let i = 0; i < resumed.attrs.h - oldOutbound; i++) { + let stanza = sm.outbound_q.shift(); + sm.outbound++; + entity.emit("stream-management/ack", stanza); + } + let q = sm.outbound_q; + sm.outbound_q = []; + for (const item of q) { + await entity.send(item); // This will trigger the middleware and re-add to the queue + } + entity.emit("stream-management/resumed"); entity._ready(true); } function failed() { sm.enabled = false; sm.id = ""; + let stanza; + while ((stanza = sm.outbound_q.shift())) { + entity.emit("stream-management/fail", stanza); + } sm.outbound = 0; } @@ -73,11 +99,18 @@ export default function streamManagement({ } entity.on("online", () => { + if (sm.outbound_q.length > 0) { + throw "Stream Management assertion failure, queue should be empty during online"; + } sm.outbound = 0; sm.inbound = 0; }); entity.on("offline", () => { + let stanza; + while ((stanza = sm.outbound_q.shift())) { + entity.emit("stream-management/fail", stanza); + } sm.outbound = 0; sm.inbound = 0; sm.enabled = false; @@ -86,6 +119,7 @@ export default function streamManagement({ middleware.use((context, next) => { const { stanza } = context; + if (timeoutTimeout) clearTimeout(timeoutTimeout); if (["presence", "message", "iq"].includes(stanza.name)) { sm.inbound += 1; } else if (stanza.is("r", NS)) { @@ -93,7 +127,12 @@ export default function streamManagement({ entity.send(xml("a", { xmlns: NS, h: sm.inbound })).catch(() => {}); } else if (stanza.is("a", NS)) { // > When a party receives an element, it SHOULD keep a record of the 'h' value returned as the sequence number of the last handled outbound stanza for the current stream (and discard the previous value). - sm.outbound = stanza.attrs.h; + const oldOutbound = sm.outbound; + for (let i = 0; i < stanza.attrs.h - oldOutbound; i++) { + let stanza = sm.outbound_q.shift(); + sm.outbound++; + entity.emit("stream-management/ack", stanza); + } } return next(); @@ -105,6 +144,41 @@ export default function streamManagement({ if (sasl2) { setupSasl2({ sasl2, sm, failed, resumed }); } + + function requestAck() { + if (timeoutTimeout) clearTimeout(timeoutTimeout); + if (sm.timeout) { + timeoutTimeout = setTimeout(() => entity.disconnect(), sm.timeout); + } + entity.send(xml("r", { xmlns: NS })).catch(() => {}); + // Periodically send r to check the connection + // If a stanza goes out it will cancel this and set a sooner timer + requestAckTimeout = setTimeout(requestAck, 300_000); + } + + middleware.filter((context, next) => { + const { stanza } = context; + if (sm.enabled && ["presence", "message", "iq"].includes(stanza.name)) { + let qStanza = stanza; + if ( + qStanza.name === "message" && + !qStanza.getChild("delay", "urn:xmpp:delay") + ) { + qStanza = xml.clone(qStanza); + qStanza.c("delay", { + xmlns: "urn:xmpp:delay", + from: entity.jid.toString(), + stamp: datetime(), + }); + } + sm.outbound_q.push(qStanza); + // Debounce requests so we send only one after a big run of stanza together + if (requestAckTimeout) clearTimeout(requestAckTimeout); + requestAckTimeout = setTimeout(requestAck, 100); + } + return next(); + }); + if (streamFeatures) { setupStreamFeature({ streamFeatures, @@ -133,8 +207,7 @@ function setupStreamFeature({ // Resuming if (sm.id) { try { - await resume(entity, sm); - resumed(); + await resumed(await resume(entity, sm)); return; // If resumption fails, continue with session establishment } catch { @@ -150,6 +223,9 @@ function setupStreamFeature({ const promiseEnable = enable(entity, sm); // > The counter for an entity's own sent stanzas is set to zero and started after sending either or . + if (sm.outbound_q.length > 0) { + throw "Stream Management assertion failure, queue should be empty after enable"; + } sm.outbound = 0; try { @@ -172,7 +248,7 @@ function setupSasl2({ sasl2, sm, failed, resumed }) { }, (element) => { if (element.is("resumed")) { - resumed(); + resumed(element); } else if (element.is(failed)) { // const error = StreamError.fromElement(element) failed(); diff --git a/packages/stream-management/package.json b/packages/stream-management/package.json index e9818cbb..72503e38 100644 --- a/packages/stream-management/package.json +++ b/packages/stream-management/package.json @@ -16,7 +16,8 @@ "dependencies": { "@xmpp/error": "^0.14.0", "@xmpp/events": "^0.14.0", - "@xmpp/xml": "^0.14.0" + "@xmpp/xml": "^0.14.0", + "@xmpp/time": "^0.14.0" }, "engines": { "node": ">= 20" diff --git a/packages/stream-management/stream-features.test.js b/packages/stream-management/stream-features.test.js index a8edc437..845e11f2 100644 --- a/packages/stream-management/stream-features.test.js +++ b/packages/stream-management/stream-features.test.js @@ -22,6 +22,7 @@ test("enable - enabled", async () => { ); expect(entity.streamManagement.outbound).toBe(0); + expect(entity.streamManagement.outbound_q).toBeEmpty(); expect(entity.streamManagement.enabled).toBe(false); expect(entity.streamManagement.id).toBe(""); @@ -73,6 +74,7 @@ test("enable - message - enabled", async () => { ); expect(entity.streamManagement.outbound).toBe(0); + expect(entity.streamManagement.outbound_q).toBeEmpty(); expect(entity.streamManagement.enabled).toBe(false); expect(entity.streamManagement.id).toBe(""); @@ -112,6 +114,7 @@ test("enable - failed", async () => { ); expect(entity.streamManagement.outbound).toBe(0); + expect(entity.streamManagement.outbound_q).toBeEmpty(); entity.streamManagement.enabled = true; entity.mockInput( @@ -125,6 +128,54 @@ test("enable - failed", async () => { expect(entity.streamManagement.enabled).toBe(false); }); +test("enable - enabled - message", async () => { + const { entity } = mockClient(); + + entity.mockInput( + + + , + ); + + expect(await entity.catchOutgoing()).toEqual( + , + ); + + entity.mockInput( + , + ); + + await tick(); + + expect(entity.streamManagement.outbound).toBe(0); + expect(entity.streamManagement.outbound_q).toBeEmpty(); + expect(entity.streamManagement.enabled).toBe(true); + + await entity.send(); + entity.streamManagement._teardown(); + + expect(entity.streamManagement.outbound).toBe(0); + expect(entity.streamManagement.outbound_q).toHaveLength(1); + + let acks = 0; + entity.on("stream-management/ack", (stanza) => { + expect(stanza.attrs.id).toBe("a"); + acks++; + }); + + entity.mockInput(); + await tick(); + + expect(acks).toBe(1); + expect(entity.streamManagement.outbound).toBe(1); + expect(entity.streamManagement.outbound_q).toHaveLength(0); +}); + test("resume - resumed", async () => { const { entity } = mockClient(); @@ -138,6 +189,7 @@ test("resume - resumed", async () => { ); entity.streamManagement.outbound = 45; + entity.streamManagement.outbound_q = [, ]; expect(await entity.catchOutgoing()).toEqual( , @@ -147,11 +199,31 @@ test("resume - resumed", async () => { expect(entity.status).toBe("offline"); - entity.mockInput(); + entity.mockInput(); - await tick(); + let acks = 0; + entity.on("stream-management/ack", (stanza) => { + expect(stanza.attrs.id).toBe("a"); + acks++; + }); + + expect(await entity.catchOutgoing()).toEqual(); - expect(entity.streamManagement.outbound).toBe(45); + let resumed = false; + entity.on("stream-management/resumed", () => { + resumed = true; + }); + + await tick(); + entity.streamManagement._teardown(); + + expect(resumed).toBe(true); + expect(acks).toBe(1); + expect(entity.streamManagement.outbound).toBe(46); + expect(entity.streamManagement.outbound_q).toHaveLength(1); + expect( + entity.streamManagement.outbound_q[0].getChild("delay", "urn:xmpp:delay"), + ).not.toBeUndefined(); expect(entity.status).toBe("online"); }); @@ -162,6 +234,7 @@ test("resume - failed", async () => { entity.streamManagement.id = "bar"; entity.streamManagement.enabled = true; entity.streamManagement.outbound = 45; + entity.streamManagement.outbound_q = ["hai"]; entity.mockInput( @@ -179,10 +252,18 @@ test("resume - failed", async () => { , ); + let failures = 0; + entity.on("stream-management/fail", (failed) => { + failures++; + expect(failed).toBe("hai"); + }); + await tick(); + expect(failures).toBe(1); expect(entity.status).toBe("bar"); expect(entity.streamManagement.id).toBe(""); expect(entity.streamManagement.enabled).toBe(false); expect(entity.streamManagement.outbound).toBe(0); + expect(entity.streamManagement.outbound_q).toBeEmpty(); }); diff --git a/packages/xml/index.js b/packages/xml/index.js index efb06e0c..7b512e86 100644 --- a/packages/xml/index.js +++ b/packages/xml/index.js @@ -1,4 +1,5 @@ import Element from "ltx/lib/Element.js"; +import clone from "ltx/lib/clone.js"; import createElement from "ltx/lib/createElement.js"; import Parser from "./lib/Parser.js"; import { @@ -17,6 +18,7 @@ Object.assign(xml, { Element, createElement, Parser, + clone, escapeXML, unescapeXML, escapeXMLText, @@ -29,6 +31,7 @@ export { Element, createElement, Parser, + clone, escapeXML, unescapeXML, escapeXMLText, From 6fa312782c49e5514c1aacc7d978abcd9da0092a Mon Sep 17 00:00:00 2001 From: Sonny Piers Date: Wed, 8 Jan 2025 10:13:58 +0100 Subject: [PATCH 02/42] Emit events on streamManagement object as suggested on https://github.com/xmppjs/xmpp.js/pull/1005#pullrequestreview-2499707865 --- packages/stream-management/index.js | 17 +++++++++-------- .../stream-management/stream-features.test.js | 8 ++++---- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/packages/stream-management/index.js b/packages/stream-management/index.js index dc5dc7d1..a04759bf 100644 --- a/packages/stream-management/index.js +++ b/packages/stream-management/index.js @@ -1,5 +1,5 @@ import XMPPError from "@xmpp/error"; -import { procedure } from "@xmpp/events"; +import { EventEmitter, procedure } from "@xmpp/events"; import xml from "@xmpp/xml"; import { datetime } from "@xmpp/time"; @@ -49,7 +49,8 @@ export default function streamManagement({ let timeoutTimeout = null; let requestAckTimeout = null; - const sm = { + const sm = new EventEmitter(); + Object.assign(sm, { allowResume: true, preferredMaximum: null, enabled: false, @@ -63,7 +64,7 @@ export default function streamManagement({ if (timeoutTimeout) clearTimeout(timeoutTimeout); if (requestAckTimeout) clearTimeout(requestAckTimeout); }, - }; + }); async function resumed(resumed) { sm.enabled = true; @@ -71,14 +72,14 @@ export default function streamManagement({ for (let i = 0; i < resumed.attrs.h - oldOutbound; i++) { let stanza = sm.outbound_q.shift(); sm.outbound++; - entity.emit("stream-management/ack", stanza); + sm.emit("ack", stanza); } let q = sm.outbound_q; sm.outbound_q = []; for (const item of q) { await entity.send(item); // This will trigger the middleware and re-add to the queue } - entity.emit("stream-management/resumed"); + sm.emit("resumed"); entity._ready(true); } @@ -87,7 +88,7 @@ export default function streamManagement({ sm.id = ""; let stanza; while ((stanza = sm.outbound_q.shift())) { - entity.emit("stream-management/fail", stanza); + sm.emit("fail", stanza); } sm.outbound = 0; } @@ -109,7 +110,7 @@ export default function streamManagement({ entity.on("offline", () => { let stanza; while ((stanza = sm.outbound_q.shift())) { - entity.emit("stream-management/fail", stanza); + sm.emit("fail", stanza); } sm.outbound = 0; sm.inbound = 0; @@ -131,7 +132,7 @@ export default function streamManagement({ for (let i = 0; i < stanza.attrs.h - oldOutbound; i++) { let stanza = sm.outbound_q.shift(); sm.outbound++; - entity.emit("stream-management/ack", stanza); + sm.emit("ack", stanza); } } diff --git a/packages/stream-management/stream-features.test.js b/packages/stream-management/stream-features.test.js index 845e11f2..cb83a48e 100644 --- a/packages/stream-management/stream-features.test.js +++ b/packages/stream-management/stream-features.test.js @@ -163,7 +163,7 @@ test("enable - enabled - message", async () => { expect(entity.streamManagement.outbound_q).toHaveLength(1); let acks = 0; - entity.on("stream-management/ack", (stanza) => { + entity.streamManagement.on("ack", (stanza) => { expect(stanza.attrs.id).toBe("a"); acks++; }); @@ -202,7 +202,7 @@ test("resume - resumed", async () => { entity.mockInput(); let acks = 0; - entity.on("stream-management/ack", (stanza) => { + entity.streamManagement.on("ack", (stanza) => { expect(stanza.attrs.id).toBe("a"); acks++; }); @@ -210,7 +210,7 @@ test("resume - resumed", async () => { expect(await entity.catchOutgoing()).toEqual(); let resumed = false; - entity.on("stream-management/resumed", () => { + entity.streamManagement.on("resumed", () => { resumed = true; }); @@ -253,7 +253,7 @@ test("resume - failed", async () => { ); let failures = 0; - entity.on("stream-management/fail", (failed) => { + entity.streamManagement.on("fail", (failed) => { failures++; expect(failed).toBe("hai"); }); From 710b16754428ca5d0501e812e98d74a50710a715 Mon Sep 17 00:00:00 2001 From: Sonny Piers Date: Wed, 8 Jan 2025 10:17:20 +0100 Subject: [PATCH 03/42] Remove unecessary conditions to clearTimeout --- packages/stream-management/index.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/stream-management/index.js b/packages/stream-management/index.js index a04759bf..8cc8c575 100644 --- a/packages/stream-management/index.js +++ b/packages/stream-management/index.js @@ -61,8 +61,8 @@ export default function streamManagement({ max: null, timeout: 60_000, _teardown: () => { - if (timeoutTimeout) clearTimeout(timeoutTimeout); - if (requestAckTimeout) clearTimeout(requestAckTimeout); + clearTimeout(timeoutTimeout); + clearTimeout(requestAckTimeout); }, }); @@ -120,7 +120,7 @@ export default function streamManagement({ middleware.use((context, next) => { const { stanza } = context; - if (timeoutTimeout) clearTimeout(timeoutTimeout); + clearTimeout(timeoutTimeout); if (["presence", "message", "iq"].includes(stanza.name)) { sm.inbound += 1; } else if (stanza.is("r", NS)) { @@ -147,7 +147,7 @@ export default function streamManagement({ } function requestAck() { - if (timeoutTimeout) clearTimeout(timeoutTimeout); + clearTimeout(timeoutTimeout); if (sm.timeout) { timeoutTimeout = setTimeout(() => entity.disconnect(), sm.timeout); } @@ -174,7 +174,7 @@ export default function streamManagement({ } sm.outbound_q.push(qStanza); // Debounce requests so we send only one after a big run of stanza together - if (requestAckTimeout) clearTimeout(requestAckTimeout); + clearTimeout(requestAckTimeout); requestAckTimeout = setTimeout(requestAck, 100); } return next(); From ce30e8b960fb4d87ec310a6095a5488744bfed53 Mon Sep 17 00:00:00 2001 From: Sonny Piers Date: Wed, 8 Jan 2025 10:25:18 +0100 Subject: [PATCH 04/42] prefer early return --- packages/stream-management/index.js | 35 +++++++++++++++-------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/packages/stream-management/index.js b/packages/stream-management/index.js index 8cc8c575..74d2eee9 100644 --- a/packages/stream-management/index.js +++ b/packages/stream-management/index.js @@ -158,25 +158,26 @@ export default function streamManagement({ } middleware.filter((context, next) => { + if (!sm.enabled) return next(); const { stanza } = context; - if (sm.enabled && ["presence", "message", "iq"].includes(stanza.name)) { - let qStanza = stanza; - if ( - qStanza.name === "message" && - !qStanza.getChild("delay", "urn:xmpp:delay") - ) { - qStanza = xml.clone(qStanza); - qStanza.c("delay", { - xmlns: "urn:xmpp:delay", - from: entity.jid.toString(), - stamp: datetime(), - }); - } - sm.outbound_q.push(qStanza); - // Debounce requests so we send only one after a big run of stanza together - clearTimeout(requestAckTimeout); - requestAckTimeout = setTimeout(requestAck, 100); + if (!["presence", "message", "iq"].includes(stanza.name)) return next(); + + let qStanza = stanza; + if ( + qStanza.name === "message" && + !qStanza.getChild("delay", "urn:xmpp:delay") + ) { + qStanza = xml.clone(qStanza); + qStanza.c("delay", { + xmlns: "urn:xmpp:delay", + from: entity.jid.toString(), + stamp: datetime(), + }); } + sm.outbound_q.push(qStanza); + // Debounce requests so we send only one after a big run of stanza together + clearTimeout(requestAckTimeout); + requestAckTimeout = setTimeout(requestAck, 100); return next(); }); From 7ef1d8de6c5430e44904477c44d7be11ae3b25eb Mon Sep 17 00:00:00 2001 From: Stephen Paul Weber Date: Wed, 8 Jan 2025 10:50:23 -0500 Subject: [PATCH 05/42] No more clone Store the stamp seperately in the queue and then add it the first time we retry. Saves some work and a dependency. --- packages/stream-management/index.js | 44 ++++++++++--------- .../stream-management/stream-features.test.js | 20 ++++++--- packages/xml/index.js | 3 -- 3 files changed, 37 insertions(+), 30 deletions(-) diff --git a/packages/stream-management/index.js b/packages/stream-management/index.js index 74d2eee9..56c75bdb 100644 --- a/packages/stream-management/index.js +++ b/packages/stream-management/index.js @@ -66,18 +66,32 @@ export default function streamManagement({ }, }); + async function sendQueueItem({ stanza, stamp }) { + if ( + stanza.name === "message" && + !stanza.getChild("delay", "urn:xmpp:delay") + ) { + stanza.append(xml("delay", { + xmlns: "urn:xmpp:delay", + from: entity.jid.toString(), + stamp: stamp, + })); + } + await entity.send(stanza); + } + async function resumed(resumed) { sm.enabled = true; const oldOutbound = sm.outbound; for (let i = 0; i < resumed.attrs.h - oldOutbound; i++) { - let stanza = sm.outbound_q.shift(); + let item = sm.outbound_q.shift(); sm.outbound++; - sm.emit("ack", stanza); + sm.emit("ack", item.stanza); } let q = sm.outbound_q; sm.outbound_q = []; for (const item of q) { - await entity.send(item); // This will trigger the middleware and re-add to the queue + await sendQueueItem(item); // This will trigger the middleware and re-add to the queue } sm.emit("resumed"); entity._ready(true); @@ -86,9 +100,9 @@ export default function streamManagement({ function failed() { sm.enabled = false; sm.id = ""; - let stanza; - while ((stanza = sm.outbound_q.shift())) { - sm.emit("fail", stanza); + let item; + while ((item = sm.outbound_q.shift())) { + sm.emit("fail", item.stanza); } sm.outbound = 0; } @@ -130,9 +144,9 @@ export default function streamManagement({ // > When a party receives an element, it SHOULD keep a record of the 'h' value returned as the sequence number of the last handled outbound stanza for the current stream (and discard the previous value). const oldOutbound = sm.outbound; for (let i = 0; i < stanza.attrs.h - oldOutbound; i++) { - let stanza = sm.outbound_q.shift(); + let item = sm.outbound_q.shift(); sm.outbound++; - sm.emit("ack", stanza); + sm.emit("ack", item.stanza); } } @@ -162,19 +176,7 @@ export default function streamManagement({ const { stanza } = context; if (!["presence", "message", "iq"].includes(stanza.name)) return next(); - let qStanza = stanza; - if ( - qStanza.name === "message" && - !qStanza.getChild("delay", "urn:xmpp:delay") - ) { - qStanza = xml.clone(qStanza); - qStanza.c("delay", { - xmlns: "urn:xmpp:delay", - from: entity.jid.toString(), - stamp: datetime(), - }); - } - sm.outbound_q.push(qStanza); + sm.outbound_q.push({ stanza, stamp: datetime() }); // Debounce requests so we send only one after a big run of stanza together clearTimeout(requestAckTimeout); requestAckTimeout = setTimeout(requestAck, 100); diff --git a/packages/stream-management/stream-features.test.js b/packages/stream-management/stream-features.test.js index cb83a48e..f30c572d 100644 --- a/packages/stream-management/stream-features.test.js +++ b/packages/stream-management/stream-features.test.js @@ -189,7 +189,10 @@ test("resume - resumed", async () => { ); entity.streamManagement.outbound = 45; - entity.streamManagement.outbound_q = [, ]; + entity.streamManagement.outbound_q = [ + { stanza: , stamp: "1990-01-01T00:00:00Z" }, + { stanza: , stamp: "1990-01-01T00:00:00Z" }, + ]; expect(await entity.catchOutgoing()).toEqual( , @@ -207,7 +210,15 @@ test("resume - resumed", async () => { acks++; }); - expect(await entity.catchOutgoing()).toEqual(); + expect(await entity.catchOutgoing()).toEqual( + + + , + ); let resumed = false; entity.streamManagement.on("resumed", () => { @@ -221,9 +232,6 @@ test("resume - resumed", async () => { expect(acks).toBe(1); expect(entity.streamManagement.outbound).toBe(46); expect(entity.streamManagement.outbound_q).toHaveLength(1); - expect( - entity.streamManagement.outbound_q[0].getChild("delay", "urn:xmpp:delay"), - ).not.toBeUndefined(); expect(entity.status).toBe("online"); }); @@ -234,7 +242,7 @@ test("resume - failed", async () => { entity.streamManagement.id = "bar"; entity.streamManagement.enabled = true; entity.streamManagement.outbound = 45; - entity.streamManagement.outbound_q = ["hai"]; + entity.streamManagement.outbound_q = [{ stanza: "hai" }]; entity.mockInput( diff --git a/packages/xml/index.js b/packages/xml/index.js index 7b512e86..efb06e0c 100644 --- a/packages/xml/index.js +++ b/packages/xml/index.js @@ -1,5 +1,4 @@ import Element from "ltx/lib/Element.js"; -import clone from "ltx/lib/clone.js"; import createElement from "ltx/lib/createElement.js"; import Parser from "./lib/Parser.js"; import { @@ -18,7 +17,6 @@ Object.assign(xml, { Element, createElement, Parser, - clone, escapeXML, unescapeXML, escapeXMLText, @@ -31,7 +29,6 @@ export { Element, createElement, Parser, - clone, escapeXML, unescapeXML, escapeXMLText, From d76e479eed725a10ce1f1f7b9fc3e3b949a49223 Mon Sep 17 00:00:00 2001 From: Stephen Paul Weber Date: Wed, 8 Jan 2025 10:51:01 -0500 Subject: [PATCH 06/42] Throw Error not string --- packages/stream-management/index.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/stream-management/index.js b/packages/stream-management/index.js index 56c75bdb..6273002b 100644 --- a/packages/stream-management/index.js +++ b/packages/stream-management/index.js @@ -115,7 +115,9 @@ export default function streamManagement({ entity.on("online", () => { if (sm.outbound_q.length > 0) { - throw "Stream Management assertion failure, queue should be empty during online"; + throw new Error( + "Stream Management assertion failure, queue should be empty during online", + ); } sm.outbound = 0; sm.inbound = 0; @@ -228,7 +230,9 @@ function setupStreamFeature({ // > The counter for an entity's own sent stanzas is set to zero and started after sending either or . if (sm.outbound_q.length > 0) { - throw "Stream Management assertion failure, queue should be empty after enable"; + throw new Error( + "Stream Management assertion failure, queue should be empty after enable", + ); } sm.outbound = 0; From 69d0752e6085a3ab51d6ddc98853206d2c0f72b8 Mon Sep 17 00:00:00 2001 From: Stephen Paul Weber Date: Wed, 8 Jan 2025 10:51:16 -0500 Subject: [PATCH 07/42] Catch any disconnect failure --- packages/stream-management/index.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/stream-management/index.js b/packages/stream-management/index.js index 6273002b..e5f1ce69 100644 --- a/packages/stream-management/index.js +++ b/packages/stream-management/index.js @@ -165,7 +165,10 @@ export default function streamManagement({ function requestAck() { clearTimeout(timeoutTimeout); if (sm.timeout) { - timeoutTimeout = setTimeout(() => entity.disconnect(), sm.timeout); + timeoutTimeout = setTimeout( + () => entity.disconnect().catch(), + sm.timeout, + ); } entity.send(xml("r", { xmlns: NS })).catch(() => {}); // Periodically send r to check the connection From 0190cb6bb5013a8746e48f05f34e741c87217876 Mon Sep 17 00:00:00 2001 From: Stephen Paul Weber Date: Wed, 8 Jan 2025 10:51:28 -0500 Subject: [PATCH 08/42] Reformat this line --- packages/stream-management/index.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/stream-management/index.js b/packages/stream-management/index.js index e5f1ce69..dcae39fa 100644 --- a/packages/stream-management/index.js +++ b/packages/stream-management/index.js @@ -216,7 +216,8 @@ function setupStreamFeature({ // Resuming if (sm.id) { try { - await resumed(await resume(entity, sm)); + const element = await resume(entity, sm); + await resumed(element); return; // If resumption fails, continue with session establishment } catch { From b2bf99cc19cd96e5f78022b309df3016c409ff1d Mon Sep 17 00:00:00 2001 From: Stephen Paul Weber Date: Wed, 8 Jan 2025 10:55:45 -0500 Subject: [PATCH 09/42] Make more timeouts configurable --- packages/stream-management/index.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/stream-management/index.js b/packages/stream-management/index.js index dcae39fa..158ed6ac 100644 --- a/packages/stream-management/index.js +++ b/packages/stream-management/index.js @@ -60,6 +60,8 @@ export default function streamManagement({ inbound: 0, max: null, timeout: 60_000, + requestAckInterval: 300_000, + debounceAckRequest: 100, _teardown: () => { clearTimeout(timeoutTimeout); clearTimeout(requestAckTimeout); @@ -173,7 +175,7 @@ export default function streamManagement({ entity.send(xml("r", { xmlns: NS })).catch(() => {}); // Periodically send r to check the connection // If a stanza goes out it will cancel this and set a sooner timer - requestAckTimeout = setTimeout(requestAck, 300_000); + requestAckTimeout = setTimeout(requestAck, sm.requestAckInterval); } middleware.filter((context, next) => { @@ -184,7 +186,7 @@ export default function streamManagement({ sm.outbound_q.push({ stanza, stamp: datetime() }); // Debounce requests so we send only one after a big run of stanza together clearTimeout(requestAckTimeout); - requestAckTimeout = setTimeout(requestAck, 100); + requestAckTimeout = setTimeout(requestAck, sm.debounceAckRequest); return next(); }); From 0f79b3b3ab717bc5a134ab785eacfd548c27a4ba Mon Sep 17 00:00:00 2001 From: Stephen Paul Weber Date: Wed, 8 Jan 2025 10:58:21 -0500 Subject: [PATCH 10/42] Test failure with something in queue seperately --- .../stream-management/stream-features.test.js | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/packages/stream-management/stream-features.test.js b/packages/stream-management/stream-features.test.js index f30c572d..1439a25d 100644 --- a/packages/stream-management/stream-features.test.js +++ b/packages/stream-management/stream-features.test.js @@ -238,6 +238,40 @@ test("resume - resumed", async () => { test("resume - failed", async () => { const { entity } = mockClient(); + entity.status = "bar"; + entity.streamManagement.id = "bar"; + entity.streamManagement.enabled = true; + entity.streamManagement.outbound = 45; + entity.streamManagement.outbound_q = []; + + entity.mockInput( + + + , + ); + + expect(await entity.catchOutgoing()).toEqual( + , + ); + + entity.mockInput( + + + , + ); + + await tick(); + + expect(entity.status).toBe("bar"); + expect(entity.streamManagement.id).toBe(""); + expect(entity.streamManagement.enabled).toBe(false); + expect(entity.streamManagement.outbound).toBe(0); + expect(entity.streamManagement.outbound_q).toBeEmpty(); +}); + +test("resume - failed with something in queue", async () => { + const { entity } = mockClient(); + entity.status = "bar"; entity.streamManagement.id = "bar"; entity.streamManagement.enabled = true; From efad5c1e3dd7271888156bb3f8e9a682912451f5 Mon Sep 17 00:00:00 2001 From: Stephen Paul Weber Date: Wed, 8 Jan 2025 11:03:37 -0500 Subject: [PATCH 11/42] Document events in readme --- packages/stream-management/README.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/stream-management/README.md b/packages/stream-management/README.md index 296b8ad4..d28596e5 100644 --- a/packages/stream-management/README.md +++ b/packages/stream-management/README.md @@ -10,7 +10,13 @@ When the session is resumed the `online` event is not emitted as session resumpt However `entity.status` is set to `online`. If the session fails to resume, entity will fallback to regular session establishment in which case `online` event will be emitted. -Automatically responds to acks but does not support requesting acks yet. +Automatically responds to acks and requests them. Also requests periodically even if you haven't sent anything. If server fails to respond to a request, the module triggers a reconnect. + +## Events + +**resumed**: Indicates that the connection was resumed (so online with no online event) +**fail**: Indicates that a stanza failed to send to the server and will not be retried +**ack**: Indicates that a stanza has been acknowledged by the server ## References From 8cb8dff2ae1d245cc2bff651a92eddcdece8c2af Mon Sep 17 00:00:00 2001 From: Stephen Paul Weber Date: Mon, 13 Jan 2025 11:09:38 -0500 Subject: [PATCH 12/42] Fix comment position --- packages/stream-management/index.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/stream-management/index.js b/packages/stream-management/index.js index 158ed6ac..eebdd5a2 100644 --- a/packages/stream-management/index.js +++ b/packages/stream-management/index.js @@ -234,12 +234,13 @@ function setupStreamFeature({ const promiseEnable = enable(entity, sm); - // > The counter for an entity's own sent stanzas is set to zero and started after sending either or . if (sm.outbound_q.length > 0) { throw new Error( "Stream Management assertion failure, queue should be empty after enable", ); } + + // > The counter for an entity's own sent stanzas is set to zero and started after sending either or . sm.outbound = 0; try { From 8c408ef572c9f16658960fe72ee3afbc840ce93a Mon Sep 17 00:00:00 2001 From: Stephen Paul Weber Date: Wed, 8 Jan 2025 11:36:43 -0500 Subject: [PATCH 13/42] e2e test for stream management --- test/stream-management.js | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 test/stream-management.js diff --git a/test/stream-management.js b/test/stream-management.js new file mode 100644 index 00000000..3db72ee1 --- /dev/null +++ b/test/stream-management.js @@ -0,0 +1,37 @@ +import { client } from "../packages/client/index.js"; +import debug from "../packages/debug/index.js"; +import server from "../server/index.js"; + +const username = "client"; +const password = "foobar"; +const credentials = { username, password }; +const domain = "localhost"; + +let xmpp; + +afterEach(async () => { + await xmpp?.stop(); + await server.reset(); +}); + +test("client ack stanzas", async () => { + expect.assertions(1); + + await server.enableModules(["smacks"]); + await server.restart(); + + xmpp = client({ credentials, service: domain }); + debug(xmpp); + + xmpp.streamManagement.on("ack", (el) => { + expect(el.attrs.id).toEqual("ping"); + xmpp.streamManagement._teardown(); + }); + + await xmpp.start(); + await xmpp.send( + + + , + ); +}); From 43a2d69180511ce727497d94d9da73ac6591e987 Mon Sep 17 00:00:00 2001 From: Stephen Paul Weber Date: Mon, 13 Jan 2025 11:29:05 -0500 Subject: [PATCH 14/42] Use sendMany --- packages/stream-management/index.js | 21 +++++++++++---------- packages/test/mockClient.js | 5 +++++ 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/packages/stream-management/index.js b/packages/stream-management/index.js index eebdd5a2..02bfb7f7 100644 --- a/packages/stream-management/index.js +++ b/packages/stream-management/index.js @@ -68,18 +68,20 @@ export default function streamManagement({ }, }); - async function sendQueueItem({ stanza, stamp }) { + function queueToStanza({ stanza, stamp }) { if ( stanza.name === "message" && !stanza.getChild("delay", "urn:xmpp:delay") ) { - stanza.append(xml("delay", { - xmlns: "urn:xmpp:delay", - from: entity.jid.toString(), - stamp: stamp, - })); + stanza.append( + xml("delay", { + xmlns: "urn:xmpp:delay", + from: entity.jid.toString(), + stamp: stamp, + }), + ); } - await entity.send(stanza); + return stanza; } async function resumed(resumed) { @@ -92,9 +94,8 @@ export default function streamManagement({ } let q = sm.outbound_q; sm.outbound_q = []; - for (const item of q) { - await sendQueueItem(item); // This will trigger the middleware and re-add to the queue - } + // This will trigger the middleware and re-add to the queue + await entity.sendMany(q.map((item) => queueToStanza(item))); sm.emit("resumed"); entity._ready(true); } diff --git a/packages/test/mockClient.js b/packages/test/mockClient.js index c7025edd..45eff046 100644 --- a/packages/test/mockClient.js +++ b/packages/test/mockClient.js @@ -5,6 +5,11 @@ import context from "./context.js"; export default function mockClient(options) { const xmpp = client(options); xmpp.send = Connection.prototype.send; + xmpp.sendMany = async (stanzas) => { + for (const stanza of stanzas) { + await xmpp.send(stanza); + } + }; const ctx = context(xmpp); return Object.assign(xmpp, ctx); } From 97e8d283e8a44e0ac47e139396cb9be6734a5004 Mon Sep 17 00:00:00 2001 From: Stephen Paul Weber Date: Mon, 13 Jan 2025 11:30:19 -0500 Subject: [PATCH 15/42] Change test name --- packages/stream-management/stream-features.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/stream-management/stream-features.test.js b/packages/stream-management/stream-features.test.js index 1439a25d..7b42b4f0 100644 --- a/packages/stream-management/stream-features.test.js +++ b/packages/stream-management/stream-features.test.js @@ -128,7 +128,7 @@ test("enable - failed", async () => { expect(entity.streamManagement.enabled).toBe(false); }); -test("enable - enabled - message", async () => { +test("stanza ack", async () => { const { entity } = mockClient(); entity.mockInput( From f076fd1e544447370a1fa0ebf0ebfe4d3e6b30b4 Mon Sep 17 00:00:00 2001 From: Stephen Paul Weber Date: Mon, 13 Jan 2025 11:32:41 -0500 Subject: [PATCH 16/42] Split up resumed test --- .../stream-management/stream-features.test.js | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/packages/stream-management/stream-features.test.js b/packages/stream-management/stream-features.test.js index 7b42b4f0..757fe358 100644 --- a/packages/stream-management/stream-features.test.js +++ b/packages/stream-management/stream-features.test.js @@ -220,6 +220,59 @@ test("resume - resumed", async () => { , ); + await tick(); + entity.streamManagement._teardown(); + + expect(acks).toBe(1); + expect(entity.streamManagement.outbound).toBe(46); + expect(entity.streamManagement.outbound_q).toHaveLength(1); + expect(entity.status).toBe("online"); +}); + +test("resumed event", async () => { + const { entity } = mockClient(); + + entity.status = "offline"; + entity.streamManagement.id = "bar"; + + entity.mockInput( + + + , + ); + + entity.streamManagement.outbound = 45; + entity.streamManagement.outbound_q = [ + { stanza: , stamp: "1990-01-01T00:00:00Z" }, + { stanza: , stamp: "1990-01-01T00:00:00Z" }, + ]; + + expect(await entity.catchOutgoing()).toEqual( + , + ); + + expect(entity.streamManagement.enabled).toBe(false); + + expect(entity.status).toBe("offline"); + + entity.mockInput(); + + let acks = 0; + entity.streamManagement.on("ack", (stanza) => { + expect(stanza.attrs.id).toBe("a"); + acks++; + }); + + expect(await entity.catchOutgoing()).toEqual( + + + , + ); + let resumed = false; entity.streamManagement.on("resumed", () => { resumed = true; From f6e15ba1b559e5dd052057dd402d0f21260d48b4 Mon Sep 17 00:00:00 2001 From: Stephen Paul Weber Date: Mon, 13 Jan 2025 11:36:02 -0500 Subject: [PATCH 17/42] Teardown on disconnect --- packages/stream-management/index.js | 9 +++++---- packages/stream-management/stream-features.test.js | 3 --- test/stream-management.js | 1 - 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/packages/stream-management/index.js b/packages/stream-management/index.js index 02bfb7f7..68059037 100644 --- a/packages/stream-management/index.js +++ b/packages/stream-management/index.js @@ -62,10 +62,11 @@ export default function streamManagement({ timeout: 60_000, requestAckInterval: 300_000, debounceAckRequest: 100, - _teardown: () => { - clearTimeout(timeoutTimeout); - clearTimeout(requestAckTimeout); - }, + }); + + entity.on("disconnect", () => { + clearTimeout(timeoutTimeout); + clearTimeout(requestAckTimeout); }); function queueToStanza({ stanza, stamp }) { diff --git a/packages/stream-management/stream-features.test.js b/packages/stream-management/stream-features.test.js index 757fe358..3eefbbe9 100644 --- a/packages/stream-management/stream-features.test.js +++ b/packages/stream-management/stream-features.test.js @@ -157,7 +157,6 @@ test("stanza ack", async () => { expect(entity.streamManagement.enabled).toBe(true); await entity.send(); - entity.streamManagement._teardown(); expect(entity.streamManagement.outbound).toBe(0); expect(entity.streamManagement.outbound_q).toHaveLength(1); @@ -221,7 +220,6 @@ test("resume - resumed", async () => { ); await tick(); - entity.streamManagement._teardown(); expect(acks).toBe(1); expect(entity.streamManagement.outbound).toBe(46); @@ -279,7 +277,6 @@ test("resumed event", async () => { }); await tick(); - entity.streamManagement._teardown(); expect(resumed).toBe(true); expect(acks).toBe(1); diff --git a/test/stream-management.js b/test/stream-management.js index 3db72ee1..e277c7ba 100644 --- a/test/stream-management.js +++ b/test/stream-management.js @@ -25,7 +25,6 @@ test("client ack stanzas", async () => { xmpp.streamManagement.on("ack", (el) => { expect(el.attrs.id).toEqual("ping"); - xmpp.streamManagement._teardown(); }); await xmpp.start(); From 2209036c6a5511cc6808761f1099e6d78261d7b8 Mon Sep 17 00:00:00 2001 From: Stephen Paul Weber Date: Mon, 13 Jan 2025 11:41:52 -0500 Subject: [PATCH 18/42] Use promise helper --- test/stream-management.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/test/stream-management.js b/test/stream-management.js index e277c7ba..72bda9a6 100644 --- a/test/stream-management.js +++ b/test/stream-management.js @@ -1,4 +1,5 @@ import { client } from "../packages/client/index.js"; +import { promise } from "../packages/events/index.js"; import debug from "../packages/debug/index.js"; import server from "../server/index.js"; @@ -15,22 +16,20 @@ afterEach(async () => { }); test("client ack stanzas", async () => { - expect.assertions(1); - await server.enableModules(["smacks"]); await server.restart(); xmpp = client({ credentials, service: domain }); debug(xmpp); - xmpp.streamManagement.on("ack", (el) => { - expect(el.attrs.id).toEqual("ping"); - }); - + const elP = promise(xmpp.streamManagement, "ack"); await xmpp.start(); await xmpp.send( , ); + + const el = await elP; + expect(el.attrs.id).toEqual("ping"); }); From c66696c4d9d0cddc7176a62bc7908696c47d3636 Mon Sep 17 00:00:00 2001 From: Stephen Paul Weber Date: Mon, 13 Jan 2025 11:46:55 -0500 Subject: [PATCH 19/42] e2e test for fail event when offline --- packages/stream-management/index.js | 6 +++--- test/stream-management.js | 23 +++++++++++++++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/packages/stream-management/index.js b/packages/stream-management/index.js index 68059037..ace127af 100644 --- a/packages/stream-management/index.js +++ b/packages/stream-management/index.js @@ -128,9 +128,9 @@ export default function streamManagement({ }); entity.on("offline", () => { - let stanza; - while ((stanza = sm.outbound_q.shift())) { - sm.emit("fail", stanza); + let item; + while ((item = sm.outbound_q.shift())) { + sm.emit("fail", item.stanza); } sm.outbound = 0; sm.inbound = 0; diff --git a/test/stream-management.js b/test/stream-management.js index 72bda9a6..a8669bfd 100644 --- a/test/stream-management.js +++ b/test/stream-management.js @@ -1,5 +1,6 @@ import { client } from "../packages/client/index.js"; import { promise } from "../packages/events/index.js"; +import { datetime } from "../packages/time/index.js"; import debug from "../packages/debug/index.js"; import server from "../server/index.js"; @@ -33,3 +34,25 @@ test("client ack stanzas", async () => { const el = await elP; expect(el.attrs.id).toEqual("ping"); }); + +test("client fail stanzas", async () => { + await server.enableModules(["smacks"]); + await server.restart(); + + xmpp = client({ credentials, service: domain }); + debug(xmpp); + + const elP = promise(xmpp.streamManagement, "fail"); + await xmpp.start(); + // Expect send but don't actually send to server, so it will fail + await xmpp.streamManagement.outbound_q.push({ + stanza: + + , + stamp: datetime() + }); + await xmpp.stop(); + + const el = await elP; + expect(el.attrs.id).toEqual("ping"); +}); From bb3998ada4bdaec6ee0dd4fa7ef65374cdec784a Mon Sep 17 00:00:00 2001 From: Stephen Paul Weber Date: Mon, 13 Jan 2025 11:51:57 -0500 Subject: [PATCH 20/42] e2e test for stanza retry --- test/stream-management.js | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/test/stream-management.js b/test/stream-management.js index a8669bfd..926bbc4d 100644 --- a/test/stream-management.js +++ b/test/stream-management.js @@ -56,3 +56,25 @@ test("client fail stanzas", async () => { const el = await elP; expect(el.attrs.id).toEqual("ping"); }); + +test("client retry stanzas", async () => { + await server.enableModules(["smacks"]); + await server.restart(); + + xmpp = client({ credentials, service: domain }); + debug(xmpp); + + const elP = promise(xmpp.streamManagement, "ack"); + await xmpp.start(); + // Add to queue but don't actually send so it can retry after disconnect + await xmpp.streamManagement.outbound_q.push({ + stanza: + + , + stamp: datetime() + }); + await xmpp.disconnect(); + + const el = await elP; + expect(el.attrs.id).toEqual("ping"); +}); From 473612001c37e36dcd50979a4f15dbbcb19f6d05 Mon Sep 17 00:00:00 2001 From: Stephen Paul Weber Date: Mon, 13 Jan 2025 12:30:46 -0500 Subject: [PATCH 21/42] e2e test auto reconnect Needs to use force-disconnect especially if we emulate an issue using pause --- packages/connection/index.js | 15 +++++++++++++++ packages/stream-management/index.js | 2 +- packages/tls/lib/Socket.js | 4 ++++ packages/websocket/lib/Socket.js | 4 ++++ test/stream-management.js | 22 ++++++++++++++++++++++ 5 files changed, 46 insertions(+), 1 deletion(-) diff --git a/packages/connection/index.js b/packages/connection/index.js index 77f53d2e..3f631860 100644 --- a/packages/connection/index.js +++ b/packages/connection/index.js @@ -252,6 +252,21 @@ class Connection extends EventEmitter { await promise(this.socket, "close", "error", timeout); } + /** + * Forcibly disconnects the socket + * https://xmpp.org/rfcs/rfc6120.html#streams-close + * https://tools.ietf.org/html/rfc7395#section-3.6 + */ + async forceDisconnect(timeout = this.timeout) { + if (!this.socket) return; + + this._status("disconnecting"); + this.socket.destroy(); + + // The 'disconnect' status is set by the socket 'close' listener + await promise(this.socket, "close", "error", timeout); + } + /** * Opens the stream */ diff --git a/packages/stream-management/index.js b/packages/stream-management/index.js index ace127af..0cc7fa8b 100644 --- a/packages/stream-management/index.js +++ b/packages/stream-management/index.js @@ -170,7 +170,7 @@ export default function streamManagement({ clearTimeout(timeoutTimeout); if (sm.timeout) { timeoutTimeout = setTimeout( - () => entity.disconnect().catch(), + () => entity.forceDisconnect().catch(), sm.timeout, ); } diff --git a/packages/tls/lib/Socket.js b/packages/tls/lib/Socket.js index e0bbfa27..9b60506c 100644 --- a/packages/tls/lib/Socket.js +++ b/packages/tls/lib/Socket.js @@ -63,6 +63,10 @@ class Socket extends EventEmitter { this.socket.end(); } + destroy() { + this.socket.destroy(); + } + write(data, fn) { this.socket.write(data, fn); } diff --git a/packages/websocket/lib/Socket.js b/packages/websocket/lib/Socket.js index 3227a11b..d48e5fa4 100644 --- a/packages/websocket/lib/Socket.js +++ b/packages/websocket/lib/Socket.js @@ -75,6 +75,10 @@ export default class Socket extends EventEmitter { this.socket.close(); } + destroy() { + this.socket.close(); + } + write(data, fn) { if (WebSocket === WS) { this.socket.send(data, fn); diff --git a/test/stream-management.js b/test/stream-management.js index 926bbc4d..5d8aacca 100644 --- a/test/stream-management.js +++ b/test/stream-management.js @@ -78,3 +78,25 @@ test("client retry stanzas", async () => { const el = await elP; expect(el.attrs.id).toEqual("ping"); }); + +test("client reconnect automatically", async () => { + await server.enableModules(["smacks"]); + await server.restart(); + + xmpp = client({ credentials, service: domain }); + xmpp.streamManagement.timeout = 10; + xmpp.streamManagement.debounceAckRequest = 1; + debug(xmpp); + + const resumedP = promise(xmpp.streamManagement, "resumed"); + await xmpp.start(); + await xmpp.send( + + + , + ); + xmpp.socket.socket.pause(); + + await resumedP; + expect().pass(); +}); From b5eb66b3819738fcb4a15a7916b1448f372d7544 Mon Sep 17 00:00:00 2001 From: Sonny Piers Date: Wed, 15 Jan 2025 11:09:46 +0100 Subject: [PATCH 22/42] better docs --- packages/stream-management/README.md | 45 +++++++++++++++++++++++++--- 1 file changed, 41 insertions(+), 4 deletions(-) diff --git a/packages/stream-management/README.md b/packages/stream-management/README.md index d28596e5..d6bb4185 100644 --- a/packages/stream-management/README.md +++ b/packages/stream-management/README.md @@ -10,13 +10,50 @@ When the session is resumed the `online` event is not emitted as session resumpt However `entity.status` is set to `online`. If the session fails to resume, entity will fallback to regular session establishment in which case `online` event will be emitted. -Automatically responds to acks and requests them. Also requests periodically even if you haven't sent anything. If server fails to respond to a request, the module triggers a reconnect. +- Automatically responds to acks. +- Periodically request acks. +- If server fails to respond, triggers a reconnect. ## Events -**resumed**: Indicates that the connection was resumed (so online with no online event) -**fail**: Indicates that a stanza failed to send to the server and will not be retried -**ack**: Indicates that a stanza has been acknowledged by the server +### resumed + +Indicates that the connection was resumed. When that happens the `online` event is not emitted but `xmpp.status` will be `online`. + +```js +const xmpp = client(...); +const {streamManagement} = xmpp; + +streamManagement.on('resumed', ()) => { + console.log("session resumed"); +} +``` + +### fail + +Indicates that a stanza failed to send to the server and will not be retried. + +```js +const xmpp = client(...); +const {streamManagement} = xmpp; + +streamManagement.on('fail', (stanza)) => { + console.log("fail to send", stanza.toString()); +} +``` + +### ack + +Indicates that a stanza has been acknowledged by the server. + +```js +const xmpp = client(...); +const {streamManagement} = xmpp; + +streamManagement.on('ack', (stanza)) => { + console.log("stanza acknowledge by the server", stanza.toString()); +} +``` ## References From 74b7f0bae9ab56f7281be090aa0783643776fbff Mon Sep 17 00:00:00 2001 From: Sonny Piers Date: Wed, 15 Jan 2025 11:09:56 +0100 Subject: [PATCH 23/42] extract fn to root --- packages/stream-management/index.js | 35 +++++++++++++++-------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/packages/stream-management/index.js b/packages/stream-management/index.js index 0cc7fa8b..26eb9f37 100644 --- a/packages/stream-management/index.js +++ b/packages/stream-management/index.js @@ -69,22 +69,6 @@ export default function streamManagement({ clearTimeout(requestAckTimeout); }); - function queueToStanza({ stanza, stamp }) { - if ( - stanza.name === "message" && - !stanza.getChild("delay", "urn:xmpp:delay") - ) { - stanza.append( - xml("delay", { - xmlns: "urn:xmpp:delay", - from: entity.jid.toString(), - stamp: stamp, - }), - ); - } - return stanza; - } - async function resumed(resumed) { sm.enabled = true; const oldOutbound = sm.outbound; @@ -96,7 +80,7 @@ export default function streamManagement({ let q = sm.outbound_q; sm.outbound_q = []; // This will trigger the middleware and re-add to the queue - await entity.sendMany(q.map((item) => queueToStanza(item))); + await entity.sendMany(q.map((item) => queueToStanza({ entity, item }))); sm.emit("resumed"); entity._ready(true); } @@ -291,3 +275,20 @@ function setupBind2({ bind2, sm, failed, enabled }) { }, ); } + +function queueToStanza({ entity, item }) { + const { stanza, stamp } = item; + if ( + stanza.name === "message" && + !stanza.getChild("delay", "urn:xmpp:delay") + ) { + stanza.append( + xml("delay", { + xmlns: "urn:xmpp:delay", + from: entity.jid.toString(), + stamp, + }), + ); + } + return stanza; +} From 4a06393e1dedaa044c534ff584d198decfe83106 Mon Sep 17 00:00:00 2001 From: Sonny Date: Wed, 8 Jan 2025 14:14:12 +0100 Subject: [PATCH 24/42] connection: Fix race condition when socket closes after timeout (#1048) --- packages/connection/index.js | 30 ++++++++++++++++++++---- packages/connection/test/stop.js | 30 ++++++++++++++++++++++++ test/client.js | 40 ++++++++++++++++++++++++++++++++ 3 files changed, 95 insertions(+), 5 deletions(-) diff --git a/packages/connection/index.js b/packages/connection/index.js index 3f631860..7dc6e932 100644 --- a/packages/connection/index.js +++ b/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; } diff --git a/packages/connection/test/stop.js b/packages/connection/test/stop.js index 5057068b..b82b0940 100644 --- a/packages/connection/test/stop.js +++ b/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); +}); diff --git a/test/client.js b/test/client.js index 167eb1f4..2b6f3c4e 100644 --- a/test/client.js +++ b/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); From d865571a621e14752f5a282445bb4375c0b4458e Mon Sep 17 00:00:00 2001 From: Sonny Piers Date: Wed, 8 Jan 2025 14:37:41 +0100 Subject: [PATCH 25/42] connection: Rename disconect() to _closeSocket() Users keep using it expecting it to be a public method even though it is not documented * https://github.com/xmppjs/xmpp.js/issues/1013 * https://github.com/xmppjs/xmpp.js/commit/b25e47892865c83164bac669f04facc2587f1bf3 * https://github.com/xmppjs/xmpp.js/issues/974 I guess that's fair, I'll create a new `disconnect` method that is safe to use. --- packages/connection/index.js | 4 ++-- .../test/{disconnect.js => _closeSocket.js} | 8 ++++---- packages/connection/test/end.js | 20 +++++++++---------- test/client.js | 2 +- 4 files changed, 17 insertions(+), 17 deletions(-) rename packages/connection/test/{disconnect.js => _closeSocket.js} (88%) diff --git a/packages/connection/index.js b/packages/connection/index.js index 7dc6e932..0b138fbe 100644 --- a/packages/connection/index.js +++ b/packages/connection/index.js @@ -218,7 +218,7 @@ class Connection extends EventEmitter { } catch {} try { - await this.disconnect(); + await this._closeSocket(); } catch (err) { this.#onSocketClosed(true, err); } @@ -262,7 +262,7 @@ class Connection extends EventEmitter { * https://xmpp.org/rfcs/rfc6120.html#streams-close * https://tools.ietf.org/html/rfc7395#section-3.6 */ - async disconnect(timeout = this.timeout) { + async _closeSocket(timeout = this.timeout) { if (!this.socket) return; this._status("disconnecting"); diff --git a/packages/connection/test/disconnect.js b/packages/connection/test/_closeSocket.js similarity index 88% rename from packages/connection/test/disconnect.js rename to packages/connection/test/_closeSocket.js index ae6f55b7..f30c9097 100644 --- a/packages/connection/test/disconnect.js +++ b/packages/connection/test/_closeSocket.js @@ -6,7 +6,7 @@ test("rejects with TimeoutError if socket doesn't close", (done) => { const conn = new Connection(); conn.socket = new EventEmitter(); conn.socket.end = () => {}; - conn.disconnect().catch((err) => { + conn._closeSocket().catch((err) => { expect(err.name).toBe("TimeoutError"); done(); }); @@ -21,7 +21,7 @@ test("resolves", (done) => { sock.emit("connect"); sock.end = () => {}; // eslint-disable-next-line promise/catch-or-return - conn.disconnect().then(() => { + conn._closeSocket().then(() => { expect(conn.status).toBe("disconnect"); return done(); }); @@ -41,7 +41,7 @@ test("rejects if socket.end throws", (done) => { throw error; }; - conn.disconnect().catch((err) => { + conn._closeSocket().catch((err) => { expect(err).toBe(error); done(); }); @@ -51,5 +51,5 @@ test("resolves if socket is absent", async () => { const conn = new Connection(); conn.socket = null; - await expect(conn.disconnect()).toResolve(); + await expect(conn._closeSocket()).toResolve(); }); diff --git a/packages/connection/test/end.js b/packages/connection/test/end.js index 1feececa..6616b841 100644 --- a/packages/connection/test/end.js +++ b/packages/connection/test/end.js @@ -4,12 +4,12 @@ test("#_end", async () => { const conn = new Connection(); const spy_close = jest.spyOn(conn, "close"); - const spy_disconnect = jest.spyOn(conn, "disconnect"); + const spy_closeSocket = jest.spyOn(conn, "_closeSocket"); await conn._end(); expect(spy_close).toHaveBeenCalledTimes(1); - expect(spy_disconnect).toHaveBeenCalledTimes(1); + expect(spy_closeSocket).toHaveBeenCalledTimes(1); }); test("#_end with close rejection", async () => { @@ -18,20 +18,20 @@ test("#_end with close rejection", async () => { const spy_close = jest.spyOn(conn, "close").mockImplementation(() => { return Promise.reject(); }); - const spy_disconnect = jest.spyOn(conn, "disconnect"); + const spy_closeSocket = jest.spyOn(conn, "_closeSocket"); await conn._end(); expect(spy_close).toHaveBeenCalledTimes(1); - expect(spy_disconnect).toHaveBeenCalledTimes(1); + expect(spy_closeSocket).toHaveBeenCalledTimes(1); }); test("#_end with disconnect rejection", async () => { const conn = new Connection(); const spy_close = jest.spyOn(conn, "close"); - const spy_disconnect = jest - .spyOn(conn, "disconnect") + const spy_closeSocket = jest + .spyOn(conn, "_closeSocket") .mockImplementation(() => { return Promise.reject(); }); @@ -39,7 +39,7 @@ test("#_end with disconnect rejection", async () => { await conn._end(); expect(spy_close).toHaveBeenCalledTimes(1); - expect(spy_disconnect).toHaveBeenCalledTimes(1); + expect(spy_closeSocket).toHaveBeenCalledTimes(1); }); test("#_end with close and disconnect rejection", async () => { @@ -48,8 +48,8 @@ test("#_end with close and disconnect rejection", async () => { const spy_close = jest.spyOn(conn, "close").mockImplementation(() => { return Promise.reject(); }); - const spy_disconnect = jest - .spyOn(conn, "disconnect") + const spy_closeSocket = jest + .spyOn(conn, "_closeSocket") .mockImplementation(() => { return Promise.reject(); }); @@ -57,5 +57,5 @@ test("#_end with close and disconnect rejection", async () => { await conn._end(); expect(spy_close).toHaveBeenCalledTimes(1); - expect(spy_disconnect).toHaveBeenCalledTimes(1); + expect(spy_closeSocket).toHaveBeenCalledTimes(1); }); diff --git a/test/client.js b/test/client.js index 2b6f3c4e..5adf4786 100644 --- a/test/client.js +++ b/test/client.js @@ -169,7 +169,7 @@ test("statuses", async () => { ]); // trigger reconnect - await xmpp.disconnect(); + await xmpp._closeSocket(); statuses = [xmpp.status]; await promise(xmpp, "open"); From 317796945e76166434fd81894ae337d02d418a2d Mon Sep 17 00:00:00 2001 From: Sonny Piers Date: Wed, 8 Jan 2025 15:18:43 +0100 Subject: [PATCH 26/42] connection: Rename close() to _closeStream() See 2877a7b4c240150f66bceac5946d7d772c81f0e7 --- packages/connection/index.js | 4 +-- .../test/{close.js => _closeStream.js} | 10 +++---- packages/connection/test/end.js | 28 +++++++++++-------- packages/connection/test/stop.js | 16 +++++------ packages/tls/test.js | 6 ++-- 5 files changed, 34 insertions(+), 30 deletions(-) rename packages/connection/test/{close.js => _closeStream.js} (91%) diff --git a/packages/connection/index.js b/packages/connection/index.js index 0b138fbe..95b12a7a 100644 --- a/packages/connection/index.js +++ b/packages/connection/index.js @@ -214,7 +214,7 @@ class Connection extends EventEmitter { async _end() { let el; try { - el = await this.close(); + el = await this._closeStream(); } catch {} try { @@ -326,7 +326,7 @@ class Connection extends EventEmitter { * https://xmpp.org/rfcs/rfc6120.html#streams-close * https://tools.ietf.org/html/rfc7395#section-3.6 */ - async close(timeout = this.timeout) { + async _closeStream(timeout = this.timeout) { const fragment = this.footer(this.footerElement()); const p = Promise.all([ diff --git a/packages/connection/test/close.js b/packages/connection/test/_closeStream.js similarity index 91% rename from packages/connection/test/close.js rename to packages/connection/test/_closeStream.js index 30c9b24d..53d5fce5 100644 --- a/packages/connection/test/close.js +++ b/packages/connection/test/_closeStream.js @@ -28,7 +28,7 @@ test("timeout", async () => { expect(el).toBe(""); }); - await expect(conn.close()).rejects.toThrow(new TimeoutError()); + await expect(conn._closeStream()).rejects.toThrow(new TimeoutError()); }); test("error on status closing", async () => { @@ -47,7 +47,7 @@ test("error on status closing", async () => { conn.parser.emit("end"); - await expect(conn.close()).rejects.toThrow( + await expect(conn._closeStream()).rejects.toThrow( new Error("Connection is closing"), ); }); @@ -69,7 +69,7 @@ test("resolves", async () => { expect(el).toBe(""); }); - const promiseClose = conn.close(); + const promiseClose = conn._closeStream(); conn.parser.emit("end", xml("goodbye")); const el = await promiseClose; @@ -91,7 +91,7 @@ test("emits closing status", () => { const p = Promise.all([ promise(conn, "status").then((status) => expect(status).toBe("closing")), - conn.close(), + conn._closeStream(), ]); conn.parser.emit("end"); @@ -115,6 +115,6 @@ test("do not emit closing status if parser property is missing", async () => { expect(timeout(promise(conn, "status"), 500)).rejects.toThrow( new TimeoutError(), ), - expect(conn.close()).rejects.toThrow(), + expect(conn._closeStream()).rejects.toThrow(), ]); }); diff --git a/packages/connection/test/end.js b/packages/connection/test/end.js index 6616b841..87ada10b 100644 --- a/packages/connection/test/end.js +++ b/packages/connection/test/end.js @@ -3,33 +3,35 @@ import Connection from "../index.js"; test("#_end", async () => { const conn = new Connection(); - const spy_close = jest.spyOn(conn, "close"); + const spy_closeStream = jest.spyOn(conn, "_closeStream"); const spy_closeSocket = jest.spyOn(conn, "_closeSocket"); await conn._end(); - expect(spy_close).toHaveBeenCalledTimes(1); + expect(spy_closeStream).toHaveBeenCalledTimes(1); expect(spy_closeSocket).toHaveBeenCalledTimes(1); }); test("#_end with close rejection", async () => { const conn = new Connection(); - const spy_close = jest.spyOn(conn, "close").mockImplementation(() => { - return Promise.reject(); - }); + const spy_closeStream = jest + .spyOn(conn, "_closeStream") + .mockImplementation(() => { + return Promise.reject(); + }); const spy_closeSocket = jest.spyOn(conn, "_closeSocket"); await conn._end(); - expect(spy_close).toHaveBeenCalledTimes(1); + expect(spy_closeStream).toHaveBeenCalledTimes(1); expect(spy_closeSocket).toHaveBeenCalledTimes(1); }); test("#_end with disconnect rejection", async () => { const conn = new Connection(); - const spy_close = jest.spyOn(conn, "close"); + const spy_closeStream = jest.spyOn(conn, "_closeStream"); const spy_closeSocket = jest .spyOn(conn, "_closeSocket") .mockImplementation(() => { @@ -38,16 +40,18 @@ test("#_end with disconnect rejection", async () => { await conn._end(); - expect(spy_close).toHaveBeenCalledTimes(1); + expect(spy_closeStream).toHaveBeenCalledTimes(1); expect(spy_closeSocket).toHaveBeenCalledTimes(1); }); test("#_end with close and disconnect rejection", async () => { const conn = new Connection(); - const spy_close = jest.spyOn(conn, "close").mockImplementation(() => { - return Promise.reject(); - }); + const spy_closeStream = jest + .spyOn(conn, "_closeStream") + .mockImplementation(() => { + return Promise.reject(); + }); const spy_closeSocket = jest .spyOn(conn, "_closeSocket") .mockImplementation(() => { @@ -56,6 +60,6 @@ test("#_end with close and disconnect rejection", async () => { await conn._end(); - expect(spy_close).toHaveBeenCalledTimes(1); + expect(spy_closeStream).toHaveBeenCalledTimes(1); expect(spy_closeSocket).toHaveBeenCalledTimes(1); }); diff --git a/packages/connection/test/stop.js b/packages/connection/test/stop.js index b82b0940..28c376d1 100644 --- a/packages/connection/test/stop.js +++ b/packages/connection/test/stop.js @@ -9,18 +9,18 @@ test("resolves if socket property is undefined", async () => { expect().pass(); }); -test("resolves if close rejects", async () => { +test("resolves if _closeStream rejects", async () => { const conn = new Connection(); - conn.close = () => Promise.reject(); - conn.disconnect = () => Promise.resolve(); + conn._closeStream = () => Promise.reject(); + conn._closeSocket = () => Promise.resolve(); await conn.stop(); expect().pass(); }); -test("resolves if disconnect rejects", async () => { +test("resolves if _closeSocket rejects", async () => { const conn = new Connection(); - conn.disconnect = () => Promise.reject(); - conn.close = () => Promise.resolve(); + conn._closeStream = () => Promise.resolve(); + conn._closeSocket = () => Promise.reject(); await conn.stop(); expect().pass(); }); @@ -29,8 +29,8 @@ test("resolves with the result of close", async () => { const conn = new Connection(); conn.socket = {}; const el = {}; - conn.close = () => Promise.resolve(el); - conn.disconnect = () => Promise.resolve(); + conn._closeStream = () => Promise.resolve(el); + conn._closeSocket = () => Promise.resolve(); expect(await conn.stop()).toBe(el); }); diff --git a/packages/tls/test.js b/packages/tls/test.js index 06443562..006ecf41 100644 --- a/packages/tls/test.js +++ b/packages/tls/test.js @@ -90,7 +90,7 @@ NebQHyTBqa5P7vjSioiWiSRCNOIL4HywMWtN/nZVk0cl8zwlLtMaGt9Tz7ty2OgL const error = await promise(conn, "error"); expect(error.message).toBe("certificate has expired"); - await conn.close().catch(() => {}); + await conn._closeSocket().catch(() => {}); server.close(); }); @@ -111,7 +111,7 @@ test("rejects self signed certificates", async () => { const error = await promise(conn, "error"); expect(error.code).toBe("DEPTH_ZERO_SELF_SIGNED_CERT"); - await conn.close().catch(() => {}); + await conn._closeSocket().catch(() => {}); server.close(); }); @@ -213,6 +213,6 @@ yA== // which is what we want as it delays the sending of the stream header (conn.open) expect(connect_emitted_on_conn_socket).toBe(true); - await conn.close().catch(() => {}); + await conn._closeSocket().catch(() => {}); server.close(); }); From 96be425020064d56ef7dc47dcf1e27ff2293a396 Mon Sep 17 00:00:00 2001 From: Sonny Piers Date: Wed, 8 Jan 2025 15:50:07 +0100 Subject: [PATCH 27/42] connection: Make disconnect a public function See * 2877a7b4c240150f66bceac5946d7d772c81f0e7 * 31bb31c5a9da70ac396cbe14cd108c7ab54cb15d --- packages/connection/index.js | 13 +++-- .../connection/test/{end.js => disconnect.js} | 35 ++++++++++---- packages/connection/test/onElement.js | 4 +- packages/connection/test/stop.js | 48 ++++++------------- packages/connection/test/streamError.js | 4 +- test/client.js | 2 +- 6 files changed, 54 insertions(+), 52 deletions(-) rename packages/connection/test/{end.js => disconnect.js} (61%) diff --git a/packages/connection/index.js b/packages/connection/index.js index 95b12a7a..008d57a2 100644 --- a/packages/connection/index.js +++ b/packages/connection/index.js @@ -38,7 +38,7 @@ class Connection extends EventEmitter { ); } catch {} - return this._end(); + return this.disconnect(); } _onData(data) { @@ -107,7 +107,7 @@ class Connection extends EventEmitter { if (isStreamError) { // "Stream Errors Are Unrecoverable" // "The entity that receives the stream error then SHALL close the stream" - this._end(); + this.disconnect(); } } @@ -211,15 +211,18 @@ class Connection extends EventEmitter { } } - async _end() { + async disconnect() { let el; try { el = await this._closeStream(); - } catch {} + } catch (err) { + console.log(err); + } try { await this._closeSocket(); } catch (err) { + console.log(err); this.#onSocketClosed(true, err); } @@ -316,7 +319,7 @@ class Connection extends EventEmitter { * https://tools.ietf.org/html/rfc7395#section-3.6 */ async stop() { - const el = await this._end(); + const el = await this.disconnect(); this._status("offline", el); return el; } diff --git a/packages/connection/test/end.js b/packages/connection/test/disconnect.js similarity index 61% rename from packages/connection/test/end.js rename to packages/connection/test/disconnect.js index 87ada10b..4c50d349 100644 --- a/packages/connection/test/end.js +++ b/packages/connection/test/disconnect.js @@ -1,18 +1,21 @@ import Connection from "../index.js"; -test("#_end", async () => { +test("disconnect", async () => { const conn = new Connection(); - const spy_closeStream = jest.spyOn(conn, "_closeStream"); + const el = {}; + const spy_closeStream = jest + .spyOn(conn, "_closeStream") + .mockImplementation(async () => el); const spy_closeSocket = jest.spyOn(conn, "_closeSocket"); - await conn._end(); + expect(await conn.disconnect()).toBe(el); expect(spy_closeStream).toHaveBeenCalledTimes(1); expect(spy_closeSocket).toHaveBeenCalledTimes(1); }); -test("#_end with close rejection", async () => { +test("disconnect with _closeStream rejection", async () => { const conn = new Connection(); const spy_closeStream = jest @@ -22,13 +25,13 @@ test("#_end with close rejection", async () => { }); const spy_closeSocket = jest.spyOn(conn, "_closeSocket"); - await conn._end(); + await conn.disconnect(); expect(spy_closeStream).toHaveBeenCalledTimes(1); expect(spy_closeSocket).toHaveBeenCalledTimes(1); }); -test("#_end with disconnect rejection", async () => { +test("disconnect with _closeSocket rejection", async () => { const conn = new Connection(); const spy_closeStream = jest.spyOn(conn, "_closeStream"); @@ -38,13 +41,13 @@ test("#_end with disconnect rejection", async () => { return Promise.reject(); }); - await conn._end(); + await conn.disconnect(); expect(spy_closeStream).toHaveBeenCalledTimes(1); expect(spy_closeSocket).toHaveBeenCalledTimes(1); }); -test("#_end with close and disconnect rejection", async () => { +test("disconnect with _closeStream and _closeSocket rejections", async () => { const conn = new Connection(); const spy_closeStream = jest @@ -58,8 +61,22 @@ test("#_end with close and disconnect rejection", async () => { return Promise.reject(); }); - await conn._end(); + await conn.disconnect(); expect(spy_closeStream).toHaveBeenCalledTimes(1); expect(spy_closeSocket).toHaveBeenCalledTimes(1); }); + +test("resolves if socket property is undefined", async () => { + const conn = new Connection(); + conn.footerElement = () => ; + conn.socket = undefined; + await conn.disconnect(); + expect().pass(); +}); + +test("does not reject if connection is not established", async () => { + const conn = new Connection(); + await conn.disconnect(); + expect().pass(); +}); diff --git a/packages/connection/test/onElement.js b/packages/connection/test/onElement.js index c6e558a3..e0001fb2 100644 --- a/packages/connection/test/onElement.js +++ b/packages/connection/test/onElement.js @@ -27,10 +27,10 @@ test("#_onElement stream:error", (done) => { application, ]); const conn = new Connection(); - conn._end = () => { + jest.spyOn(conn, "disconnect").mockImplementation(() => { done(); return Promise.resolve(); - }; + }); conn.on("element", (el) => { expect(el).toBe(foo); diff --git a/packages/connection/test/stop.js b/packages/connection/test/stop.js index 28c376d1..8a94a49a 100644 --- a/packages/connection/test/stop.js +++ b/packages/connection/test/stop.js @@ -1,43 +1,25 @@ -import Connection from "../index.js"; import { EventEmitter } from "@xmpp/events"; +import Connection from "../index.js"; -test("resolves if socket property is undefined", async () => { +test("stop", async () => { const conn = new Connection(); - conn.footerElement = () => ; - conn.socket = undefined; - await conn.stop(); - expect().pass(); -}); -test("resolves if _closeStream rejects", async () => { - const conn = new Connection(); - conn._closeStream = () => Promise.reject(); - conn._closeSocket = () => Promise.resolve(); - await conn.stop(); - expect().pass(); -}); + const close_el = {}; + const spy_disconnect = jest + .spyOn(conn, "disconnect") + .mockImplementation(async () => { + return close_el; + }); + const spy_status = jest.spyOn(conn, "_status"); -test("resolves if _closeSocket rejects", async () => { - const conn = new Connection(); - conn._closeStream = () => Promise.resolve(); - conn._closeSocket = () => Promise.reject(); - await conn.stop(); - expect().pass(); -}); + conn.status = "online"; -test("resolves with the result of close", async () => { - const conn = new Connection(); - conn.socket = {}; - const el = {}; - conn._closeStream = () => Promise.resolve(el); - conn._closeSocket = () => Promise.resolve(); - expect(await conn.stop()).toBe(el); -}); - -test("does not throw if connection is not established", async () => { - const conn = new Connection(); await conn.stop(); - expect().pass(); + + expect(spy_disconnect).toHaveBeenCalledTimes(1); + expect(spy_status).toHaveBeenCalledTimes(1); + expect(spy_status).toHaveBeenCalledWith("offline", close_el); + expect(conn.status).toBe("offline"); }); // https://github.com/xmppjs/xmpp.js/issues/956 diff --git a/packages/connection/test/streamError.js b/packages/connection/test/streamError.js index 79d8967a..5b0fc3e9 100644 --- a/packages/connection/test/streamError.js +++ b/packages/connection/test/streamError.js @@ -4,12 +4,12 @@ import xml from "@xmpp/xml"; test("#_streamError", async () => { const conn = new Connection(); - const spy_end = jest.spyOn(conn, "_end"); + const spy_disconnect = jest.spyOn(conn, "disconnect"); const spy_send = jest.spyOn(conn, "send"); await conn._streamError("foo-bar"); - expect(spy_end).toHaveBeenCalled(); + expect(spy_disconnect).toHaveBeenCalled(); expect(spy_send).toHaveBeenCalledWith( xml("stream:error", {}, [ diff --git a/test/client.js b/test/client.js index 5adf4786..2b6f3c4e 100644 --- a/test/client.js +++ b/test/client.js @@ -169,7 +169,7 @@ test("statuses", async () => { ]); // trigger reconnect - await xmpp._closeSocket(); + await xmpp.disconnect(); statuses = [xmpp.status]; await promise(xmpp, "open"); From a8298f7c90b05e98726d783ee80cb1bb7b55704a Mon Sep 17 00:00:00 2001 From: Sonny Piers Date: Wed, 8 Jan 2025 17:25:58 +0100 Subject: [PATCH 28/42] Simplify status checks --- packages/client/README.md | 21 ++++++-- packages/component/README.md | 21 ++++++-- packages/connection/index.js | 35 ++++++------- packages/connection/test/_closeSocket.js | 7 --- packages/connection/test/_closeStream.js | 63 +++--------------------- packages/connection/test/socketClose.js | 10 ++-- packages/resolve/index.js | 2 +- packages/tls/test.js | 6 +-- test/client.js | 5 +- test/component.js | 8 +-- 10 files changed, 69 insertions(+), 109 deletions(-) diff --git a/packages/client/README.md b/packages/client/README.md index 1a5191f8..12eda4b8 100644 --- a/packages/client/README.md +++ b/packages/client/README.md @@ -204,33 +204,44 @@ xmpp.on("offline", () => { Starts the connection. Attempts to reconnect will automatically happen if it cannot connect or gets disconnected. ```js -xmpp.start().catch(console.error); xmpp.on("online", (address) => { console.log("online", address.toString()); }); +await xmpp.start(); ``` Returns a promise that resolves if the first attempt succeed or rejects if the first attempt fails. ### stop -Stops the connection and prevent any further auto reconnect/retry. +Stops the connection and prevent any further auto reconnect. ```js -xmpp.stop().catch(console.error); xmpp.on("offline", () => { console.log("offline"); }); +await xmpp.stop(); ``` Returns a promise that resolves once the stream closes and the socket disconnects. +### disconnect + +Like [`stop`](#stop) but will not prevent auto reconnect. + +```js +xmpp.on("disconnect", () => { + console.log("disconnect"); +}); +await xmpp.disconnect(); +``` + ### send Sends a stanza. ```js -xmpp.send(xml("presence")).catch(console.error); +await xmpp.send(xml("presence")); ``` Returns a promise that resolves once the stanza is serialized and written to the socket or rejects if any of those fails. @@ -247,7 +258,7 @@ const recipients = ["romeo@example.com", "juliet@example.com"]; const stanzas = recipients.map((address) => xml("message", { to: address, type: "chat" }, xml("body", null, message)), ); -xmpp.sendMany(stanzas).catch(console.error); +await xmpp.sendMany(stanzas); ``` Returns a promise that resolves once all the stanzas have been sent. diff --git a/packages/component/README.md b/packages/component/README.md index 4a1fbb93..43b26b87 100644 --- a/packages/component/README.md +++ b/packages/component/README.md @@ -170,23 +170,34 @@ xmpp.on("offline", () => { Starts the connection. Attempts to reconnect will automatically happen if it cannot connect or gets disconnected. ```js -xmpp.start().catch(console.error); xmpp.on("online", (address) => { console.log("online", address.toString()); }); +await xmpp.start(); ``` Returns a promise that resolves if the first attempt succeed or rejects if the first attempt fails. ### stop -Stops the connection and prevent any further auto reconnect/retry. +Stops the connection and prevent any further auto reconnect. ```js -xmpp.stop().catch(console.error); xmpp.on("offline", () => { console.log("offline"); }); +await xmpp.stop(); +``` + +### disconnect + +Like [`stop`](#stop) but will not prevent auto reconnect. + +```js +xmpp.on("disconnect", () => { + console.log("disconnect"); +}); +await xmpp.disconnect(); ``` Returns a promise that resolves once the stream closes and the socket disconnects. @@ -196,7 +207,7 @@ Returns a promise that resolves once the stream closes and the socket disconnect Sends a stanza. ```js -xmpp.send(xml("presence")).catch(console.error); +await xmpp.send(xml("presence")); ``` Returns a promise that resolves once the stanza is serialized and written to the socket or rejects if any of those fails. @@ -213,7 +224,7 @@ const recipients = ["romeo@example.com", "juliet@example.com"]; const stanzas = recipients.map((address) => xml("message", { to: address, type: "chat" }, xml("body", null, message)), ); -xmpp.sendMany(stanzas).catch(console.error); +await xmpp.sendMany(stanzas); ``` Returns a promise that resolves once all the stanzas have been sent. diff --git a/packages/connection/index.js b/packages/connection/index.js index 008d57a2..c74b0233 100644 --- a/packages/connection/index.js +++ b/packages/connection/index.js @@ -21,13 +21,6 @@ class Connection extends EventEmitter { this.root = null; } - _reset() { - this.status = "offline"; - this._detachSocket(); - this._detachParser(); - this.root = null; - } - async _streamError(condition, children) { try { await this.send( @@ -57,9 +50,14 @@ class Connection extends EventEmitter { this.emit("error", error); } - #onSocketClosed(dirty, event) { - this._reset(); - this._status("disconnect", { clean: !dirty, event }); + #onSocketClosed(dirty, reason) { + this._detachSocket(); + this._status("disconnect", { clean: !dirty, reason }); + } + + #onStreamClosed(dirty, reason) { + this._detachParser(); + this._status("close", { clean: !dirty, reason }); } _attachSocket(socket) { @@ -91,7 +89,6 @@ class Connection extends EventEmitter { delete socketListeners[k]; } this.socket = null; - return socket; } _onElement(element) { @@ -151,10 +148,7 @@ class Connection extends EventEmitter { listeners.element = this._onElement.bind(this); listeners.error = this._onParserError.bind(this); - listeners.end = (element) => { - this._detachParser(); - this._status("close", element); - }; + listeners.end = this.#onStreamClosed.bind(this); listeners.start = (element) => { this._status("open", element); @@ -173,6 +167,7 @@ class Connection extends EventEmitter { delete listeners[k]; } this.parser = null; + this.root = null; } _jid(id) { @@ -213,16 +208,16 @@ class Connection extends EventEmitter { async disconnect() { let el; + try { el = await this._closeStream(); } catch (err) { - console.log(err); + this.#onStreamClosed(err); } try { await this._closeSocket(); } catch (err) { - console.log(err); this.#onSocketClosed(true, err); } @@ -266,8 +261,6 @@ class Connection extends EventEmitter { * https://tools.ietf.org/html/rfc7395#section-3.6 */ async _closeSocket(timeout = this.timeout) { - if (!this.socket) return; - this._status("disconnecting"); this.socket.end(); @@ -337,9 +330,9 @@ class Connection extends EventEmitter { this.write(fragment), ]); - if (this.parser && this.socket) this._status("closing"); + this._status("closing"); + const [el] = await p; - this.root = null; return el; // The 'close' status is set by the parser 'end' listener } diff --git a/packages/connection/test/_closeSocket.js b/packages/connection/test/_closeSocket.js index f30c9097..7ab0f4e3 100644 --- a/packages/connection/test/_closeSocket.js +++ b/packages/connection/test/_closeSocket.js @@ -46,10 +46,3 @@ test("rejects if socket.end throws", (done) => { done(); }); }); - -test("resolves if socket is absent", async () => { - const conn = new Connection(); - conn.socket = null; - - await expect(conn._closeSocket()).toResolve(); -}); diff --git a/packages/connection/test/_closeStream.js b/packages/connection/test/_closeStream.js index 53d5fce5..e8eefdfc 100644 --- a/packages/connection/test/_closeStream.js +++ b/packages/connection/test/_closeStream.js @@ -1,5 +1,5 @@ import Connection from "../index.js"; -import { EventEmitter, promise, timeout, TimeoutError } from "@xmpp/events"; +import { EventEmitter, promise, TimeoutError } from "@xmpp/events"; import { xml } from "@xmpp/test"; test("resets properties on socket close event", () => { @@ -11,22 +11,11 @@ test("resets properties on socket close event", () => { expect(conn.status).toBe("disconnect"); }); -test("timeout", async () => { - expect.assertions(2); +test("timeout on parser end", async () => { const conn = new Connection(); conn.parser = new EventEmitter(); - conn.footerElement = () => { - return xml("hello"); - }; - - conn.socket = new EventEmitter(); - conn.socket.write = (data, cb) => { - return cb(); - }; - - conn.on("output", (el) => { - expect(el).toBe(""); - }); + jest.spyOn(conn, "footerElement").mockImplementation(() => xml("hello")); + jest.spyOn(conn, "write").mockImplementation(async () => {}); await expect(conn._closeStream()).rejects.toThrow(new TimeoutError()); }); @@ -53,21 +42,11 @@ test("error on status closing", async () => { }); test("resolves", async () => { - expect.assertions(2); const conn = new Connection(); conn.parser = new EventEmitter(); - conn.footerElement = () => { - return xml("hello"); - }; - conn.socket = new EventEmitter(); - conn.socket.write = (data, cb) => { - return cb(); - }; - - conn.on("output", (el) => { - expect(el).toBe(""); - }); + jest.spyOn(conn, "footerElement").mockImplementation(() => xml("hello")); + jest.spyOn(conn, "write").mockImplementation(async () => {}); const promiseClose = conn._closeStream(); conn.parser.emit("end", xml("goodbye")); @@ -80,14 +59,9 @@ test("resolves", async () => { test("emits closing status", () => { const conn = new Connection(); conn.parser = new EventEmitter(); - conn.footerElement = () => { - return xml("hello"); - }; - conn.socket = new EventEmitter(); - conn.socket.write = (data, cb) => { - return cb(); - }; + jest.spyOn(conn, "footerElement").mockImplementation(() => xml("hello")); + jest.spyOn(conn, "write").mockImplementation(async () => {}); const p = Promise.all([ promise(conn, "status").then((status) => expect(status).toBe("closing")), @@ -97,24 +71,3 @@ test("emits closing status", () => { conn.parser.emit("end"); return p; }); - -test("do not emit closing status if parser property is missing", async () => { - expect.assertions(2); - const conn = new Connection(); - conn.parser = null; - conn.footerElement = () => { - return xml("hello"); - }; - - conn.socket = new EventEmitter(); - conn.socket.write = (data, cb) => { - return cb(); - }; - - await Promise.all([ - expect(timeout(promise(conn, "status"), 500)).rejects.toThrow( - new TimeoutError(), - ), - expect(conn._closeStream()).rejects.toThrow(), - ]); -}); diff --git a/packages/connection/test/socketClose.js b/packages/connection/test/socketClose.js index 3ff56c1a..4b56fbe7 100644 --- a/packages/connection/test/socketClose.js +++ b/packages/connection/test/socketClose.js @@ -1,21 +1,21 @@ import Connection from "../index.js"; import { EventEmitter } from "@xmpp/events"; -test("calls _reset and _status", () => { +test("calls _detachSocket and _status", () => { expect.assertions(3); const conn = new Connection(); const sock = new EventEmitter(); conn._attachSocket(sock); const evt = {}; - conn._status = (status, { clean, event }) => { + conn._status = (status, { clean, reason }) => { expect(clean).toBe(false); - expect(event).toBe(evt); + expect(reason).toBe(evt); }; - const spy_reset = jest.spyOn(conn, "_reset"); + const spy_detachSocket = jest.spyOn(conn, "_detachSocket"); sock.emit("close", true, evt); - expect(spy_reset).toHaveBeenCalled(); + expect(spy_detachSocket).toHaveBeenCalled(); }); diff --git a/packages/resolve/index.js b/packages/resolve/index.js index fb5a2777..ddeca0e6 100644 --- a/packages/resolve/index.js +++ b/packages/resolve/index.js @@ -71,7 +71,7 @@ export default function resolve({ entity }) { try { await fallbackConnect(entity, uris); } catch (err) { - entity._reset(); + await entity.disconnect(); entity._status("disconnect"); throw err; } diff --git a/packages/tls/test.js b/packages/tls/test.js index 006ecf41..82097918 100644 --- a/packages/tls/test.js +++ b/packages/tls/test.js @@ -90,7 +90,7 @@ NebQHyTBqa5P7vjSioiWiSRCNOIL4HywMWtN/nZVk0cl8zwlLtMaGt9Tz7ty2OgL const error = await promise(conn, "error"); expect(error.message).toBe("certificate has expired"); - await conn._closeSocket().catch(() => {}); + await conn.disconnect(); server.close(); }); @@ -111,7 +111,7 @@ test("rejects self signed certificates", async () => { const error = await promise(conn, "error"); expect(error.code).toBe("DEPTH_ZERO_SELF_SIGNED_CERT"); - await conn._closeSocket().catch(() => {}); + await conn.disconnect(); server.close(); }); @@ -213,6 +213,6 @@ yA== // which is what we want as it delays the sending of the stream header (conn.open) expect(connect_emitted_on_conn_socket).toBe(true); - await conn._closeSocket().catch(() => {}); + await conn.disconnect(); server.close(); }); diff --git a/test/client.js b/test/client.js index 2b6f3c4e..457f180f 100644 --- a/test/client.js +++ b/test/client.js @@ -17,6 +17,7 @@ beforeEach(async () => { }); afterEach(async () => { + xmpp?.removeAllListeners(); await xmpp?.stop(); }); @@ -89,7 +90,6 @@ test("reconnects when server restarts gracefully", (done) => { c++; expect().pass(); if (c === 2) { - await xmpp.stop(); done(); } else { await server.restart(); @@ -112,7 +112,6 @@ test("reconnects when server restarts non-gracefully", (done) => { c++; expect().pass(); if (c === 2) { - await xmpp.stop(); done(); } else { await server.restart("SIGKILL"); @@ -130,7 +129,7 @@ test("does not reconnect when stop is called", (done) => { xmpp.on("online", async () => { await xmpp.stop(); - server.stop(); + await server.stop(); done(); }); diff --git a/test/component.js b/test/component.js index aa922dc6..92f252f4 100644 --- a/test/component.js +++ b/test/component.js @@ -16,6 +16,7 @@ beforeEach(async () => { }); afterEach(async () => { + xmpp?.removeAllListeners(); await xmpp?.stop(); }); @@ -39,7 +40,6 @@ test("component", async () => { expect(id instanceof jid.JID).toBe(true); expect(id.toString()).toBe("component.localhost"); - await xmpp.stop; }); test("reconnects when server restarts", (done) => { @@ -52,10 +52,9 @@ test("reconnects when server restarts", (done) => { c++; expect().pass(); if (c === 2) { - await xmpp.stop(); done(); } else { - server.restart(); + await server.restart(); } }); @@ -63,11 +62,12 @@ test("reconnects when server restarts", (done) => { }); test("does not reconnect when stop is called", (done) => { - expect.assertions(2); + expect.assertions(3); xmpp.on("online", async () => { await xmpp.stop(); await server.stop(); + expect(xmpp.status).toBe("offline"); done(); }); From 9e2ff4b4ce2646d538943c3f0f6b06b27b7a86f8 Mon Sep 17 00:00:00 2001 From: Sonny Piers Date: Wed, 8 Jan 2025 20:38:43 +0100 Subject: [PATCH 29/42] reconnect: Simplify code --- packages/reconnect/index.js | 16 ++++++------ packages/reconnect/test.js | 49 ++++++++++++++++++------------------- 2 files changed, 31 insertions(+), 34 deletions(-) diff --git a/packages/reconnect/index.js b/packages/reconnect/index.js index 0c04cc94..644dcedd 100644 --- a/packages/reconnect/index.js +++ b/packages/reconnect/index.js @@ -9,6 +9,10 @@ class Reconnect extends EventEmitter { this._timeout = null; } + #onDisconnect = () => { + this.scheduleReconnect(); + }; + scheduleReconnect() { const { entity, delay, _timeout } = this; clearTimeout(_timeout); @@ -38,18 +42,12 @@ class Reconnect extends EventEmitter { start() { const { entity } = this; - const listeners = {}; - listeners.disconnect = () => { - this.scheduleReconnect(); - }; - - this.listeners = listeners; - entity.on("disconnect", listeners.disconnect); + entity.on("disconnect", this.#onDisconnect); } stop() { - const { entity, listeners, _timeout } = this; - entity.removeListener("disconnect", listeners.disconnect); + const { entity, _timeout } = this; + entity.removeListener("disconnect", this.#onDisconnect); clearTimeout(_timeout); } } diff --git a/packages/reconnect/test.js b/packages/reconnect/test.js index c7f325cb..6ace155d 100644 --- a/packages/reconnect/test.js +++ b/packages/reconnect/test.js @@ -1,38 +1,37 @@ import _reconnect from "./index.js"; -import { EventEmitter } from "@xmpp/events"; +// eslint-disable-next-line n/no-extraneous-import +import Connection from "@xmpp/connection"; -test("it schedule a reconnect when disconnect is emitted", (done) => { - const entity = new EventEmitter(); +test("schedules a reconnect when disconnect is emitted", () => { + const entity = new Connection(); const reconnect = _reconnect({ entity }); + const spy_scheduleReconnect = jest.spyOn(reconnect, "scheduleReconnect"); - reconnect.scheduleReconnect = () => { - expect.pass(); - done(); - }; - + expect(spy_scheduleReconnect).toHaveBeenCalledTimes(0); entity.emit("disconnect"); + expect(spy_scheduleReconnect).toHaveBeenCalledTimes(1); }); test("#reconnect", async () => { - expect.assertions(3); - - const entity = new EventEmitter(); + const service = "service"; + const lang = "lang"; + const domain = "domain"; + + const entity = new Connection({ + service, + lang, + domain, + }); const reconnect = _reconnect({ entity }); - entity.options = { - service: "service", - lang: "lang", - domain: "domain", - }; - - entity.connect = (service) => { - expect(service).toBe(entity.options.service); - }; - - entity.open = ({ domain, lang }) => { - expect(domain).toBe(entity.options.domain); - expect(lang).toBe(entity.options.lang); - }; + const spy_connect = jest.spyOn(entity, "connect").mockResolvedValue(); + const spy_open = jest.spyOn(entity, "open").mockResolvedValue(); await reconnect.reconnect(); + + expect(spy_connect).toHaveBeenCalledWith(service); + expect(spy_open).toHaveBeenCalledWith({ + domain, + lang, + }); }); From aa6b3c13dfbde750fd7c8be12a230f96e67457f1 Mon Sep 17 00:00:00 2001 From: Sonny Date: Thu, 9 Jan 2025 00:19:46 +0100 Subject: [PATCH 30/42] fast: Fix configurable saveToken and fetchToken (#1049) --- packages/client-core/src/fast/fast.js | 43 +++++++++------------ packages/client/lib/createOnAuthenticate.js | 2 +- 2 files changed, 20 insertions(+), 25 deletions(-) diff --git a/packages/client-core/src/fast/fast.js b/packages/client-core/src/fast/fast.js index 5af90be5..9b064b05 100644 --- a/packages/client-core/src/fast/fast.js +++ b/packages/client-core/src/fast/fast.js @@ -5,32 +5,31 @@ import SASLFactory from "saslmechanisms"; const NS = "urn:xmpp:fast:0"; -export default function fast({ sasl2 }, { saveToken, fetchToken } = {}) { +export default function fast({ sasl2 }) { const saslFactory = new SASLFactory(); - const fast = new EventEmitter(); - let token; - saveToken ??= async function saveToken(t) { - token = t; - }; - fetchToken ??= async function fetchToken() { - return token; - }; + const fast = new EventEmitter(); Object.assign(fast, { mechanism: null, mechanisms: [], - async saveToken() { + async saveToken(t) { + token = t; + }, + async fetchToken() { + return token; + }, + async save(token) { try { - await saveToken(); + await this.saveToken(token); } catch (err) { fast.emit("error", err); } }, - async fetchToken() { + async fetch() { try { - return await fetchToken(); + return this.fetchToken(); } catch (err) { fast.emit("error", err); } @@ -107,17 +106,13 @@ export default function fast({ sasl2 }, { saveToken, fetchToken } = {}) { }, async (element) => { if (element.is("token", NS)) { - try { - await saveToken({ - // The token is bound by the mechanism - // > Servers MUST bind tokens to the mechanism selected by the client in its original request, and reject attempts to use them with other mechanisms. - mechanism: fast.mechanism, - token: element.attrs.token, - expiry: element.attrs.expiry, - }); - } catch (err) { - fast.emit("error", err); - } + await fast.save({ + // The token is bound by the mechanism + // > Servers MUST bind tokens to the mechanism selected by the client in its original request, and reject attempts to use them with other mechanisms. + mechanism: fast.mechanism, + token: element.attrs.token, + expiry: element.attrs.expiry, + }); } }, ); diff --git a/packages/client/lib/createOnAuthenticate.js b/packages/client/lib/createOnAuthenticate.js index e248eb8f..907a180b 100644 --- a/packages/client/lib/createOnAuthenticate.js +++ b/packages/client/lib/createOnAuthenticate.js @@ -16,7 +16,7 @@ export default function createOnAuthenticate(credentials, userAgent) { return; } - credentials.token = await fast?.fetchToken?.(); + credentials.token = await fast?.fetch?.(); await authenticate(credentials, mechanisms[0], userAgent); }; From bfef1153fa32e5b4aac846ff46107932d0895917 Mon Sep 17 00:00:00 2001 From: Sonny Date: Thu, 9 Jan 2025 01:26:10 +0100 Subject: [PATCH 31/42] Remove bundlesize (#1050) --- Makefile | 2 +- e2e.config.cjs | 2 +- package-lock.json | 555 +----------------- package.json | 8 +- test/{browser.js => browser.test.js} | 0 test/bundlesize.js | 31 + test/{client.js => client.test.js} | 0 test/{component.js => component.test.js} | 0 test/{sasl.js => sasl.test.js} | 0 ...e-other-host.js => see-other-host.test.js} | 0 10 files changed, 37 insertions(+), 561 deletions(-) rename test/{browser.js => browser.test.js} (100%) create mode 100644 test/bundlesize.js rename test/{client.js => client.test.js} (100%) rename test/{component.js => component.test.js} (100%) rename test/{sasl.js => sasl.test.js} (100%) rename test/{see-other-host.js => see-other-host.test.js} (100%) diff --git a/Makefile b/Makefile index 9cdb14dd..89a77289 100644 --- a/Makefile +++ b/Makefile @@ -62,7 +62,7 @@ restart: ./server/ctl.js restart bundlesize: - npx bundlesize + node test/bundlesize.js bundle: npx rollup -c rollup.config.js diff --git a/e2e.config.cjs b/e2e.config.cjs index 71764ac7..1fd95065 100644 --- a/e2e.config.cjs +++ b/e2e.config.cjs @@ -2,6 +2,6 @@ /** @type {import('jest').Config} */ module.exports = { - testMatch: ["/test/*.js"], + testMatch: ["/test/*.test.js"], setupFilesAfterEnv: ["jest-extended/all"], }; diff --git a/package-lock.json b/package-lock.json index de35cabd..7fb3b8a7 100644 --- a/package-lock.json +++ b/package-lock.json @@ -20,7 +20,7 @@ "@rollup/plugin-terser": "^0.4.4", "babel-jest": "^29.7.0", "babel-plugin-jsx-pragmatic": "^1.0.2", - "bundlesize": "^0.18.2", + "bytes": "^3.1.2", "eslint": "^9.17.0", "eslint-config-prettier": "^9.1.0", "eslint-plugin-jest": "^28.10.0", @@ -4498,15 +4498,6 @@ "dev": true, "license": "ISC" }, - "node_modules/are-we-there-yet": { - "version": "1.1.7", - "dev": true, - "license": "ISC", - "dependencies": { - "delegates": "^1.0.0", - "readable-stream": "^2.0.6" - } - }, "node_modules/argparse": { "version": "1.0.10", "dev": true, @@ -4842,18 +4833,6 @@ "node": ">=8" } }, - "node_modules/brotli-size": { - "version": "0.1.0", - "dev": true, - "license": "MIT", - "dependencies": { - "duplexer": "^0.1.1", - "iltorb": "^2.4.3" - }, - "engines": { - "node": ">=0.12.0" - } - }, "node_modules/browserslist": { "version": "4.24.2", "dev": true, @@ -4909,28 +4888,6 @@ "url": "https://github.com/sponsors/sindresorhus" } }, - "node_modules/bundlesize": { - "version": "0.18.2", - "dev": true, - "license": "MIT", - "dependencies": { - "axios": "^1.6.2", - "brotli-size": "0.1.0", - "bytes": "^3.1.0", - "ci-env": "^1.4.0", - "commander": "^2.20.0", - "cosmiconfig": "^5.2.1", - "github-build": "^1.2.4", - "glob": "^7.1.4", - "gzip-size": "^4.0.0", - "prettycli": "^1.4.3" - }, - "bin": { - "bundlesize": "index.js", - "bundlesize-init": "src/init-status.js", - "bundlesize-pipe": "pipe.js" - } - }, "node_modules/byte-size": { "version": "8.1.1", "dev": true, @@ -4941,6 +4898,8 @@ }, "node_modules/bytes": { "version": "3.1.2", + "resolved": "https://registry.npmjs.org/bytes/-/bytes-3.1.2.tgz", + "integrity": "sha512-/Nf7TyzTx6S3yRJObOAV7956r8cr2+Oj8AC5dt8wSP3BQAoeX58NoHyCU8P8zGkNXStjTSi6fzO6F0pBdcYbEg==", "dev": true, "license": "MIT", "engines": { @@ -5015,36 +4974,6 @@ "url": "https://github.com/sponsors/isaacs" } }, - "node_modules/caller-callsite": { - "version": "2.0.0", - "dev": true, - "license": "MIT", - "dependencies": { - "callsites": "^2.0.0" - }, - "engines": { - "node": ">=4" - } - }, - "node_modules/caller-callsite/node_modules/callsites": { - "version": "2.0.0", - "dev": true, - "license": "MIT", - "engines": { - "node": ">=4" - } - }, - "node_modules/caller-path": { - "version": "2.0.0", - "dev": true, - "license": "MIT", - "dependencies": { - "caller-callsite": "^2.0.0" - }, - "engines": { - "node": ">=4" - } - }, "node_modules/callsites": { "version": "3.1.0", "dev": true, @@ -5146,11 +5075,6 @@ "node": ">=10" } }, - "node_modules/ci-env": { - "version": "1.17.0", - "dev": true, - "license": "MIT" - }, "node_modules/ci-info": { "version": "3.9.0", "dev": true, @@ -5355,14 +5279,6 @@ "node": ">= 0.12.0" } }, - "node_modules/code-point-at": { - "version": "1.1.0", - "dev": true, - "license": "MIT", - "engines": { - "node": ">=0.10.0" - } - }, "node_modules/collect-v8-coverage": { "version": "1.0.2", "dev": true, @@ -5797,20 +5713,6 @@ "dev": true, "license": "MIT" }, - "node_modules/cosmiconfig": { - "version": "5.2.1", - "dev": true, - "license": "MIT", - "dependencies": { - "import-fresh": "^2.0.0", - "is-directory": "^0.3.1", - "js-yaml": "^3.13.1", - "parse-json": "^4.0.0" - }, - "engines": { - "node": ">=4" - } - }, "node_modules/create-hash": { "version": "1.2.0", "license": "MIT", @@ -5969,25 +5871,6 @@ "dev": true, "license": "MIT" }, - "node_modules/decompress-response": { - "version": "4.2.1", - "dev": true, - "license": "MIT", - "dependencies": { - "mimic-response": "^2.0.0" - }, - "engines": { - "node": ">=8" - } - }, - "node_modules/deep-extend": { - "version": "0.6.0", - "dev": true, - "license": "MIT", - "engines": { - "node": ">=4.0.0" - } - }, "node_modules/deep-is": { "version": "0.1.4", "dev": true, @@ -6025,11 +5908,6 @@ "node": ">=0.4.0" } }, - "node_modules/delegates": { - "version": "1.0.0", - "dev": true, - "license": "MIT" - }, "node_modules/deprecation": { "version": "2.3.1", "dev": true, @@ -6043,17 +5921,6 @@ "node": ">=4" } }, - "node_modules/detect-libc": { - "version": "1.0.3", - "dev": true, - "license": "Apache-2.0", - "bin": { - "detect-libc": "bin/detect-libc.js" - }, - "engines": { - "node": ">=0.10" - } - }, "node_modules/detect-newline": { "version": "3.1.0", "dev": true, @@ -6857,14 +6724,6 @@ "node": ">= 0.8.0" } }, - "node_modules/expand-template": { - "version": "2.0.3", - "dev": true, - "license": "(MIT OR WTFPL)", - "engines": { - "node": ">=6" - } - }, "node_modules/expect": { "version": "29.7.0", "dev": true, @@ -7184,69 +7043,6 @@ "url": "https://github.com/sponsors/ljharb" } }, - "node_modules/gauge": { - "version": "2.7.4", - "dev": true, - "license": "ISC", - "dependencies": { - "aproba": "^1.0.3", - "console-control-strings": "^1.0.0", - "has-unicode": "^2.0.0", - "object-assign": "^4.1.0", - "signal-exit": "^3.0.0", - "string-width": "^1.0.1", - "strip-ansi": "^3.0.1", - "wide-align": "^1.1.0" - } - }, - "node_modules/gauge/node_modules/ansi-regex": { - "version": "2.1.1", - "dev": true, - "license": "MIT", - "engines": { - "node": ">=0.10.0" - } - }, - "node_modules/gauge/node_modules/aproba": { - "version": "1.2.0", - "dev": true, - "license": "ISC" - }, - "node_modules/gauge/node_modules/is-fullwidth-code-point": { - "version": "1.0.0", - "dev": true, - "license": "MIT", - "dependencies": { - "number-is-nan": "^1.0.0" - }, - "engines": { - "node": ">=0.10.0" - } - }, - "node_modules/gauge/node_modules/string-width": { - "version": "1.0.2", - "dev": true, - "license": "MIT", - "dependencies": { - "code-point-at": "^1.0.0", - "is-fullwidth-code-point": "^1.0.0", - "strip-ansi": "^3.0.0" - }, - "engines": { - "node": ">=0.10.0" - } - }, - "node_modules/gauge/node_modules/strip-ansi": { - "version": "3.0.1", - "dev": true, - "license": "MIT", - "dependencies": { - "ansi-regex": "^2.0.0" - }, - "engines": { - "node": ">=0.10.0" - } - }, "node_modules/gensync": { "version": "1.0.0-beta.2", "dev": true, @@ -7430,29 +7226,6 @@ "ini": "^1.3.2" } }, - "node_modules/github-build": { - "version": "1.2.4", - "dev": true, - "license": "MIT", - "dependencies": { - "axios": "1.6.0" - } - }, - "node_modules/github-build/node_modules/axios": { - "version": "1.6.0", - "dev": true, - "license": "MIT", - "dependencies": { - "follow-redirects": "^1.15.0", - "form-data": "^4.0.0", - "proxy-from-env": "^1.1.0" - } - }, - "node_modules/github-from-package": { - "version": "0.0.0", - "dev": true, - "license": "MIT" - }, "node_modules/glob": { "version": "7.2.3", "dev": true, @@ -7518,18 +7291,6 @@ "dev": true, "license": "ISC" }, - "node_modules/gzip-size": { - "version": "4.1.0", - "dev": true, - "license": "MIT", - "dependencies": { - "duplexer": "^0.1.1", - "pify": "^3.0.0" - }, - "engines": { - "node": ">=4" - } - }, "node_modules/handlebars": { "version": "4.7.8", "dev": true, @@ -7768,39 +7529,6 @@ "url": "https://github.com/sponsors/isaacs" } }, - "node_modules/iltorb": { - "version": "2.4.5", - "dev": true, - "hasInstallScript": true, - "license": "MIT", - "dependencies": { - "detect-libc": "^1.0.3", - "nan": "^2.14.0", - "npmlog": "^4.1.2", - "prebuild-install": "^5.3.3", - "which-pm-runs": "^1.0.0" - } - }, - "node_modules/import-fresh": { - "version": "2.0.0", - "dev": true, - "license": "MIT", - "dependencies": { - "caller-path": "^2.0.0", - "resolve-from": "^3.0.0" - }, - "engines": { - "node": ">=4" - } - }, - "node_modules/import-fresh/node_modules/resolve-from": { - "version": "3.0.0", - "dev": true, - "license": "MIT", - "engines": { - "node": ">=4" - } - }, "node_modules/import-local": { "version": "3.1.0", "dev": true, @@ -8013,14 +7741,6 @@ "url": "https://github.com/sponsors/ljharb" } }, - "node_modules/is-directory": { - "version": "0.3.1", - "dev": true, - "license": "MIT", - "engines": { - "node": ">=0.10.0" - } - }, "node_modules/is-docker": { "version": "2.2.1", "dev": true, @@ -10501,17 +10221,6 @@ "url": "https://github.com/sponsors/sindresorhus" } }, - "node_modules/mimic-response": { - "version": "2.1.0", - "dev": true, - "license": "MIT", - "engines": { - "node": ">=8" - }, - "funding": { - "url": "https://github.com/sponsors/sindresorhus" - } - }, "node_modules/min-indent": { "version": "1.0.1", "dev": true, @@ -10695,11 +10404,6 @@ "node": ">=10" } }, - "node_modules/mkdirp-classic": { - "version": "0.5.3", - "dev": true, - "license": "MIT" - }, "node_modules/modify-values": { "version": "1.0.1", "dev": true, @@ -10736,16 +10440,6 @@ "dev": true, "license": "ISC" }, - "node_modules/nan": { - "version": "2.22.0", - "dev": true, - "license": "MIT" - }, - "node_modules/napi-build-utils": { - "version": "1.0.2", - "dev": true, - "license": "MIT" - }, "node_modules/natural-compare": { "version": "1.4.0", "dev": true, @@ -10764,22 +10458,6 @@ "dev": true, "license": "MIT" }, - "node_modules/node-abi": { - "version": "2.30.1", - "dev": true, - "license": "MIT", - "dependencies": { - "semver": "^5.4.1" - } - }, - "node_modules/node-abi/node_modules/semver": { - "version": "5.7.2", - "dev": true, - "license": "ISC", - "bin": { - "semver": "bin/semver" - } - }, "node_modules/node-fetch": { "version": "2.7.0", "dev": true, @@ -10938,11 +10616,6 @@ "dev": true, "license": "MIT" }, - "node_modules/noop-logger": { - "version": "0.1.1", - "dev": true, - "license": "MIT" - }, "node_modules/nopt": { "version": "7.2.1", "dev": true, @@ -11120,25 +10793,6 @@ "node": ">=8" } }, - "node_modules/npmlog": { - "version": "4.1.2", - "dev": true, - "license": "ISC", - "dependencies": { - "are-we-there-yet": "~1.1.2", - "console-control-strings": "~1.1.0", - "gauge": "~2.7.3", - "set-blocking": "~2.0.0" - } - }, - "node_modules/number-is-nan": { - "version": "1.0.1", - "dev": true, - "license": "MIT", - "engines": { - "node": ">=0.10.0" - } - }, "node_modules/nwsapi": { "version": "2.2.16", "dev": true, @@ -11313,14 +10967,6 @@ "node": ">=12" } }, - "node_modules/object-assign": { - "version": "4.1.1", - "dev": true, - "license": "MIT", - "engines": { - "node": ">=0.10.0" - } - }, "node_modules/once": { "version": "1.4.0", "dev": true, @@ -11795,34 +11441,6 @@ "node": ">=4" } }, - "node_modules/prebuild-install": { - "version": "5.3.6", - "dev": true, - "license": "MIT", - "dependencies": { - "detect-libc": "^1.0.3", - "expand-template": "^2.0.3", - "github-from-package": "0.0.0", - "minimist": "^1.2.3", - "mkdirp-classic": "^0.5.3", - "napi-build-utils": "^1.0.1", - "node-abi": "^2.7.0", - "noop-logger": "^0.1.1", - "npmlog": "^4.0.1", - "pump": "^3.0.0", - "rc": "^1.2.7", - "simple-get": "^3.0.3", - "tar-fs": "^2.0.0", - "tunnel-agent": "^0.6.0", - "which-pm-runs": "^1.0.0" - }, - "bin": { - "prebuild-install": "bin.js" - }, - "engines": { - "node": ">=6" - } - }, "node_modules/prelude-ls": { "version": "1.2.1", "dev": true, @@ -11858,78 +11476,6 @@ "node": "^14.15.0 || ^16.10.0 || >=18.0.0" } }, - "node_modules/prettycli": { - "version": "1.4.3", - "dev": true, - "license": "MIT", - "dependencies": { - "chalk": "2.1.0" - } - }, - "node_modules/prettycli/node_modules/ansi-styles": { - "version": "3.2.1", - "dev": true, - "license": "MIT", - "dependencies": { - "color-convert": "^1.9.0" - }, - "engines": { - "node": ">=4" - } - }, - "node_modules/prettycli/node_modules/chalk": { - "version": "2.1.0", - "dev": true, - "license": "MIT", - "dependencies": { - "ansi-styles": "^3.1.0", - "escape-string-regexp": "^1.0.5", - "supports-color": "^4.0.0" - }, - "engines": { - "node": ">=4" - } - }, - "node_modules/prettycli/node_modules/color-convert": { - "version": "1.9.3", - "dev": true, - "license": "MIT", - "dependencies": { - "color-name": "1.1.3" - } - }, - "node_modules/prettycli/node_modules/color-name": { - "version": "1.1.3", - "dev": true, - "license": "MIT" - }, - "node_modules/prettycli/node_modules/escape-string-regexp": { - "version": "1.0.5", - "dev": true, - "license": "MIT", - "engines": { - "node": ">=0.8.0" - } - }, - "node_modules/prettycli/node_modules/has-flag": { - "version": "2.0.0", - "dev": true, - "license": "MIT", - "engines": { - "node": ">=0.10.0" - } - }, - "node_modules/prettycli/node_modules/supports-color": { - "version": "4.5.0", - "dev": true, - "license": "MIT", - "dependencies": { - "has-flag": "^2.0.0" - }, - "engines": { - "node": ">=4" - } - }, "node_modules/proc-log": { "version": "4.2.0", "dev": true, @@ -12017,15 +11563,6 @@ "dev": true, "license": "MIT" }, - "node_modules/pump": { - "version": "3.0.0", - "dev": true, - "license": "MIT", - "dependencies": { - "end-of-stream": "^1.1.0", - "once": "^1.3.1" - } - }, "node_modules/pure-rand": { "version": "6.1.0", "dev": true, @@ -12075,28 +11612,6 @@ "safe-buffer": "^5.1.0" } }, - "node_modules/rc": { - "version": "1.2.8", - "dev": true, - "license": "(BSD-2-Clause OR MIT OR Apache-2.0)", - "dependencies": { - "deep-extend": "^0.6.0", - "ini": "~1.3.0", - "minimist": "^1.2.0", - "strip-json-comments": "~2.0.1" - }, - "bin": { - "rc": "cli.js" - } - }, - "node_modules/rc/node_modules/strip-json-comments": { - "version": "2.0.1", - "dev": true, - "license": "MIT", - "engines": { - "node": ">=0.10.0" - } - }, "node_modules/react-is": { "version": "18.3.1", "dev": true, @@ -12770,35 +12285,6 @@ "node": "^16.14.0 || >=18.0.0" } }, - "node_modules/simple-concat": { - "version": "1.0.1", - "dev": true, - "funding": [ - { - "type": "github", - "url": "https://github.com/sponsors/feross" - }, - { - "type": "patreon", - "url": "https://www.patreon.com/feross" - }, - { - "type": "consulting", - "url": "https://feross.org/support" - } - ], - "license": "MIT" - }, - "node_modules/simple-get": { - "version": "3.1.1", - "dev": true, - "license": "MIT", - "dependencies": { - "decompress-response": "^4.2.0", - "once": "^1.3.1", - "simple-concat": "^1.0.0" - } - }, "node_modules/sisteransi": { "version": "1.0.5", "dev": true, @@ -13199,22 +12685,6 @@ "node": ">=10" } }, - "node_modules/tar-fs": { - "version": "2.1.1", - "dev": true, - "license": "MIT", - "dependencies": { - "chownr": "^1.1.1", - "mkdirp-classic": "^0.5.2", - "pump": "^3.0.0", - "tar-stream": "^2.1.4" - } - }, - "node_modules/tar-fs/node_modules/chownr": { - "version": "1.1.4", - "dev": true, - "license": "ISC" - }, "node_modules/tar-stream": { "version": "2.2.0", "dev": true, @@ -13461,17 +12931,6 @@ "node": "^16.14.0 || >=18.0.0" } }, - "node_modules/tunnel-agent": { - "version": "0.6.0", - "dev": true, - "license": "Apache-2.0", - "dependencies": { - "safe-buffer": "^5.0.1" - }, - "engines": { - "node": "*" - } - }, "node_modules/type-check": { "version": "0.4.0", "dev": true, @@ -13808,14 +13267,6 @@ "node": ">= 8" } }, - "node_modules/which-pm-runs": { - "version": "1.1.0", - "dev": true, - "license": "MIT", - "engines": { - "node": ">=4" - } - }, "node_modules/wide-align": { "version": "1.1.5", "dev": true, diff --git a/package.json b/package.json index fdd16165..524a8ccb 100644 --- a/package.json +++ b/package.json @@ -14,7 +14,7 @@ "@rollup/plugin-terser": "^0.4.4", "babel-jest": "^29.7.0", "babel-plugin-jsx-pragmatic": "^1.0.2", - "bundlesize": "^0.18.2", + "bytes": "^3.1.2", "eslint": "^9.17.0", "eslint-config-prettier": "^9.1.0", "eslint-plugin-jest": "^28.10.0", @@ -45,12 +45,6 @@ "workspaces": [ "packages/*" ], - "bundlesize": [ - { - "path": "./packages/client/dist/xmpp.min.js", - "maxSize": "15 KB" - } - ], "lint-staged": { "*.{js,cjs,mjs}": "eslint --cache --fix", "*.{json,md,html,css,yaml,yml}": "prettier --write" diff --git a/test/browser.js b/test/browser.test.js similarity index 100% rename from test/browser.js rename to test/browser.test.js diff --git a/test/bundlesize.js b/test/bundlesize.js new file mode 100644 index 00000000..39b952e2 --- /dev/null +++ b/test/bundlesize.js @@ -0,0 +1,31 @@ +/* eslint-disable unicorn/no-process-exit */ +/* eslint-disable n/no-process-exit */ + +import zlib from "node:zlib"; +import { readFile } from "node:fs/promises"; +import bytes from "bytes"; + +import { promisify } from "node:util"; + +const brotliCompress = promisify(zlib.brotliCompress); + +const path = "packages/client/dist/xmpp.min.js"; +const buffer = await readFile(path); +const compressed = await brotliCompress(buffer); + +const max_size = "15 KB"; + +console.log(`${path}:`); +if (compressed.length > bytes(max_size)) { + console.log( + "\u001B[31m%s\u001B[0m", + `${bytes(compressed.length)} > ${max_size} ❌`, + ); + process.exit(1); +} else { + console.log( + "\u001B[32m%s\u001B[0m", + `${bytes(compressed.length)} < ${max_size} ✅`, + ); + process.exit(0); +} diff --git a/test/client.js b/test/client.test.js similarity index 100% rename from test/client.js rename to test/client.test.js diff --git a/test/component.js b/test/component.test.js similarity index 100% rename from test/component.js rename to test/component.test.js diff --git a/test/sasl.js b/test/sasl.test.js similarity index 100% rename from test/sasl.js rename to test/sasl.test.js diff --git a/test/see-other-host.js b/test/see-other-host.test.js similarity index 100% rename from test/see-other-host.js rename to test/see-other-host.test.js From d1790a2e1f1b65cbdc6eb705ff48618d122e4f5c Mon Sep 17 00:00:00 2001 From: Sonny Date: Thu, 9 Jan 2025 20:43:22 +0100 Subject: [PATCH 32/42] events: Add helpers for event listeners (#1051) * Simplify code * Support EventTarget and EventEmitter --- packages/connection/index.js | 68 ++++++++++-------------------- packages/events/index.js | 4 ++ packages/events/lib/listeners.js | 18 ++++++++ packages/events/lib/onoff.js | 21 ++++++++++ packages/events/lib/promise.js | 14 ++++--- packages/websocket/lib/Socket.js | 72 +++++++++++++------------------- 6 files changed, 103 insertions(+), 94 deletions(-) create mode 100644 packages/events/lib/listeners.js create mode 100644 packages/events/lib/onoff.js diff --git a/packages/connection/index.js b/packages/connection/index.js index c74b0233..7799247e 100644 --- a/packages/connection/index.js +++ b/packages/connection/index.js @@ -1,4 +1,4 @@ -import { EventEmitter, promise } from "@xmpp/events"; +import { EventEmitter, promise, listeners } from "@xmpp/events"; import jid from "@xmpp/jid"; import xml from "@xmpp/xml"; import StreamError from "./lib/StreamError.js"; @@ -8,13 +8,14 @@ const NS_STREAM = "urn:ietf:params:xml:ns:xmpp-streams"; const NS_JABBER_STREAM = "http://etherx.jabber.org/streams"; class Connection extends EventEmitter { + #socketListeners = null; + #parserListeners = null; + constructor(options = {}) { super(); this.jid = null; this.timeout = 2000; this.options = options; - this.socketListeners = Object.create(null); - this.parserListeners = Object.create(null); this.status = "offline"; this.socket = null; this.parser = null; @@ -40,7 +41,7 @@ class Connection extends EventEmitter { this.parser.write(str); } - _onParserError(error) { + #onParserError(error) { // https://xmpp.org/rfcs/rfc6120.html#streams-error-conditions-bad-format // "This error can be used instead of the more specific XML-related errors, // such as , , , , @@ -62,32 +63,17 @@ class Connection extends EventEmitter { _attachSocket(socket) { this.socket = socket; - const listeners = this.socketListeners; - - listeners.data = this._onData.bind(this); - - listeners.close = this.#onSocketClosed.bind(this); - - listeners.connect = () => { - this._status("connect"); - }; - - listeners.error = (error) => { - this.emit("error", error); - }; - - this.socket.on("close", listeners.close); - this.socket.on("data", listeners.data); - this.socket.on("error", listeners.error); - this.socket.on("connect", listeners.connect); + this.#socketListeners ??= listeners({ + data: this._onData.bind(this), + close: this.#onSocketClosed.bind(this), + connect: () => this._status("connect"), + error: (error) => this.emit("error", error), + }); + this.#socketListeners.subscribe(this.socket); } _detachSocket() { - const { socketListeners, socket } = this; - for (const k of Object.getOwnPropertyNames(socketListeners)) { - socket.removeListener(k, socketListeners[k]); - delete socketListeners[k]; - } + this.socket && this.#socketListeners?.unsubscribe(this.socket); this.socket = null; } @@ -143,29 +129,17 @@ class Connection extends EventEmitter { _attachParser(parser) { this.parser = parser; - const listeners = this.parserListeners; - - listeners.element = this._onElement.bind(this); - listeners.error = this._onParserError.bind(this); - - listeners.end = this.#onStreamClosed.bind(this); - - listeners.start = (element) => { - this._status("open", element); - }; - - this.parser.on("error", listeners.error); - this.parser.on("element", listeners.element); - this.parser.on("end", listeners.end); - this.parser.on("start", listeners.start); + this.#parserListeners ??= listeners({ + element: this._onElement.bind(this), + error: this.#onParserError.bind(this), + end: this.#onStreamClosed.bind(this), + start: (element) => this._status("open", element), + }); + this.#parserListeners.subscribe(this.parser); } _detachParser() { - const listeners = this.parserListeners; - for (const k of Object.getOwnPropertyNames(listeners)) { - this.parser.removeListener(k, listeners[k]); - delete listeners[k]; - } + this.parser && this.#parserListeners?.unsubscribe(this.parser); this.parser = null; this.root = null; } diff --git a/packages/events/index.js b/packages/events/index.js index e952ad71..cbe287f7 100644 --- a/packages/events/index.js +++ b/packages/events/index.js @@ -6,6 +6,8 @@ import TimeoutError from "./lib/TimeoutError.js"; import promise from "./lib/promise.js"; import Deferred from "./lib/Deferred.js"; import procedure from "./lib/procedure.js"; +import listeners from "./lib/listeners.js"; +import onoff from "./lib/onoff.js"; export { EventEmitter, @@ -15,4 +17,6 @@ export { promise, Deferred, procedure, + listeners, + onoff, }; diff --git a/packages/events/lib/listeners.js b/packages/events/lib/listeners.js new file mode 100644 index 00000000..53aa7254 --- /dev/null +++ b/packages/events/lib/listeners.js @@ -0,0 +1,18 @@ +import onoff from "./onoff.js"; + +export default function listeners(events) { + return { + subscribe(target) { + const { on } = onoff(target); + for (const [event, handler] of Object.entries(events)) { + on(event, handler); + } + }, + unsubscribe(target) { + const { off } = onoff(target); + for (const [event, handler] of Object.entries(events)) { + off(event, handler); + } + }, + }; +} diff --git a/packages/events/lib/onoff.js b/packages/events/lib/onoff.js new file mode 100644 index 00000000..6d4c10a7 --- /dev/null +++ b/packages/events/lib/onoff.js @@ -0,0 +1,21 @@ +const map = new WeakMap(); + +export default function onoff(target) { + let m = map.get(target); + + if (!m) { + const on = (target.addEventListener ?? target.addListener).bind(target); + const off = (target.removeEventListener ?? target.removeListener).bind( + target, + ); + const once = ( + target.once ?? + ((event, handler) => + target.addEventListener(event, handler, { once: true })) + ).bind(target); + m = { on, off, once }; + map.set(target, m); + } + + return m; +} diff --git a/packages/events/lib/promise.js b/packages/events/lib/promise.js index 69fb61b4..c741b9b7 100644 --- a/packages/events/lib/promise.js +++ b/packages/events/lib/promise.js @@ -1,13 +1,17 @@ +import onoff from "./onoff.js"; + import TimeoutError from "./TimeoutError.js"; -export default function promise(EE, event, rejectEvent = "error", timeout) { +export default function promise(target, event, rejectEvent = "error", timeout) { return new Promise((resolve, reject) => { let timeoutId; + const { off, once } = onoff(target); + const cleanup = () => { clearTimeout(timeoutId); - EE.removeListener(event, onEvent); - EE.removeListener(rejectEvent, onError); + off(event, onEvent); + off(rejectEvent, onError); }; function onError(reason) { @@ -20,9 +24,9 @@ export default function promise(EE, event, rejectEvent = "error", timeout) { cleanup(); } - EE.once(event, onEvent); + once(event, onEvent); if (rejectEvent) { - EE.once(rejectEvent, onError); + once(rejectEvent, onError); } if (timeout) { diff --git a/packages/websocket/lib/Socket.js b/packages/websocket/lib/Socket.js index d48e5fa4..3e99ee41 100644 --- a/packages/websocket/lib/Socket.js +++ b/packages/websocket/lib/Socket.js @@ -1,5 +1,5 @@ import WS from "ws"; -import { EventEmitter } from "@xmpp/events"; +import { EventEmitter, listeners } from "@xmpp/events"; import { parseURI } from "@xmpp/connection/lib/util.js"; // eslint-disable-next-line n/no-unsupported-features/node-builtins @@ -8,10 +8,9 @@ const WebSocket = globalThis.WebSocket || WS; const CODE = "ECONNERROR"; export default class Socket extends EventEmitter { - constructor() { - super(); - this.listeners = Object.create(null); - } + #listeners = null; + socket = null; + url = null; isSecure() { if (!this.url) return false; @@ -28,47 +27,36 @@ export default class Socket extends EventEmitter { _attachSocket(socket) { this.socket = socket; - const { listeners } = this; - listeners.open = () => { - this.emit("connect"); - }; - - listeners.message = ({ data }) => this.emit("data", data); - listeners.error = (event) => { - const { url } = this; - // WS - let { error } = event; - // DOM - if (!error) { - error = new Error(`WebSocket ${CODE} ${url}`); - error.errno = CODE; - error.code = CODE; - } + this.#listeners ??= listeners({ + open: () => this.emit("connect"), + message: ({ data }) => this.emit("data", data), + error: (event) => { + const { url } = this; + // WS + let { error } = event; + // DOM + if (!error) { + error = new Error(event.message || `WebSocket ${CODE} ${url}`); + error.errno = CODE; + error.code = CODE; + } - error.event = event; - error.url = url; - this.emit("error", error); - }; - - listeners.close = (event) => { - this._detachSocket(); - this.emit("close", !event.wasClean, event); - }; - - this.socket.addEventListener("open", listeners.open); - this.socket.addEventListener("message", listeners.message); - this.socket.addEventListener("error", listeners.error); - this.socket.addEventListener("close", listeners.close); + error.event = event; + error.url = url; + this.emit("error", error); + }, + close: (event) => { + this._detachSocket(); + this.emit("close", !event.wasClean, event); + }, + }); + this.#listeners.subscribe(this.socket); } _detachSocket() { - delete this.url; - const { socket, listeners } = this; - for (const k of Object.getOwnPropertyNames(listeners)) { - socket.removeEventListener(k, listeners[k]); - delete listeners[k]; - } - delete this.socket; + this.url = null; + this.socket && this.#listeners?.unsubscribe(this.socket); + this.socket = null; } end() { From f932be3d60d3082b302e2f2349f7ab23bdab2181 Mon Sep 17 00:00:00 2001 From: Sonny Date: Thu, 9 Jan 2025 21:52:37 +0100 Subject: [PATCH 33/42] websocket: Remove ws dependency (#1052) And improve `sendMany` for WebSocket --- package-lock.json | 74 ++++++++++----------- package.json | 6 +- packages/base64/package.json | 2 +- packages/client-core/package.json | 2 +- packages/client/README.md | 7 -- packages/client/package.json | 2 +- packages/component-core/package.json | 2 +- packages/component/package.json | 2 +- packages/connection-tcp/package.json | 2 +- packages/connection/package.json | 2 +- packages/debug/package.json | 2 +- packages/error/package.json | 2 +- packages/events/package.json | 2 +- packages/id/package.json | 2 +- packages/iq/package.json | 2 +- packages/jid/package.json | 2 +- packages/middleware/package.json | 2 +- packages/reconnect/package.json | 2 +- packages/resolve/package.json | 2 +- packages/resource-binding/package.json | 2 +- packages/sasl-anonymous/package.json | 2 +- packages/sasl-ht-sha-256-none/package.json | 2 +- packages/sasl-plain/package.json | 2 +- packages/sasl-scram-sha-1/package.json | 2 +- packages/sasl/package.json | 2 +- packages/session-establishment/package.json | 2 +- packages/starttls/package.json | 2 +- packages/stream-features/package.json | 2 +- packages/stream-management/package.json | 2 +- packages/tcp/package.json | 2 +- packages/test/package.json | 2 +- packages/time/package.json | 2 +- packages/tls/package.json | 2 +- packages/uri/package.json | 2 +- packages/websocket/lib/Connection.js | 10 +-- packages/websocket/lib/Socket.js | 21 ++---- packages/websocket/package.json | 8 +-- packages/websocket/test/test.js | 49 +++++++------- packages/xml/package.json | 2 +- packages/xmpp.js/package.json | 2 +- 40 files changed, 111 insertions(+), 130 deletions(-) diff --git a/package-lock.json b/package-lock.json index 7fb3b8a7..894db09f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -41,7 +41,7 @@ "selfsigned": "^2.4.1" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" } }, "node_modules/@ampproject/remapping": { @@ -13431,6 +13431,7 @@ }, "node_modules/ws": { "version": "8.18.0", + "dev": true, "license": "MIT", "engines": { "node": ">=10.0.0" @@ -13538,7 +13539,7 @@ "version": "0.14.0", "license": "ISC", "engines": { - "node": ">= 20" + "node": ">= 20.10" } }, "packages/client": { @@ -13567,7 +13568,7 @@ "saslmechanisms": "^0.1.1" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" } }, "packages/client-core": { @@ -13583,7 +13584,7 @@ "saslmechanisms": "^0.1.1" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" } }, "packages/component": { @@ -13597,7 +13598,7 @@ "@xmpp/reconnect": "^0.14.0" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" } }, "packages/component-core": { @@ -13610,7 +13611,7 @@ "@xmpp/xml": "^0.14.0" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" } }, "packages/connection": { @@ -13624,7 +13625,7 @@ "@xmpp/xml": "^0.14.0" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" } }, "packages/connection-tcp": { @@ -13636,7 +13637,7 @@ "@xmpp/xml": "^0.14.0" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" } }, "packages/debug": { @@ -13648,7 +13649,7 @@ "ltx": "^3.1.1" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" } }, "packages/error": { @@ -13656,7 +13657,7 @@ "version": "0.14.0", "license": "ISC", "engines": { - "node": ">= 20" + "node": ">= 20.10" } }, "packages/events": { @@ -13667,7 +13668,7 @@ "events": "^3.3.0" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" } }, "packages/id": { @@ -13675,7 +13676,7 @@ "version": "0.14.0", "license": "ISC", "engines": { - "node": ">= 20" + "node": ">= 20.10" } }, "packages/iq": { @@ -13689,7 +13690,7 @@ "@xmpp/xml": "^0.14.0" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" } }, "packages/jid": { @@ -13697,7 +13698,7 @@ "version": "0.14.0", "license": "ISC", "engines": { - "node": ">= 20" + "node": ">= 20.10" } }, "packages/middleware": { @@ -13711,7 +13712,7 @@ "koa-compose": "^4.1.0" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" } }, "packages/reconnect": { @@ -13722,7 +13723,7 @@ "@xmpp/events": "^0.14.0" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" } }, "packages/resolve": { @@ -13734,7 +13735,7 @@ "@xmpp/xml": "^0.14.0" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" } }, "packages/resource-binding": { @@ -13745,7 +13746,7 @@ "@xmpp/xml": "^0.14.0" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" } }, "packages/sasl": { @@ -13759,7 +13760,7 @@ "@xmpp/xml": "^0.14.0" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" } }, "packages/sasl-anonymous": { @@ -13770,7 +13771,7 @@ "sasl-anonymous": "^0.1.0" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" } }, "packages/sasl-ht-sha-256-none": { @@ -13778,7 +13779,7 @@ "version": "0.14.0", "license": "ISC", "engines": { - "node": ">= 20" + "node": ">= 20.10" } }, "packages/sasl-plain": { @@ -13789,7 +13790,7 @@ "sasl-plain": "^0.1.0" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" } }, "packages/sasl-scram-sha-1": { @@ -13800,7 +13801,7 @@ "sasl-scram-sha-1": "^1.3.0" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" } }, "packages/sasl2": { @@ -13827,7 +13828,7 @@ "@xmpp/xml": "^0.14.0" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" } }, "packages/starttls": { @@ -13840,7 +13841,7 @@ "@xmpp/xml": "^0.14.0" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" } }, "packages/stream-features": { @@ -13848,7 +13849,7 @@ "version": "0.14.0", "license": "ISC", "engines": { - "node": ">= 20" + "node": ">= 20.10" } }, "packages/stream-management": { @@ -13862,7 +13863,7 @@ "@xmpp/xml": "^0.14.0" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" } }, "packages/tcp": { @@ -13873,7 +13874,7 @@ "@xmpp/connection-tcp": "^0.14.0" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" } }, "packages/test": { @@ -13892,7 +13893,7 @@ "ltx": "^3.1.1" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" } }, "packages/time": { @@ -13900,7 +13901,7 @@ "version": "0.14.0", "license": "ISC", "engines": { - "node": ">= 20" + "node": ">= 20.10" } }, "packages/tls": { @@ -13913,7 +13914,7 @@ "@xmpp/events": "^0.14.0" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" } }, "packages/uri": { @@ -13925,7 +13926,7 @@ "iri": "^1.3.1" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" } }, "packages/websocket": { @@ -13935,11 +13936,10 @@ "dependencies": { "@xmpp/connection": "^0.14.0", "@xmpp/events": "^0.14.0", - "@xmpp/xml": "^0.14.0", - "ws": "^8.18.0" + "@xmpp/xml": "^0.14.0" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" } }, "packages/xml": { @@ -13951,7 +13951,7 @@ "ltx": "^3.1.1" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" } }, "packages/xmpp.js": { @@ -13994,7 +13994,7 @@ "@xmpp/xml": "^0.14.0" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" } } } diff --git a/package.json b/package.json index 524a8ccb..26c130c5 100644 --- a/package.json +++ b/package.json @@ -35,12 +35,12 @@ "selfsigned": "^2.4.1" }, "scripts": { - "test": "npx jest", - "e2e": "NODE_TLS_REJECT_UNAUTHORIZED=0 npx jest --runInBand --config e2e.config.cjs", + "test": "node --experimental-websocket ./node_modules/.bin/jest", + "e2e": "NODE_TLS_REJECT_UNAUTHORIZED=0 node --experimental-websocket ./node_modules/.bin/jest --runInBand --config e2e.config.cjs", "preversion": "make bundle" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" }, "workspaces": [ "packages/*" diff --git a/packages/base64/package.json b/packages/base64/package.json index 1875ca42..5f77d17e 100644 --- a/packages/base64/package.json +++ b/packages/base64/package.json @@ -13,7 +13,7 @@ "base64" ], "engines": { - "node": ">= 20" + "node": ">= 20.10" }, "publishConfig": { "access": "public" diff --git a/packages/client-core/package.json b/packages/client-core/package.json index c6ed923c..749ddad3 100644 --- a/packages/client-core/package.json +++ b/packages/client-core/package.json @@ -16,7 +16,7 @@ "saslmechanisms": "^0.1.1" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" }, "publishConfig": { "access": "public" diff --git a/packages/client/README.md b/packages/client/README.md index 12eda4b8..2ac26d1f 100644 --- a/packages/client/README.md +++ b/packages/client/README.md @@ -293,10 +293,3 @@ PLAIN should only be used over secure WebSocket (`wss://)`, direct TLS (`xmpps:` - ☐ : Optional - ✗ : Unavailable - ✔ : Included - -## Common issues - -
- Unable to resolve module -

If you are using an older React Native version, please require/import @xmpp/client/reat-native instead of @xmpp/client.

-
diff --git a/packages/client/package.json b/packages/client/package.json index f23b77b7..49bdbdd3 100644 --- a/packages/client/package.json +++ b/packages/client/package.json @@ -35,7 +35,7 @@ "@xmpp/sasl-scram-sha-1": false }, "engines": { - "node": ">= 20" + "node": ">= 20.10" }, "publishConfig": { "access": "public" diff --git a/packages/component-core/package.json b/packages/component-core/package.json index adad8b19..c75f5234 100644 --- a/packages/component-core/package.json +++ b/packages/component-core/package.json @@ -19,7 +19,7 @@ "@xmpp/xml": "^0.14.0" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" }, "publishConfig": { "access": "public" diff --git a/packages/component/package.json b/packages/component/package.json index 1e538aa0..919cb3c7 100644 --- a/packages/component/package.json +++ b/packages/component/package.json @@ -19,7 +19,7 @@ "@xmpp/reconnect": "^0.14.0" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" }, "publishConfig": { "access": "public" diff --git a/packages/connection-tcp/package.json b/packages/connection-tcp/package.json index 16bf68f0..893733a2 100644 --- a/packages/connection-tcp/package.json +++ b/packages/connection-tcp/package.json @@ -18,7 +18,7 @@ "@xmpp/xml": "^0.14.0" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" }, "publishConfig": { "access": "public" diff --git a/packages/connection/package.json b/packages/connection/package.json index 8ffff73f..50c7e2c9 100644 --- a/packages/connection/package.json +++ b/packages/connection/package.json @@ -19,7 +19,7 @@ "@xmpp/xml": "^0.14.0" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" }, "publishConfig": { "access": "public" diff --git a/packages/debug/package.json b/packages/debug/package.json index 01342432..3e22f377 100644 --- a/packages/debug/package.json +++ b/packages/debug/package.json @@ -13,7 +13,7 @@ "debug" ], "engines": { - "node": ">= 20" + "node": ">= 20.10" }, "dependencies": { "@xmpp/xml": "^0.14.0", diff --git a/packages/error/package.json b/packages/error/package.json index 1bf254fe..8d6b6490 100644 --- a/packages/error/package.json +++ b/packages/error/package.json @@ -13,7 +13,7 @@ "error" ], "engines": { - "node": ">= 20" + "node": ">= 20.10" }, "publishConfig": { "access": "public" diff --git a/packages/events/package.json b/packages/events/package.json index d8a392b1..d8e1954e 100644 --- a/packages/events/package.json +++ b/packages/events/package.json @@ -15,7 +15,7 @@ "EventEmitter" ], "engines": { - "node": ">= 20" + "node": ">= 20.10" }, "publishConfig": { "access": "public" diff --git a/packages/id/package.json b/packages/id/package.json index 196b4656..1a25c3d2 100644 --- a/packages/id/package.json +++ b/packages/id/package.json @@ -13,7 +13,7 @@ "id" ], "engines": { - "node": ">= 20" + "node": ">= 20.10" }, "publishConfig": { "access": "public" diff --git a/packages/iq/package.json b/packages/iq/package.json index e8e317e7..516623cf 100644 --- a/packages/iq/package.json +++ b/packages/iq/package.json @@ -15,7 +15,7 @@ "callee" ], "engines": { - "node": ">= 20" + "node": ">= 20.10" }, "dependencies": { "@xmpp/events": "^0.14.0", diff --git a/packages/jid/package.json b/packages/jid/package.json index 8b85bc72..2b495bbf 100644 --- a/packages/jid/package.json +++ b/packages/jid/package.json @@ -13,7 +13,7 @@ "JID" ], "engines": { - "node": ">= 20" + "node": ">= 20.10" }, "publishConfig": { "access": "public" diff --git a/packages/middleware/package.json b/packages/middleware/package.json index 30eaa078..ae06012b 100644 --- a/packages/middleware/package.json +++ b/packages/middleware/package.json @@ -19,7 +19,7 @@ "koa-compose": "^4.1.0" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" }, "publishConfig": { "access": "public" diff --git a/packages/reconnect/package.json b/packages/reconnect/package.json index a6fc4ce8..f6349657 100644 --- a/packages/reconnect/package.json +++ b/packages/reconnect/package.json @@ -16,7 +16,7 @@ "@xmpp/events": "^0.14.0" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" }, "publishConfig": { "access": "public" diff --git a/packages/resolve/package.json b/packages/resolve/package.json index b5ee247b..3d8f9546 100644 --- a/packages/resolve/package.json +++ b/packages/resolve/package.json @@ -26,7 +26,7 @@ "@xmpp/xml": "^0.14.0" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" }, "publishConfig": { "access": "public" diff --git a/packages/resource-binding/package.json b/packages/resource-binding/package.json index d3ac6f8d..464db07b 100644 --- a/packages/resource-binding/package.json +++ b/packages/resource-binding/package.json @@ -17,7 +17,7 @@ "@xmpp/xml": "^0.14.0" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" }, "publishConfig": { "access": "public" diff --git a/packages/sasl-anonymous/package.json b/packages/sasl-anonymous/package.json index 00fbc57c..1aae183a 100644 --- a/packages/sasl-anonymous/package.json +++ b/packages/sasl-anonymous/package.json @@ -16,7 +16,7 @@ "sasl-anonymous": "^0.1.0" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" }, "publishConfig": { "access": "public" diff --git a/packages/sasl-ht-sha-256-none/package.json b/packages/sasl-ht-sha-256-none/package.json index 6af011c2..3d4e0d96 100644 --- a/packages/sasl-ht-sha-256-none/package.json +++ b/packages/sasl-ht-sha-256-none/package.json @@ -13,7 +13,7 @@ "sasl" ], "engines": { - "node": ">= 20" + "node": ">= 20.10" }, "publishConfig": { "access": "public" diff --git a/packages/sasl-plain/package.json b/packages/sasl-plain/package.json index 9b401b8f..d529e0dc 100644 --- a/packages/sasl-plain/package.json +++ b/packages/sasl-plain/package.json @@ -16,7 +16,7 @@ "sasl-plain": "^0.1.0" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" }, "publishConfig": { "access": "public" diff --git a/packages/sasl-scram-sha-1/package.json b/packages/sasl-scram-sha-1/package.json index 17ee55c0..ac121e80 100644 --- a/packages/sasl-scram-sha-1/package.json +++ b/packages/sasl-scram-sha-1/package.json @@ -16,7 +16,7 @@ "sasl-scram-sha-1": "^1.3.0" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" }, "publishConfig": { "access": "public" diff --git a/packages/sasl/package.json b/packages/sasl/package.json index 445c5b92..7aeced69 100644 --- a/packages/sasl/package.json +++ b/packages/sasl/package.json @@ -19,7 +19,7 @@ "@xmpp/xml": "^0.14.0" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" }, "publishConfig": { "access": "public" diff --git a/packages/session-establishment/package.json b/packages/session-establishment/package.json index f916982c..f3ef5714 100644 --- a/packages/session-establishment/package.json +++ b/packages/session-establishment/package.json @@ -17,7 +17,7 @@ "@xmpp/xml": "^0.14.0" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" }, "publishConfig": { "access": "public" diff --git a/packages/starttls/package.json b/packages/starttls/package.json index 803b4b60..0359af86 100644 --- a/packages/starttls/package.json +++ b/packages/starttls/package.json @@ -18,7 +18,7 @@ "@xmpp/xml": "^0.14.0" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" }, "publishConfig": { "access": "public" diff --git a/packages/stream-features/package.json b/packages/stream-features/package.json index 29638c20..cc9c5eb2 100644 --- a/packages/stream-features/package.json +++ b/packages/stream-features/package.json @@ -14,7 +14,7 @@ "features" ], "engines": { - "node": ">= 20" + "node": ">= 20.10" }, "publishConfig": { "access": "public" diff --git a/packages/stream-management/package.json b/packages/stream-management/package.json index 72503e38..e0e3c3dd 100644 --- a/packages/stream-management/package.json +++ b/packages/stream-management/package.json @@ -20,7 +20,7 @@ "@xmpp/time": "^0.14.0" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" }, "publishConfig": { "access": "public" diff --git a/packages/tcp/package.json b/packages/tcp/package.json index 38fdab15..2ae44a73 100644 --- a/packages/tcp/package.json +++ b/packages/tcp/package.json @@ -16,7 +16,7 @@ "tcp" ], "engines": { - "node": ">= 20" + "node": ">= 20.10" }, "publishConfig": { "access": "public" diff --git a/packages/test/package.json b/packages/test/package.json index 99583494..aa63b797 100644 --- a/packages/test/package.json +++ b/packages/test/package.json @@ -13,7 +13,7 @@ "test" ], "engines": { - "node": ">= 20" + "node": ">= 20.10" }, "dependencies": { "@xmpp/client": "^0.14.0", diff --git a/packages/time/package.json b/packages/time/package.json index 88721c6e..97722d54 100644 --- a/packages/time/package.json +++ b/packages/time/package.json @@ -14,7 +14,7 @@ "date" ], "engines": { - "node": ">= 20" + "node": ">= 20.10" }, "publishConfig": { "access": "public" diff --git a/packages/tls/package.json b/packages/tls/package.json index d3811246..1b1f8c74 100644 --- a/packages/tls/package.json +++ b/packages/tls/package.json @@ -18,7 +18,7 @@ "tls" ], "engines": { - "node": ">= 20" + "node": ">= 20.10" }, "publishConfig": { "access": "public" diff --git a/packages/uri/package.json b/packages/uri/package.json index 77165677..69a0ecda 100644 --- a/packages/uri/package.json +++ b/packages/uri/package.json @@ -17,7 +17,7 @@ "iri": "^1.3.1" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" }, "publishConfig": { "access": "public" diff --git a/packages/websocket/lib/Connection.js b/packages/websocket/lib/Connection.js index 335b7661..f73b0924 100644 --- a/packages/websocket/lib/Connection.js +++ b/packages/websocket/lib/Connection.js @@ -13,16 +13,16 @@ const NS_FRAMING = "urn:ietf:params:xml:ns:xmpp-framing"; class ConnectionWebSocket extends Connection { send(element, ...args) { - if (!element.attrs.xmlns && super.isStanza(element)) { - element.attrs.xmlns = "jabber:client"; - } - + element.attrs.xmlns ??= this.NS; return super.send(element, ...args); } async sendMany(elements) { for (const element of elements) { - await this.send(element); + element.attrs.xmlns ??= this.NS; + element.parent = this.root; + this.socket.write(element.toString()); + this.emit("send", element); } } diff --git a/packages/websocket/lib/Socket.js b/packages/websocket/lib/Socket.js index 3e99ee41..69647fdb 100644 --- a/packages/websocket/lib/Socket.js +++ b/packages/websocket/lib/Socket.js @@ -1,10 +1,6 @@ -import WS from "ws"; import { EventEmitter, listeners } from "@xmpp/events"; import { parseURI } from "@xmpp/connection/lib/util.js"; -// eslint-disable-next-line n/no-unsupported-features/node-builtins -const WebSocket = globalThis.WebSocket || WS; - const CODE = "ECONNERROR"; export default class Socket extends EventEmitter { @@ -22,6 +18,7 @@ export default class Socket extends EventEmitter { connect(url) { this.url = url; + // eslint-disable-next-line n/no-unsupported-features/node-builtins this._attachSocket(new WebSocket(url, ["xmpp"])); } @@ -68,16 +65,12 @@ export default class Socket extends EventEmitter { } write(data, fn) { - if (WebSocket === WS) { - this.socket.send(data, fn); - } else { - try { - this.socket.send(data); - } catch (err) { - fn(err); - return; - } - fn(); + try { + this.socket.send(data); + } catch (err) { + fn?.(err); + return; } + fn?.(); } } diff --git a/packages/websocket/package.json b/packages/websocket/package.json index 001afdde..7a5e891f 100644 --- a/packages/websocket/package.json +++ b/packages/websocket/package.json @@ -11,18 +11,14 @@ "dependencies": { "@xmpp/connection": "^0.14.0", "@xmpp/events": "^0.14.0", - "@xmpp/xml": "^0.14.0", - "ws": "^8.18.0" - }, - "browser": { - "ws": false + "@xmpp/xml": "^0.14.0" }, "keywords": [ "XMPP", "websocket" ], "engines": { - "node": ">= 20" + "node": ">= 20.10" }, "publishConfig": { "access": "public" diff --git a/packages/websocket/test/test.js b/packages/websocket/test/test.js index 742cd1c4..fda33685 100644 --- a/packages/websocket/test/test.js +++ b/packages/websocket/test/test.js @@ -3,15 +3,18 @@ import Socket from "../lib/Socket.js"; import { EventEmitter } from "@xmpp/events"; import xml from "@xmpp/xml"; -test("send() adds jabber:client xmlns", () => { +test("send()", () => { const connection = new ConnectionWebSocket(); connection.write = () => {}; + connection.root = xml("root"); const element = xml("presence"); expect(element.attrs.xmlns).toBe(undefined); + expect(element.parent).toBe(null); connection.send(element); expect(element.attrs.xmlns).toBe("jabber:client"); + expect(element.parent).toBe(connection.root); }); test("socketParameters()", () => { @@ -27,7 +30,7 @@ test("socketParameters()", () => { expect(params).toBe(undefined); }); -test("DOM WebSocket error", () => { +test("WebSocket error", () => { const socket = new Socket(); const sock = new EventEmitter(); sock.addEventListener = sock.addListener; @@ -44,22 +47,6 @@ test("DOM WebSocket error", () => { socket.socket.emit("error", evt); }); -test("WS WebSocket error", () => { - const socket = new Socket(); - const sock = new EventEmitter(); - sock.addEventListener = sock.addListener; - socket._attachSocket(sock); - socket.url = "ws://foobar"; - const error = {}; - const evt = { error }; - socket.on("error", (err) => { - expect(err).toBe(error); - expect(err.event).toBe(evt); - expect(err.url).toBe("ws://foobar"); - }); - socket.socket.emit("error", evt); -}); - test("socket close", () => { expect.assertions(3); const socket = new Socket(); @@ -81,15 +68,27 @@ test("socket close", () => { test("sendMany", async () => { const conn = new ConnectionWebSocket(); + conn.socket = new Socket(); + const spy_write = jest.spyOn(conn.socket, "write"); + conn.root = xml("root"); + + const foo = xml("presence"); + const bar = xml("presence"); + const elements = [foo, bar]; - const foo = xml("foo"); - const bar = xml("bar"); + for (const element of elements) { + expect(element.attrs.xmlns).toBe(undefined); + expect(element.parent).toBe(null); + } - const spy_send = (conn.send = jest.fn()); + conn.sendMany(elements); - await conn.sendMany([foo, bar]); + for (const element of elements) { + expect(element.attrs.xmlns).toBe("jabber:client"); + expect(element.parent).toBe(conn.root); + } - expect(spy_send).toHaveBeenCalledWith(foo); - expect(spy_send).toHaveBeenCalledWith(bar); - expect(spy_send).toHaveBeenCalledTimes(2); + expect(spy_write).toHaveBeenCalledWith(foo.toString()); + expect(spy_write).toHaveBeenCalledWith(bar.toString()); + expect(spy_write).toHaveBeenCalledTimes(2); }); diff --git a/packages/xml/package.json b/packages/xml/package.json index bb7eb11f..59c48588 100644 --- a/packages/xml/package.json +++ b/packages/xml/package.json @@ -20,7 +20,7 @@ "ltx": "^3.1.1" }, "engines": { - "node": ">= 20" + "node": ">= 20.10" }, "publishConfig": { "access": "public" diff --git a/packages/xmpp.js/package.json b/packages/xmpp.js/package.json index 368ea788..8d5e3d20 100644 --- a/packages/xmpp.js/package.json +++ b/packages/xmpp.js/package.json @@ -14,7 +14,7 @@ "component" ], "engines": { - "node": ">= 20" + "node": ">= 20.10" }, "dependencies": { "@xmpp/base64": "^0.14.0", From fb5b7997ff47213a109e8d75568aede29690aca9 Mon Sep 17 00:00:00 2001 From: Sonny Piers Date: Thu, 9 Jan 2025 22:13:18 +0100 Subject: [PATCH 34/42] tls: Use listeners() --- packages/tls/lib/Socket.js | 72 ++++++++++++++++---------------------- 1 file changed, 31 insertions(+), 41 deletions(-) diff --git a/packages/tls/lib/Socket.js b/packages/tls/lib/Socket.js index 9b60506c..b8b0338a 100644 --- a/packages/tls/lib/Socket.js +++ b/packages/tls/lib/Socket.js @@ -1,12 +1,10 @@ import tls from "tls"; -import { EventEmitter } from "@xmpp/events"; +import { EventEmitter, listeners } from "@xmpp/events"; class Socket extends EventEmitter { - constructor() { - super(); - this.listeners = Object.create(null); - this.timeout = null; - } + timeout = null; + #listeners = null; + socket = null; isSecure() { return true; @@ -18,45 +16,37 @@ class Socket extends EventEmitter { _attachSocket(socket) { this.socket = socket; - const { listeners } = this; - - listeners.close = () => { - this._detachSocket(); - this.emit("close"); - }; - listeners.data = (data) => { - this.emit("data", data); - }; - listeners.error = (err) => { - this.emit("error", err); - }; - listeners.secureConnect = () => { - if (this.socket.getProtocol() !== "TLSv1.3") { - return this.emit("connect"); - } - - // Waiting before sending the stream header improves compatibility - // with Openfire TLSv1.3 implementation. For more info, see: - // https://github.com/xmppjs/xmpp.js/issues/889#issuecomment-902686879 - // https://github.com/xmppjs/xmpp.js/pull/912 - this.timeout = setTimeout(() => { - this.emit("connect"); - }, 1); - }; + this.#listeners ??= listeners({ + close: () => { + this._detachSocket(); + this.emit("close"); + }, + data: (data) => { + this.emit("data", data); + }, + error: (err) => { + this.emit("error", err); + }, + secureConnect: () => { + if (this.socket.getProtocol() !== "TLSv1.3") { + return this.emit("connect"); + } - for (const [event, listener] of Object.entries(listeners)) { - socket.on(event, listener); - } + // Waiting before sending the stream header improves compatibility + // with Openfire TLSv1.3 implementation. For more info, see: + // https://github.com/xmppjs/xmpp.js/issues/889#issuecomment-902686879 + // https://github.com/xmppjs/xmpp.js/pull/912 + this.timeout = setTimeout(() => { + this.emit("connect"); + }, 1); + }, + }); + this.#listeners.subscribe(this.socket); } _detachSocket() { - clearTimeout(this.timeout); - const { socket, listeners } = this; - for (const [event, listener] of Object.entries(listeners)) { - socket.removeListener(event, listener); - delete listeners[event]; - } - delete this.socket; + this.#listeners.unsubscribe(this.socket); + this.socket = null; } end() { From 2d6e46fe00fba6112be55a3f166d4f44da75d067 Mon Sep 17 00:00:00 2001 From: Sonny Date: Thu, 9 Jan 2025 23:52:06 +0100 Subject: [PATCH 35/42] Various code improvements (#1053) --- packages/connection/index.js | 35 +++++++----------------- packages/connection/test/_closeStream.js | 22 ++++++++------- packages/connection/test/onData.js | 5 +--- packages/events/lib/TimeoutError.js | 1 + packages/starttls/test.js | 9 +----- packages/test/index.js | 2 ++ packages/test/mockSocket.js | 8 ++++-- 7 files changed, 32 insertions(+), 50 deletions(-) diff --git a/packages/connection/index.js b/packages/connection/index.js index 7799247e..f3b840c4 100644 --- a/packages/connection/index.js +++ b/packages/connection/index.js @@ -37,7 +37,6 @@ class Connection extends EventEmitter { _onData(data) { const str = data.toString("utf8"); - this.emit("input", str); this.parser.write(str); } @@ -299,15 +298,9 @@ class Connection extends EventEmitter { async _closeStream(timeout = this.timeout) { const fragment = this.footer(this.footerElement()); - const p = Promise.all([ - promise(this.parser, "end", "error", timeout), - this.write(fragment), - ]); - + await this.write(fragment); this._status("closing"); - - const [el] = await p; - return el; + return promise(this.parser, "end", "error", timeout); // The 'close' status is set by the parser 'end' listener } @@ -334,23 +327,15 @@ class Connection extends EventEmitter { ]).then(([, el]) => el); } - write(string) { + async write(string) { + // https://xmpp.org/rfcs/rfc6120.html#streams-close + // "Refrain from sending any further data over its outbound stream to the other entity" + if (this.status === "closing") { + throw new Error("Connection is closing"); + } + return new Promise((resolve, reject) => { - // https://xmpp.org/rfcs/rfc6120.html#streams-close - // "Refrain from sending any further data over its outbound stream to the other entity" - if (this.status === "closing") { - reject(new Error("Connection is closing")); - return; - } - - this.socket.write(string, (err) => { - if (err) { - return reject(err); - } - - this.emit("output", string); - resolve(); - }); + this.socket.write(string, (err) => (err ? reject(err) : resolve())); }); } diff --git a/packages/connection/test/_closeStream.js b/packages/connection/test/_closeStream.js index e8eefdfc..04719717 100644 --- a/packages/connection/test/_closeStream.js +++ b/packages/connection/test/_closeStream.js @@ -48,26 +48,28 @@ test("resolves", async () => { jest.spyOn(conn, "footerElement").mockImplementation(() => xml("hello")); jest.spyOn(conn, "write").mockImplementation(async () => {}); - const promiseClose = conn._closeStream(); - conn.parser.emit("end", xml("goodbye")); - - const el = await promiseClose; + process.nextTick(() => { + conn.parser.emit("end", xml("goodbye")); + }); + const el = await conn._closeStream(); expect(el.toString()).toBe(``); }); -test("emits closing status", () => { +test("emits closing status", async () => { const conn = new Connection(); conn.parser = new EventEmitter(); jest.spyOn(conn, "footerElement").mockImplementation(() => xml("hello")); jest.spyOn(conn, "write").mockImplementation(async () => {}); - const p = Promise.all([ - promise(conn, "status").then((status) => expect(status).toBe("closing")), + process.nextTick(() => { + conn.parser.emit("end"); + }); + + const [status] = await Promise.all([ + promise(conn, "status"), conn._closeStream(), ]); - - conn.parser.emit("end"); - return p; + expect(status).toBe("closing"); }); diff --git a/packages/connection/test/onData.js b/packages/connection/test/onData.js index 58a1f9ef..84e46748 100644 --- a/packages/connection/test/onData.js +++ b/packages/connection/test/onData.js @@ -1,7 +1,7 @@ import Connection from "../index.js"; test("#_onData", () => { - expect.assertions(2); + expect.assertions(1); const foo = ""; const conn = new Connection(); conn.parser = { @@ -10,8 +10,5 @@ test("#_onData", () => { }, }; - conn.on("input", (data) => { - expect(data).toBe(foo); - }); conn._onData(foo); }); diff --git a/packages/events/lib/TimeoutError.js b/packages/events/lib/TimeoutError.js index e6880d4f..f38e8ab8 100644 --- a/packages/events/lib/TimeoutError.js +++ b/packages/events/lib/TimeoutError.js @@ -2,5 +2,6 @@ export default class TimeoutError extends Error { constructor(message) { super(message); this.name = "TimeoutError"; + // Error.captureStackTrace?.(this, this.constructor); } } diff --git a/packages/starttls/test.js b/packages/starttls/test.js index 6c5e618f..0971f2ae 100644 --- a/packages/starttls/test.js +++ b/packages/starttls/test.js @@ -1,16 +1,9 @@ jest.mock("tls"); -import { mockClient, promise, delay } from "@xmpp/test"; +import { mockClient, promise, delay, mockSocket } from "@xmpp/test"; import tls from "tls"; -import net from "net"; import { EventEmitter } from "@xmpp/events"; -function mockSocket() { - const socket = new net.Socket(); - socket.write = (data, cb) => cb(); - return socket; -} - test("success", async () => { const { entity } = mockClient(); entity.socket = mockSocket(); diff --git a/packages/test/index.js b/packages/test/index.js index 72e940ee..4cdb0919 100644 --- a/packages/test/index.js +++ b/packages/test/index.js @@ -5,6 +5,7 @@ import mockClient from "./mockClient.js"; import mockClientCore from "./mockClientCore.js"; import { delay, promise, timeout } from "@xmpp/events"; import id from "@xmpp/id"; +import mockSocket from "./mockSocket.js"; export { context, @@ -17,6 +18,7 @@ export { promise, timeout, id, + mockSocket, }; export function mockInput(entity, el) { diff --git a/packages/test/mockSocket.js b/packages/test/mockSocket.js index d2ca0e59..09f25c72 100644 --- a/packages/test/mockSocket.js +++ b/packages/test/mockSocket.js @@ -1,8 +1,10 @@ -import { EventEmitter } from "@xmpp/events"; +import net from "node:net"; -class MockSocket extends EventEmitter { +class MockSocket extends net.Socket { write(data, cb) { - cb(); + process.nextTick(() => { + cb?.(); + }); } } From a738c4a9281ec080ed80e06a1a2ac1644f41ae16 Mon Sep 17 00:00:00 2001 From: Sonny Piers Date: Fri, 10 Jan 2025 14:29:27 +0100 Subject: [PATCH 36/42] client: Do not allow PLAIN on insecure connection Also add connection.isSecure() method Fixes https://github.com/xmppjs/xmpp.js/issues/1040 --- packages/client-core/lib/Client.js | 2 +- packages/client-core/test/Client.js | 9 +++-- packages/client/README.md | 15 ++++++++ packages/client/lib/createOnAuthenticate.js | 32 ++++++++++------- packages/client/test/getMechanism.js | 38 +++++++++++++++++++++ packages/connection/index.js | 4 +++ packages/connection/test/isSecure.js | 14 ++++++++ packages/sasl/index.js | 2 +- packages/sasl2/index.js | 7 +++- packages/test/mockSocket.js | 3 ++ rollup.config.js | 1 + 11 files changed, 107 insertions(+), 20 deletions(-) create mode 100644 packages/client/test/getMechanism.js create mode 100644 packages/connection/test/isSecure.js diff --git a/packages/client-core/lib/Client.js b/packages/client-core/lib/Client.js index bd9a18a7..b0bdcbc3 100644 --- a/packages/client-core/lib/Client.js +++ b/packages/client-core/lib/Client.js @@ -47,7 +47,7 @@ class Client extends Connection { // after the confidentiality and integrity of the stream are protected via TLS // or an equivalent security layer. // https://xmpp.org/rfcs/rfc6120.html#rfc.section.4.7.1 - const from = this.socket?.isSecure() && this.jid?.bare().toString(); + const from = this.isSecure() && this.jid?.bare().toString(); if (from) headerElement.attrs.from = from; return this.Transport.prototype.header(headerElement, ...args); } diff --git a/packages/client-core/test/Client.js b/packages/client-core/test/Client.js index e510e9cb..77913608 100644 --- a/packages/client-core/test/Client.js +++ b/packages/client-core/test/Client.js @@ -32,21 +32,20 @@ test("header", () => { const entity = new Client(); entity.Transport = Transport; - entity.socket = {}; entity.jid = null; - entity.socket.isSecure = () => false; + entity.isSecure = () => false; expect(entity.header()).toEqual(); entity.jid = null; - entity.socket.isSecure = () => true; + entity.isSecure = () => true; expect(entity.header()).toEqual(); entity.jid = new JID("foo@bar/example"); - entity.socket.isSecure = () => false; + entity.isSecure = () => false; expect(entity.header()).toEqual(); entity.jid = new JID("foo@bar/example"); - entity.socket.isSecure = () => true; + entity.isSecure = () => true; expect(entity.header()).toEqual(); }); diff --git a/packages/client/README.md b/packages/client/README.md index 2ac26d1f..f3a61d35 100644 --- a/packages/client/README.md +++ b/packages/client/README.md @@ -265,6 +265,21 @@ Returns a promise that resolves once all the stanzas have been sent. If you need to send a stanza to multiple recipients we recommend using [Extended Stanza Addressing](https://xmpp.org/extensions/xep-0033.html) instead. +### isSecure + +Returns whether the connection is considered secured. + +```js +console.log(xmpp.isSecure()); +``` + +Considered secure: + +- localhost, 127.0.0.1, ::1 +- encrypted channels (wss, xmpps, starttls) + +This method returns false if there is no connection. + ### xmpp.reconnect See [@xmpp/reconnect](/packages/reconnect). diff --git a/packages/client/lib/createOnAuthenticate.js b/packages/client/lib/createOnAuthenticate.js index 907a180b..8d8c4ea1 100644 --- a/packages/client/lib/createOnAuthenticate.js +++ b/packages/client/lib/createOnAuthenticate.js @@ -1,23 +1,31 @@ const ANONYMOUS = "ANONYMOUS"; +const PLAIN = "PLAIN"; export default function createOnAuthenticate(credentials, userAgent) { - return async function onAuthenticate(authenticate, mechanisms, fast) { + return async function onAuthenticate(authenticate, mechanisms, fast, entity) { if (typeof credentials === "function") { await credentials(authenticate, mechanisms, fast); return; } - if ( - !credentials?.username && - !credentials?.password && - mechanisms.includes(ANONYMOUS) - ) { - await authenticate(credentials, ANONYMOUS, userAgent); - return; - } + credentials.token ??= await fast?.fetch(); - credentials.token = await fast?.fetch?.(); - - await authenticate(credentials, mechanisms[0], userAgent); + const mechanism = getMechanism({ mechanisms, entity, credentials }); + await authenticate(credentials, mechanism, userAgent); }; } + +export function getMechanism({ mechanisms, entity, credentials }) { + if ( + !credentials?.username && + !credentials?.password && + !credentials?.token && + mechanisms.includes(ANONYMOUS) + ) { + return ANONYMOUS; + } + + if (entity.isSecure()) return mechanisms[0]; + + return mechanisms.find((mechanism) => mechanism !== PLAIN); +} diff --git a/packages/client/test/getMechanism.js b/packages/client/test/getMechanism.js new file mode 100644 index 00000000..36deac0d --- /dev/null +++ b/packages/client/test/getMechanism.js @@ -0,0 +1,38 @@ +import { getMechanism } from "../lib/createOnAuthenticate.js"; + +it("returns ANONYMOUS if available and there are no credentials", () => { + expect( + getMechanism({ + credentials: {}, + mechanisms: ["PLAIN", "ANONYMOUS"], + }), + ).toBe("ANONYMOUS"); +}); + +it("returns the first mechanism if the connection is secure", () => { + expect( + getMechanism({ + credentials: { username: "foo", password: "bar" }, + mechanisms: ["PLAIN", "SCRAM-SHA-1"], + entity: { isSecure: () => true }, + }), + ).toBe("PLAIN"); +}); + +it("does not return PLAIN if the connection is not secure", () => { + expect( + getMechanism({ + credentials: { username: "foo", password: "bar" }, + mechanisms: ["PLAIN", "SCRAM-SHA-1"], + entity: { isSecure: () => false }, + }), + ).toBe("SCRAM-SHA-1"); + + expect( + getMechanism({ + credentials: { username: "foo", password: "bar" }, + mechanisms: ["PLAIN"], + entity: { isSecure: () => false }, + }), + ).toBe(undefined); +}); diff --git a/packages/connection/index.js b/packages/connection/index.js index f3b840c4..9adf3cb5 100644 --- a/packages/connection/index.js +++ b/packages/connection/index.js @@ -22,6 +22,10 @@ class Connection extends EventEmitter { this.root = null; } + isSecure() { + return !!this.socket?.isSecure(); + } + async _streamError(condition, children) { try { await this.send( diff --git a/packages/connection/test/isSecure.js b/packages/connection/test/isSecure.js new file mode 100644 index 00000000..e76571d5 --- /dev/null +++ b/packages/connection/test/isSecure.js @@ -0,0 +1,14 @@ +import Connection from "../index.js"; + +test("isSecure()", () => { + const conn = new Connection(); + + conn.socket = null; + expect(conn.isSecure()).toBe(false); + + conn.socket = { isSecure: () => false }; + expect(conn.isSecure()).toBe(false); + + conn.socket = { isSecure: () => true }; + expect(conn.isSecure()).toBe(true); +}); diff --git a/packages/sasl/index.js b/packages/sasl/index.js index 7c47312a..0fad0407 100644 --- a/packages/sasl/index.js +++ b/packages/sasl/index.js @@ -84,7 +84,7 @@ export default function sasl({ streamFeatures, saslFactory }, onAuthenticate) { }); } - await onAuthenticate(done, mechanisms); + await onAuthenticate(done, mechanisms, null, entity); await entity.restart(); }); diff --git a/packages/sasl2/index.js b/packages/sasl2/index.js index d81ddfed..f6d2c86a 100644 --- a/packages/sasl2/index.js +++ b/packages/sasl2/index.js @@ -106,7 +106,12 @@ export default function sasl2({ streamFeatures, saslFactory }, onAuthenticate) { throw new SASLError("SASL: No compatible mechanism available."); } - await onAuthenticate(done, mechanisms, fast_available && fast); + await onAuthenticate( + done, + mechanisms, + fast_available ? fast : null, + entity, + ); async function done(credentials, mechanism, userAgent) { if (fast_available) { diff --git a/packages/test/mockSocket.js b/packages/test/mockSocket.js index 09f25c72..5bb0c455 100644 --- a/packages/test/mockSocket.js +++ b/packages/test/mockSocket.js @@ -6,6 +6,9 @@ class MockSocket extends net.Socket { cb?.(); }); } + isSecure() { + return true; + } } export default function mockSocket() { diff --git a/rollup.config.js b/rollup.config.js index fd709d68..8e905d2c 100644 --- a/rollup.config.js +++ b/rollup.config.js @@ -14,6 +14,7 @@ export default [ name: "XMPP", }, plugins: [ + terser(), babel({ babelHelpers: "runtime" }), nodePolyfills(), nodeResolve({ preferBuiltins: false, browser: true }), From 407da51042494e83879ab389a7c2b66105075e50 Mon Sep 17 00:00:00 2001 From: Sonny Piers Date: Fri, 10 Jan 2025 16:18:15 +0100 Subject: [PATCH 37/42] Replace socket.isSecure with socket.secure This helps in case the socket is proxied (eg WebWorker) --- README.md | 2 +- packages/connection-tcp/Socket.js | 4 +--- packages/connection-tcp/test/Socket.js | 4 ++-- packages/connection/index.js | 2 +- packages/connection/test/isSecure.js | 4 ++-- packages/test/mockSocket.js | 5 ++-- packages/tls/lib/Socket.js | 5 +--- packages/websocket/lib/Socket.js | 28 ++++++++++++++-------- packages/websocket/test/Socket.js | 32 ++++++++++++++++---------- 9 files changed, 48 insertions(+), 38 deletions(-) diff --git a/README.md b/README.md index 4bd5dc95..c5d2e95d 100644 --- a/README.md +++ b/README.md @@ -21,7 +21,7 @@ It aims to run everywhere JavaScript runs and make use of the best network transport available. -xmpp.js is known to be used with Node.js, browsers, React Native, GJS and Duktape. +xmpp.js is known to be used with Node.js, browsers, WebWorker, React Native, Bun, GJS and Duktape. ### reliable diff --git a/packages/connection-tcp/Socket.js b/packages/connection-tcp/Socket.js index 7fd8d457..31171370 100644 --- a/packages/connection-tcp/Socket.js +++ b/packages/connection-tcp/Socket.js @@ -1,7 +1,5 @@ import { Socket as TCPSocket } from "net"; export default class Socket extends TCPSocket { - isSecure() { - return false; - } + secure = false; } diff --git a/packages/connection-tcp/test/Socket.js b/packages/connection-tcp/test/Socket.js index 18177858..f90b8fe1 100644 --- a/packages/connection-tcp/test/Socket.js +++ b/packages/connection-tcp/test/Socket.js @@ -1,9 +1,9 @@ import net from "node:net"; import Socket from "../Socket.js"; -test("isSecure()", () => { +test("secure", () => { const socket = new Socket(); - expect(socket.isSecure()).toBe(false); + expect(socket.secure).toBe(false); }); test("instance of net.Socket", () => { diff --git a/packages/connection/index.js b/packages/connection/index.js index 9adf3cb5..20318d66 100644 --- a/packages/connection/index.js +++ b/packages/connection/index.js @@ -23,7 +23,7 @@ class Connection extends EventEmitter { } isSecure() { - return !!this.socket?.isSecure(); + return this.socket?.secure === true; } async _streamError(condition, children) { diff --git a/packages/connection/test/isSecure.js b/packages/connection/test/isSecure.js index e76571d5..242a58ba 100644 --- a/packages/connection/test/isSecure.js +++ b/packages/connection/test/isSecure.js @@ -6,9 +6,9 @@ test("isSecure()", () => { conn.socket = null; expect(conn.isSecure()).toBe(false); - conn.socket = { isSecure: () => false }; + conn.socket = { secure: false }; expect(conn.isSecure()).toBe(false); - conn.socket = { isSecure: () => true }; + conn.socket = { secure: true }; expect(conn.isSecure()).toBe(true); }); diff --git a/packages/test/mockSocket.js b/packages/test/mockSocket.js index 5bb0c455..8d93a088 100644 --- a/packages/test/mockSocket.js +++ b/packages/test/mockSocket.js @@ -1,14 +1,13 @@ import net from "node:net"; class MockSocket extends net.Socket { + secure = true; + write(data, cb) { process.nextTick(() => { cb?.(); }); } - isSecure() { - return true; - } } export default function mockSocket() { diff --git a/packages/tls/lib/Socket.js b/packages/tls/lib/Socket.js index b8b0338a..93eef98d 100644 --- a/packages/tls/lib/Socket.js +++ b/packages/tls/lib/Socket.js @@ -5,10 +5,7 @@ class Socket extends EventEmitter { timeout = null; #listeners = null; socket = null; - - isSecure() { - return true; - } + secure = true; connect(...args) { this._attachSocket(tls.connect(...args)); diff --git a/packages/websocket/lib/Socket.js b/packages/websocket/lib/Socket.js index 69647fdb..63b0f8d9 100644 --- a/packages/websocket/lib/Socket.js +++ b/packages/websocket/lib/Socket.js @@ -3,21 +3,22 @@ import { parseURI } from "@xmpp/connection/lib/util.js"; const CODE = "ECONNERROR"; +export function isSecure(url) { + const uri = parseURI(url); + if (uri.protocol === "wss:") return true; + if (["localhost", "127.0.0.1", "::1"].includes(uri.hostname)) return true; + return false; +} + export default class Socket extends EventEmitter { #listeners = null; socket = null; url = null; - - isSecure() { - if (!this.url) return false; - const uri = parseURI(this.url); - if (uri.protocol === "wss:") return true; - if (["localhost", "127.0.0.1", "::1"].includes(uri.hostname)) return true; - return false; - } + secure = false; connect(url) { this.url = url; + this.secure = isSecure(url); // eslint-disable-next-line n/no-unsupported-features/node-builtins this._attachSocket(new WebSocket(url, ["xmpp"])); } @@ -52,6 +53,7 @@ export default class Socket extends EventEmitter { _detachSocket() { this.url = null; + this.secure = false; this.socket && this.#listeners?.unsubscribe(this.socket); this.socket = null; } @@ -65,12 +67,18 @@ export default class Socket extends EventEmitter { } write(data, fn) { + function done(err) { + if (!fn) return; + // eslint-disable-next-line promise/catch-or-return, promise/no-promise-in-callback + Promise.resolve().then(() => fn(err)); + } + try { this.socket.send(data); } catch (err) { - fn?.(err); + done(err); return; } - fn?.(); + done(); } } diff --git a/packages/websocket/test/Socket.js b/packages/websocket/test/Socket.js index 9f8d4c5d..ea8ad7cc 100644 --- a/packages/websocket/test/Socket.js +++ b/packages/websocket/test/Socket.js @@ -1,21 +1,29 @@ +import { EventEmitter } from "@xmpp/events"; import Socket from "../lib/Socket.js"; -test("isSecure", () => { +// eslint-disable-next-line n/no-unsupported-features/node-builtins +globalThis.WebSocket = EventEmitter; + +test("secure", () => { const socket = new Socket(); - expect(socket.isSecure()).toBe(false); - socket.url = "ws://example.com/foo"; - expect(socket.isSecure()).toBe(false); + expect(socket.secure).toBe(false); + + socket.connect("ws://example.com/foo"); + expect(socket.secure).toBe(false); + + socket.connect("ws://localhost/foo"); + expect(socket.secure).toBe(true); - socket.url = "ws://localhost/foo"; - expect(socket.isSecure()).toBe(true); + socket.connect("ws://127.0.0.1/foo"); + expect(socket.secure).toBe(true); - socket.url = "ws://127.0.0.1/foo"; - expect(socket.isSecure()).toBe(true); + socket.connect("ws://[::1]/foo"); + expect(socket.secure).toBe(true); - socket.url = "ws://[::1]/foo"; - expect(socket.isSecure()).toBe(true); + socket.connect("wss://example.com/foo"); + expect(socket.secure).toBe(true); - socket.url = "wss://example.com/foo"; - expect(socket.isSecure()).toBe(true); + socket.socket.emit("close", { wasClean: Math.random > 0.5 }); + expect(socket.secure).toBe(false); }); From 44ae9c90294cc21770997934597a2fb32c92e8ac Mon Sep 17 00:00:00 2001 From: Sonny Piers Date: Sun, 12 Jan 2025 12:58:31 +0100 Subject: [PATCH 38/42] Bump ltx --- package-lock.json | 10 ++++++---- packages/debug/package.json | 2 +- packages/test/package.json | 2 +- packages/xml/package.json | 2 +- 4 files changed, 9 insertions(+), 7 deletions(-) diff --git a/package-lock.json b/package-lock.json index 894db09f..fd364b3e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10028,7 +10028,9 @@ } }, "node_modules/ltx": { - "version": "3.1.1", + "version": "3.1.2", + "resolved": "https://registry.npmjs.org/ltx/-/ltx-3.1.2.tgz", + "integrity": "sha512-tFSKojN92FqNK6eRTmKK/ROUTUYVWKAxgohz523TPhF1G3nR3DXQS/I7/705rEPrDSloKDgMdRlh0qgMFQoVYw==", "license": "MIT", "engines": { "node": ">= 12.4.0" @@ -13646,7 +13648,7 @@ "license": "ISC", "dependencies": { "@xmpp/xml": "^0.14.0", - "ltx": "^3.1.1" + "ltx": "^3.1.2" }, "engines": { "node": ">= 20.10" @@ -13890,7 +13892,7 @@ "@xmpp/id": "^0.14.0", "@xmpp/jid": "^0.14.0", "@xmpp/xml": "^0.14.0", - "ltx": "^3.1.1" + "ltx": "^3.1.2" }, "engines": { "node": ">= 20.10" @@ -13948,7 +13950,7 @@ "license": "ISC", "dependencies": { "@xmpp/events": "^0.14.0", - "ltx": "^3.1.1" + "ltx": "^3.1.2" }, "engines": { "node": ">= 20.10" diff --git a/packages/debug/package.json b/packages/debug/package.json index 3e22f377..4532c3cd 100644 --- a/packages/debug/package.json +++ b/packages/debug/package.json @@ -17,7 +17,7 @@ }, "dependencies": { "@xmpp/xml": "^0.14.0", - "ltx": "^3.1.1" + "ltx": "^3.1.2" }, "publishConfig": { "access": "public" diff --git a/packages/test/package.json b/packages/test/package.json index aa63b797..b4bcf9b5 100644 --- a/packages/test/package.json +++ b/packages/test/package.json @@ -24,7 +24,7 @@ "@xmpp/id": "^0.14.0", "@xmpp/jid": "^0.14.0", "@xmpp/xml": "^0.14.0", - "ltx": "^3.1.1" + "ltx": "^3.1.2" }, "publishConfig": { "access": "public" diff --git a/packages/xml/package.json b/packages/xml/package.json index 59c48588..dfd83345 100644 --- a/packages/xml/package.json +++ b/packages/xml/package.json @@ -17,7 +17,7 @@ ], "dependencies": { "@xmpp/events": "^0.14.0", - "ltx": "^3.1.1" + "ltx": "^3.1.2" }, "engines": { "node": ">= 20.10" From 9cb0556a175b198bf0fca41bd40f9b6044d74046 Mon Sep 17 00:00:00 2001 From: Sonny Piers Date: Sun, 12 Jan 2025 13:00:06 +0100 Subject: [PATCH 39/42] fast: Emit error events on entity At least on the web, emitting an error on fast will stop processing. Unless there is an error event listener on it but adding error listener on all "plugins" is a bit cumbersome. So let's always emit errors on entity instead. --- packages/client-core/src/fast/fast.js | 8 ++++---- packages/client/index.js | 1 + 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/client-core/src/fast/fast.js b/packages/client-core/src/fast/fast.js index 9b064b05..6fb629d1 100644 --- a/packages/client-core/src/fast/fast.js +++ b/packages/client-core/src/fast/fast.js @@ -5,7 +5,7 @@ import SASLFactory from "saslmechanisms"; const NS = "urn:xmpp:fast:0"; -export default function fast({ sasl2 }) { +export default function fast({ sasl2, entity }) { const saslFactory = new SASLFactory(); let token; @@ -24,14 +24,14 @@ export default function fast({ sasl2 }) { try { await this.saveToken(token); } catch (err) { - fast.emit("error", err); + entity.emit("error", err); } }, async fetch() { try { return this.fetchToken(); } catch (err) { - fast.emit("error", err); + entity.emit("error", err); } }, saslFactory, @@ -68,7 +68,7 @@ export default function fast({ sasl2 }) { }); return true; } catch (err) { - fast.emit("error", err); + entity.emit("error", err); return false; } }, diff --git a/packages/client/index.js b/packages/client/index.js index 9888f5af..ddf70fa4 100644 --- a/packages/client/index.js +++ b/packages/client/index.js @@ -70,6 +70,7 @@ function client(options = {}) { const fast = _fast({ sasl2, + entity, }); sasl2.setup({ fast }); From ced205776b931803ead4f8693c917cc8d70e7a8c Mon Sep 17 00:00:00 2001 From: Sonny Piers Date: Sun, 12 Jan 2025 13:05:09 +0100 Subject: [PATCH 40/42] client: pass all arguments to credentials function --- packages/client/lib/createOnAuthenticate.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/client/lib/createOnAuthenticate.js b/packages/client/lib/createOnAuthenticate.js index 8d8c4ea1..e795c150 100644 --- a/packages/client/lib/createOnAuthenticate.js +++ b/packages/client/lib/createOnAuthenticate.js @@ -2,12 +2,14 @@ const ANONYMOUS = "ANONYMOUS"; const PLAIN = "PLAIN"; export default function createOnAuthenticate(credentials, userAgent) { - return async function onAuthenticate(authenticate, mechanisms, fast, entity) { + return async function onAuthenticate(...args) { if (typeof credentials === "function") { - await credentials(authenticate, mechanisms, fast); + await credentials(...args); return; } + const [authenticate, mechanisms, fast, entity] = args; + credentials.token ??= await fast?.fetch(); const mechanism = getMechanism({ mechanisms, entity, credentials }); From 893533608ea250193a46b2af4db8fa7a6882f5b1 Mon Sep 17 00:00:00 2001 From: Sonny Piers Date: Wed, 15 Jan 2025 13:12:09 +0100 Subject: [PATCH 41/42] we dont need forceReconnect or socket.destory anymore --- packages/connection/index.js | 15 ---- packages/stream-management/index.js | 2 +- packages/tls/lib/Socket.js | 4 - packages/websocket/lib/Socket.js | 4 - test/stream-management.js | 22 +++--- test/stream-management.test.js | 112 ++++++++++++++++++++++++++++ 6 files changed, 126 insertions(+), 33 deletions(-) create mode 100644 test/stream-management.test.js diff --git a/packages/connection/index.js b/packages/connection/index.js index 20318d66..5c1cfcbb 100644 --- a/packages/connection/index.js +++ b/packages/connection/index.js @@ -245,21 +245,6 @@ class Connection extends EventEmitter { await promise(this.socket, "close", "error", timeout); } - /** - * Forcibly disconnects the socket - * https://xmpp.org/rfcs/rfc6120.html#streams-close - * https://tools.ietf.org/html/rfc7395#section-3.6 - */ - async forceDisconnect(timeout = this.timeout) { - if (!this.socket) return; - - this._status("disconnecting"); - this.socket.destroy(); - - // The 'disconnect' status is set by the socket 'close' listener - await promise(this.socket, "close", "error", timeout); - } - /** * Opens the stream */ diff --git a/packages/stream-management/index.js b/packages/stream-management/index.js index 26eb9f37..78d76f23 100644 --- a/packages/stream-management/index.js +++ b/packages/stream-management/index.js @@ -154,7 +154,7 @@ export default function streamManagement({ clearTimeout(timeoutTimeout); if (sm.timeout) { timeoutTimeout = setTimeout( - () => entity.forceDisconnect().catch(), + () => entity.disconnect().catch(() => {}), sm.timeout, ); } diff --git a/packages/tls/lib/Socket.js b/packages/tls/lib/Socket.js index 93eef98d..f96ac449 100644 --- a/packages/tls/lib/Socket.js +++ b/packages/tls/lib/Socket.js @@ -50,10 +50,6 @@ class Socket extends EventEmitter { this.socket.end(); } - destroy() { - this.socket.destroy(); - } - write(data, fn) { this.socket.write(data, fn); } diff --git a/packages/websocket/lib/Socket.js b/packages/websocket/lib/Socket.js index 63b0f8d9..84a8ba04 100644 --- a/packages/websocket/lib/Socket.js +++ b/packages/websocket/lib/Socket.js @@ -62,10 +62,6 @@ export default class Socket extends EventEmitter { this.socket.close(); } - destroy() { - this.socket.close(); - } - write(data, fn) { function done(err) { if (!fn) return; diff --git a/test/stream-management.js b/test/stream-management.js index 5d8aacca..ef66faed 100644 --- a/test/stream-management.js +++ b/test/stream-management.js @@ -46,10 +46,12 @@ test("client fail stanzas", async () => { await xmpp.start(); // Expect send but don't actually send to server, so it will fail await xmpp.streamManagement.outbound_q.push({ - stanza: - - , - stamp: datetime() + stanza: ( + + + + ), + stamp: datetime(), }); await xmpp.stop(); @@ -68,10 +70,12 @@ test("client retry stanzas", async () => { await xmpp.start(); // Add to queue but don't actually send so it can retry after disconnect await xmpp.streamManagement.outbound_q.push({ - stanza: - - , - stamp: datetime() + stanza: ( + + + + ), + stamp: datetime(), }); await xmpp.disconnect(); @@ -79,7 +83,7 @@ test("client retry stanzas", async () => { expect(el.attrs.id).toEqual("ping"); }); -test("client reconnect automatically", async () => { +test("client reconnects when server fails to ack", async () => { await server.enableModules(["smacks"]); await server.restart(); diff --git a/test/stream-management.test.js b/test/stream-management.test.js new file mode 100644 index 00000000..fa5e055a --- /dev/null +++ b/test/stream-management.test.js @@ -0,0 +1,112 @@ +import { client } from "../packages/client/index.js"; +import { promise } from "../packages/events/index.js"; +import { datetime } from "../packages/time/index.js"; +import debug from "../packages/debug/index.js"; +import server from "../server/index.js"; + +const username = "client"; +const password = "foobar"; +const credentials = { username, password }; +const domain = "localhost"; + +let xmpp; + +afterEach(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); + + const promise_ack = promise(xmpp.streamManagement, "ack"); + await xmpp.start(); + await xmpp.send( + + + , + ); + + const el = await promise_ack; + expect(el.attrs.id).toEqual("ping"); +}); + +test("client fail stanzas", async () => { + await server.enableModules(["smacks"]); + await server.restart(); + + xmpp = client({ credentials, service: domain }); + debug(xmpp); + + const promise_fail = promise(xmpp.streamManagement, "fail"); + await xmpp.start(); + // Expect send but don't actually send to server, so it will fail + await xmpp.streamManagement.outbound_q.push({ + stanza: ( + + + + ), + stamp: datetime(), + }); + await xmpp.stop(); + + const el = await promise_fail; + expect(el.attrs.id).toEqual("ping"); +}); + +test("client retry stanzas", async () => { + await server.enableModules(["smacks"]); + await server.restart(); + + xmpp = client({ credentials, service: domain }); + debug(xmpp); + + const promise_ack = promise(xmpp.streamManagement, "ack"); + await xmpp.start(); + // Add to queue but don't actually send so it can retry after disconnect + await xmpp.streamManagement.outbound_q.push({ + stanza: ( + + + + ), + stamp: datetime(), + }); + await xmpp.disconnect(); + + const el = await promise_ack; + expect(el.attrs.id).toEqual("ping"); +}); + +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.debounceAckRequest = 1; + debug(xmpp, true); + + const promise_resumed = promise(xmpp.streamManagement, "resumed"); + await xmpp.start(); + xmpp.send( + + + , + ); + // Pretend we don't receive the ack by removing event listeners + // on the socket + xmpp._detachSocket(); + + await promise_resumed; + expect().pass(); + }, + 1000 * 10, +); From 26228e7b62e7b36041891ad7a322ae1ba689eb16 Mon Sep 17 00:00:00 2001 From: Sonny Piers Date: Thu, 16 Jan 2025 10:30:15 +0100 Subject: [PATCH 42/42] fix --- packages/stream-management/README.md | 12 +++++----- .../stream-management/stream-features.test.js | 23 ++----------------- test/stream-management.test.js | 3 +++ 3 files changed, 11 insertions(+), 27 deletions(-) diff --git a/packages/stream-management/README.md b/packages/stream-management/README.md index d6bb4185..8bb8bbd5 100644 --- a/packages/stream-management/README.md +++ b/packages/stream-management/README.md @@ -24,9 +24,9 @@ Indicates that the connection was resumed. When that happens the `online` event const xmpp = client(...); const {streamManagement} = xmpp; -streamManagement.on('resumed', ()) => { +streamManagement.on('resumed', () => { console.log("session resumed"); -} +}); ``` ### fail @@ -37,9 +37,9 @@ Indicates that a stanza failed to send to the server and will not be retried. const xmpp = client(...); const {streamManagement} = xmpp; -streamManagement.on('fail', (stanza)) => { +streamManagement.on('fail', (stanza) => { console.log("fail to send", stanza.toString()); -} +}); ``` ### ack @@ -50,9 +50,9 @@ Indicates that a stanza has been acknowledged by the server. const xmpp = client(...); const {streamManagement} = xmpp; -streamManagement.on('ack', (stanza)) => { +streamManagement.on('ack', (stanza) => { console.log("stanza acknowledge by the server", stanza.toString()); -} +}); ``` ## References diff --git a/packages/stream-management/stream-features.test.js b/packages/stream-management/stream-features.test.js index 3eefbbe9..4728a1a1 100644 --- a/packages/stream-management/stream-features.test.js +++ b/packages/stream-management/stream-features.test.js @@ -131,30 +131,11 @@ test("enable - failed", async () => { test("stanza ack", async () => { const { entity } = mockClient(); - entity.mockInput( - - - , - ); - - expect(await entity.catchOutgoing()).toEqual( - , - ); - - entity.mockInput( - , - ); - - await tick(); + entity.streamManagement.enabled = true; expect(entity.streamManagement.outbound).toBe(0); expect(entity.streamManagement.outbound_q).toBeEmpty(); - expect(entity.streamManagement.enabled).toBe(true); + // expect(entity.streamManagement.enabled).toBe(true); await entity.send(); diff --git a/test/stream-management.test.js b/test/stream-management.test.js index fa5e055a..3033ded8 100644 --- a/test/stream-management.test.js +++ b/test/stream-management.test.js @@ -77,6 +77,8 @@ test("client retry stanzas", async () => { ), stamp: datetime(), }); + // Do not close the stream so that stream resumption can happen + await xmpp._closeSocket(); await xmpp.disconnect(); const el = await promise_ack; @@ -101,6 +103,7 @@ test( , ); + // Pretend we don't receive the ack by removing event listeners // on the socket xmpp._detachSocket();