From 8ad2205a9ca62d1cc3ba7a3abdd95bc9d604fd2d Mon Sep 17 00:00:00 2001 From: Sonny Piers Date: Wed, 8 Jan 2025 17:25:58 +0100 Subject: [PATCH] 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 c7e254ae..01f0d329 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(); @@ -322,9 +315,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(); });