Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Various code improvements #1053

Merged
merged 1 commit into from
Jan 9, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 10 additions & 25 deletions packages/connection/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ class Connection extends EventEmitter {

_onData(data) {
const str = data.toString("utf8");
this.emit("input", str);
this.parser.write(str);
}

Expand Down Expand Up @@ -284,15 +283,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
}

Expand All @@ -319,23 +312,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()));
});
}

Expand Down
22 changes: 12 additions & 10 deletions packages/connection/test/_closeStream.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(`<goodbye/>`);
});

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");
});
5 changes: 1 addition & 4 deletions packages/connection/test/onData.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import Connection from "../index.js";

test("#_onData", () => {
expect.assertions(2);
expect.assertions(1);
const foo = "<foo>";
const conn = new Connection();
conn.parser = {
Expand All @@ -10,8 +10,5 @@ test("#_onData", () => {
},
};

conn.on("input", (data) => {
expect(data).toBe(foo);
});
conn._onData(foo);
});
1 change: 1 addition & 0 deletions packages/events/lib/TimeoutError.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,6 @@ export default class TimeoutError extends Error {
constructor(message) {
super(message);
this.name = "TimeoutError";
// Error.captureStackTrace?.(this, this.constructor);
}
}
9 changes: 1 addition & 8 deletions packages/starttls/test.js
Original file line number Diff line number Diff line change
@@ -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();
Expand Down
2 changes: 2 additions & 0 deletions packages/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -17,6 +18,7 @@ export {
promise,
timeout,
id,
mockSocket,
};

export function mockInput(entity, el) {
Expand Down
8 changes: 5 additions & 3 deletions packages/test/mockSocket.js
Original file line number Diff line number Diff line change
@@ -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?.();
});
}
}

Expand Down
Loading