From 2d6e46fe00fba6112be55a3f166d4f44da75d067 Mon Sep 17 00:00:00 2001 From: Sonny Date: Thu, 9 Jan 2025 23:52:06 +0100 Subject: [PATCH] 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?.(); + }); } }