Skip to content
Open
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
10 changes: 6 additions & 4 deletions src/js/node/dgram.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ const {
const { isIP } = require("internal/net/isIP");

const EventEmitter = require("node:events");
// Read once at load. The export is writable, and a listener under any other key would count as an 'error' handler.
const kErrorMonitor = EventEmitter.errorMonitor;
Comment thread
robobun marked this conversation as resolved.
Resolved

const { deprecate } = require("internal/util/deprecate");

Expand Down Expand Up @@ -689,7 +691,7 @@ Socket.prototype.bind = function (this: $Node.DgramSocket, port_, address_ /* ,
const cb: unknown = arguments.length && arguments[arguments.length - 1];
if (typeof cb === "function") {
function removeListeners(this: $Node.DgramSocket) {
this.removeListener("error", removeListeners);
this.removeListener(kErrorMonitor, removeListeners);
this.removeListener("listening", onListening);
}

Expand All @@ -698,7 +700,7 @@ Socket.prototype.bind = function (this: $Node.DgramSocket, port_, address_ /* ,
(cb as (this: $Node.DgramSocket) => void).$call(this);
}

this.on("error", removeListeners);
this.on(kErrorMonitor, removeListeners);
this.on("listening", onListening);
}

Expand Down Expand Up @@ -1099,14 +1101,14 @@ function enqueue(self: $Node.DgramSocket, toEnqueue: () => void) {
// event handler that flushes the send queue after binding is done.
if (state.queue === undefined) {
state.queue = [];
self.once(EventEmitter.errorMonitor, onListenError);
self.once(kErrorMonitor, onListenError);
self.once("listening", onListenSuccess);
}
state.queue.push(toEnqueue);
}

function onListenSuccess(this: $Node.DgramSocket) {
this.removeListener(EventEmitter.errorMonitor, onListenError);
this.removeListener(kErrorMonitor, onListenError);
clearQueue.$call(this);
}

Expand Down
21 changes: 21 additions & 0 deletions test/js/bun/udp/dgram-cluster-bind-callback-error-fixture.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

129 changes: 129 additions & 0 deletions test/js/bun/udp/dgram.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { describe, expect, jest, test } from "bun:test";
import { createSocket } from "dgram";
import { errorMonitor } from "node:events";
import { Worker } from "node:worker_threads";

import { bunEnv, bunExe, bunRun, disableAggressiveGCScope, isWindows } from "harness";
Expand Down Expand Up @@ -261,6 +262,14 @@ describe.skipIf(isWindows)("cluster", () => {
expect(stdout).toStartWith("ok: all 4 workers adopted and released the shared descriptor ");
expect(exitCode).toBe(0);
}, 40_000);

test("a bind error from the primary is raised in a worker that passed a callback and has no 'error' listener", async () => {
const { stdout, stderr, exitCode } = await runClusterFixture(
"dgram-cluster-bind-callback-error-fixture.ts",
20_000,
);
expect({ stdout, stderr, exitCode }).toEqual({ stdout: "uncaughtException UNKNOWN\n", stderr: "", exitCode: 0 });
});
});

describe("after close()", () => {
Expand Down Expand Up @@ -359,6 +368,126 @@ describe("bind()", () => {
await listening;
expect(onError).not.toHaveBeenCalled();
});

// bind(..., callback) drops the callback when the bind fails. It watches for
// that through events.errorMonitor, as Node does: a listener under 'error'
// would count as the program's own and hide the failure from a program that
// has none.
test("with a callback watches for the failure through errorMonitor, not an 'error' listener", async () => {
await using socket = createSocket("udp4");
const counts = () => ({
error: socket.listenerCount("error"),
errorMonitor: socket.listenerCount(errorMonitor),
listening: socket.listenerCount("listening"),
});
const { promise: inCallback, resolve } = Promise.withResolvers<ReturnType<typeof counts>>();
socket.bind(0, "127.0.0.1", () => resolve(counts()));
const pending = counts();
expect({ pending, inCallback: await inCallback }).toEqual({
pending: { error: 0, errorMonitor: 1, listening: 1 },
inCallback: { error: 0, errorMonitor: 0, listening: 0 },
});
});

test("with a callback and no 'error' listener, a lookup that fails synchronously throws from bind()", () => {
const failure = new Error("lookup failed");
const socket = createSocket({ type: "udp4", lookup: (_hostname, _family, callback) => callback(failure, "", 0) });
const callback = jest.fn();
let thrown: unknown;
try {
socket.bind(0, "host.invalid", callback);
} catch (err) {
thrown = err;
}
socket.close();
expect(thrown).toBe(failure);
expect(callback).not.toHaveBeenCalled();
});

// Each child installs both process handlers, and a handler prints the name
// of its event and the error code. The socket that holds the taken port is
// unref'd, so a child that hears nothing exits with no output.
const prelude = `
const dgram = require("node:dgram");
process.on("uncaughtException", err => console.log("uncaughtException", err.code));
process.on("unhandledRejection", err => console.log("unhandledRejection", err?.code));
function withTakenPort(fn) {
const holder = dgram.createSocket("udp4");
holder.bind(0, "127.0.0.1", () => {
holder.unref();
fn(holder.address().port);
});
}
`;

test.concurrent.each([
[
"bind(port, address, callback) raises the error",
`withTakenPort(port => {
dgram.createSocket("udp4").bind(port, "127.0.0.1", () => console.log("callback"));
});`,
"uncaughtException EADDRINUSE",
],
[
"bind({ port, address }, callback) raises the error",
`withTakenPort(port => {
dgram.createSocket("udp4").bind({ port, address: "127.0.0.1" }, () => console.log("callback"));
});`,
"uncaughtException EADDRINUSE",
],
[
"a send() queued behind the bind is dropped and the error is raised",
`withTakenPort(port => {
const socket = dgram.createSocket("udp4");
socket.bind(port, "127.0.0.1", () => console.log("callback"));
socket.send("x", port, "127.0.0.1", () => console.log("send callback"));
});`,
"uncaughtException EADDRINUSE",
],
[
"an overwritten events.errorMonitor export changes nothing",
`withTakenPort(port => {
require("node:events").errorMonitor = "error";
const socket = dgram.createSocket("udp4");
socket.bind(port, "127.0.0.1", () => console.log("callback"));
socket.send("x", port, "127.0.0.1", () => console.log("send callback"));
});`,
"uncaughtException EADDRINUSE",
],
[
"a lookup that fails asynchronously raises its error",
`const failure = Object.assign(new Error("lookup failed"), { code: "ELOOKUP" });
const lookup = (hostname, family, callback) => process.nextTick(callback, failure);
dgram.createSocket({ type: "udp4", lookup }).bind(0, "host.invalid", () => console.log("callback"));`,
"uncaughtException ELOOKUP",
],
[
"an 'error' listener of the program hears the error and nothing is raised",
`withTakenPort(port => {
const socket = dgram.createSocket("udp4");
socket.on("error", err => console.log("error listener", err.code));
socket.bind(port, "127.0.0.1", () => console.log("callback"));
});`,
"error listener EADDRINUSE",
],
])("a failed bind with a callback: %s", async (_name, body, expected) => {
expect(await bunRun(["-e", prelude + body])).toSpawn(expected);
});

// Which of the two process events carries this error is a separate
// difference from Node, so the row takes either.
test.concurrent("a failed bind with a callback: a receive buffer size that cannot be applied is raised", async () => {
const body = `
const socket = dgram.createSocket({ type: "udp4", recvBufferSize: 2 ** 31 });
socket.unref();
socket.bind(0, "127.0.0.1", () => console.log("callback"));`;
const { stdout, stderr, exitCode } = await bunRun(["-e", prelude + body]);
expect({ stdout: stdout.replace(/^(uncaughtException|unhandledRejection) /, ""), stderr, exitCode }).toEqual({
Comment thread
robobun marked this conversation as resolved.
Resolved
stdout: "ERR_SOCKET_BUFFER_SIZE",
stderr: "",
exitCode: 0,
});
});
});

// _createSocketHandle's three return shapes, from node's
Expand Down
23 changes: 23 additions & 0 deletions test/js/node/test/parallel/test-dgram-bind-error-callback.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
'use strict';
const common = require('../common');
const assert = require('node:assert');
const dgram = require('node:dgram');

// Ensure that bind errors (e.g. EADDRINUSE) are not silently swallowed
// when socket.bind() is called with a callback but without a user
// 'error' handler.

const socket1 = dgram.createSocket('udp4');

socket1.bind(0, common.mustCall(() => {
const { port } = socket1.address();
const socket2 = dgram.createSocket('udp4');

process.on('uncaughtException', common.mustCall((err) => {
assert.strictEqual(err.code, 'EADDRINUSE');
socket1.close();
socket2.close();
}));

socket2.bind({ port }, common.mustNotCall());
}));
Loading