From 3870e69eb31754b481dd131d9b30a0fb9d9d8557 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 24 Jul 2026 22:30:19 +0000 Subject: [PATCH 1/4] node:domain: return the callback's value from bind() and intercept() The wrappers returned by d.bind(fn) and d.intercept(fn) called fn but dropped its return value, and invoked it with this === null instead of the caller's receiver. gulp's async-done wraps every task via d.bind(task) and checks the return value to detect a promise or stream, so under bun the promise was never seen and gulp reported "Did you forget to signal async completion?". Both wrappers now enter/exit the domain around the call, forward the caller's this and arguments, and return the callback's result. intercept() now only diverts to the domain's 'error' listener when the first argument is an Error instance (with the domainBound/domainThrown/ domain properties Node sets), and bind() attaches the .domain property to the returned function, matching Node. Fixes #5923 Fixes #24287 --- src/js/node/domain.ts | 49 ++++++--- test/js/node/domain/domain.test.ts | 162 +++++++++++++++++++++++++++++ 2 files changed, 198 insertions(+), 13 deletions(-) create mode 100644 test/js/node/domain/domain.test.ts diff --git a/src/js/node/domain.ts b/src/js/node/domain.ts index c55fdfb78831..bd8f3aad0a9e 100644 --- a/src/js/node/domain.ts +++ b/src/js/node/domain.ts @@ -34,26 +34,49 @@ domain.createDomain = domain.create = function () { emitter.removeListener("error", emitError); }; d.bind = function (fn) { - return function () { - var args = Array.prototype.slice.$call(arguments); + function runBound() { + d.enter(); try { - fn.$apply(null, args); + return fn.$apply(this, arguments); } catch (err) { emitError(err); + } finally { + d.exit(); } - }; + } + ObjectDefineProperty(runBound, "domain", { + __proto__: null, + configurable: true, + enumerable: false, + value: d, + writable: true, + }); + return runBound; }; d.intercept = function (fn) { - return function (err) { - if (err) { + return function runIntercepted() { + var er = arguments[0]; + if (er && er instanceof Error) { + er.domainBound = fn; + er.domainThrown = false; + ObjectDefineProperty(er, "domain", { + __proto__: null, + configurable: true, + enumerable: false, + value: d, + writable: true, + }); + d.emit("error", er); + return; + } + var args = Array.prototype.slice.$call(arguments, 1); + d.enter(); + try { + return fn.$apply(this, args); + } catch (err) { emitError(err); - } else { - var args = Array.prototype.slice.$call(arguments, 1); - try { - fn.$apply(null, args); - } catch (err) { - emitError(err); - } + } finally { + d.exit(); } }; }; diff --git a/test/js/node/domain/domain.test.ts b/test/js/node/domain/domain.test.ts new file mode 100644 index 000000000000..c8f6a8ff1071 --- /dev/null +++ b/test/js/node/domain/domain.test.ts @@ -0,0 +1,162 @@ +import { describe, test, expect } from "bun:test"; +import { bunEnv, bunExe } from "harness"; +import domain from "node:domain"; + +describe("domain.bind()", () => { + test("returns the callback's return value", () => { + const d = domain.create(); + const bound = d.bind(() => 42); + expect(bound()).toBe(42); + }); + + test("forwards the caller's this and arguments", () => { + const d = domain.create(); + const receiver = { tag: "rx" }; + const bound = d.bind(function (this: any, a: number, b: number) { + return [this, a, b]; + }); + expect(bound.call(receiver, 1, 2)).toEqual([receiver, 1, 2]); + }); + + test("makes the domain active while the callback runs", () => { + const d = domain.create(); + let inside; + const bound = d.bind(() => { + inside = process.domain; + }); + expect(process.domain == null).toBe(true); + bound(); + expect(inside).toBe(d); + expect(process.domain == null).toBe(true); + }); + + test("sets .domain on the returned function", () => { + const d = domain.create(); + const bound = d.bind(() => {}); + expect((bound as any).domain).toBe(d); + }); + + test("routes a thrown error to the domain's 'error' listener", () => { + const d = domain.create(); + let caught; + d.on("error", (e: any) => { + caught = e; + }); + const bound = d.bind(() => { + throw new Error("boom"); + }); + expect(bound()).toBeUndefined(); + expect(caught).toBeInstanceOf(Error); + expect((caught as Error).message).toBe("boom"); + }); +}); + +describe("domain.intercept()", () => { + test("returns the callback's return value", () => { + const d = domain.create(); + const intercepted = d.intercept(() => 99); + expect(intercepted(null)).toBe(99); + }); + + test("drops the leading (error) argument before invoking the callback", () => { + const d = domain.create(); + const receiver = { tag: "rx" }; + const intercepted = d.intercept(function (this: any, ...args: unknown[]) { + return [this, ...args]; + }); + expect(intercepted.call(receiver, null, 1, 2)).toEqual([receiver, 1, 2]); + }); + + test("emits on the domain when the first argument is an Error", () => { + const d = domain.create(); + let caught: any; + d.on("error", (e: any) => { + caught = e; + }); + const fn = (..._args: unknown[]) => { + throw new Error("should not run"); + }; + const intercepted = d.intercept(fn); + const err = new Error("boom"); + expect(intercepted(err, 1, 2)).toBeUndefined(); + expect(caught).toBe(err); + expect(caught.domain).toBe(d); + expect(caught.domainBound).toBe(fn); + expect(caught.domainThrown).toBe(false); + }); + + test("does not treat a truthy non-Error first argument as an error", () => { + const d = domain.create(); + let caught; + d.on("error", (e: any) => { + caught = e; + }); + const intercepted = d.intercept((...args: unknown[]) => args); + expect(intercepted("not-an-error", 1, 2)).toEqual([1, 2]); + expect(caught).toBeUndefined(); + }); + + test("makes the domain active while the callback runs", () => { + const d = domain.create(); + let inside; + d.intercept(() => { + inside = process.domain; + })(null); + expect(inside).toBe(d); + }); +}); + +describe("domain.run()", () => { + test("returns the callback's return value and forwards arguments", () => { + const d = domain.create(); + expect( + d.run(function (this: any, a: string) { + return [this === d, a]; + }, "x"), + ).toEqual([true, "x"]); + }); +}); + +// https://github.com/oven-sh/bun/issues/5923 +// https://github.com/oven-sh/bun/issues/24287 +// gulp's async-done wraps each task via d.bind(task) and inspects the return +// value to decide whether it got a promise/stream/observable back. When bind() +// dropped the return value the promise was never awaited and gulp reported +// "Did you forget to signal async completion?". +test.concurrent("async-done style: d.bind(fn)() surfaces a returned promise", async () => { + const src = ` + const domain = require("node:domain"); + + function asyncDone(fn, cb) { + const d = domain.create(); + d.once("error", cb); + const bound = d.bind(fn); + const result = bound(cb); + if (result && typeof result.then === "function") { + result.then(r => cb(null, r), cb); + } + } + + asyncDone( + () => Promise.resolve("task-value"), + (err, res) => { + if (err) { + console.log("ERR", err && err.message); + } else { + console.log("DONE", res); + } + }, + ); + `; + + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", src], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout.trim()).toBe("DONE task-value"); + expect(exitCode).toBe(0); +}); From dbd3258b31c8e16054ad156693d09acf9566b405 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Fri, 24 Jul 2026 22:32:35 +0000 Subject: [PATCH 2/4] [autofix.ci] apply automated fixes --- test/js/node/domain/domain.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/js/node/domain/domain.test.ts b/test/js/node/domain/domain.test.ts index c8f6a8ff1071..f2823745f159 100644 --- a/test/js/node/domain/domain.test.ts +++ b/test/js/node/domain/domain.test.ts @@ -1,4 +1,4 @@ -import { describe, test, expect } from "bun:test"; +import { describe, expect, test } from "bun:test"; import { bunEnv, bunExe } from "harness"; import domain from "node:domain"; From 695afc38a9dc59a596962212db6e10ae61a8ae57 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 24 Jul 2026 22:57:16 +0000 Subject: [PATCH 3/4] node:domain: share one runInDomain() across bind/intercept/run Addresses review: - Hoist the enter/try/catch/finally/exit block into a single helper the three methods delegate to, matching Node's shape. - Drop the bind()-throw test that asserted the shim's swallow-and-emit behaviour; Node lets the throw propagate to the caller, so that test pinned a divergence rather than parity. - Add before/after process.domain checks to the intercept() active-domain test so the exit() call site is covered. --- src/js/node/domain.ts | 43 +++++++++++++----------------- test/js/node/domain/domain.test.ts | 16 ++--------- 2 files changed, 20 insertions(+), 39 deletions(-) diff --git a/src/js/node/domain.ts b/src/js/node/domain.ts index bd8f3aad0a9e..a2c1f8da1393 100644 --- a/src/js/node/domain.ts +++ b/src/js/node/domain.ts @@ -27,6 +27,21 @@ domain.createDomain = domain.create = function () { d.emit("error", e); } + // Node lets a throw from fn propagate and catches it later via + // process._fatalException's domain hook; Bun has no such hook yet, so the + // catch here stands in for it. The return value and `this` forwarding match + // Node exactly. + function runInDomain(thisArg, fn, args) { + d.enter(); + try { + return fn.$apply(thisArg, args); + } catch (err) { + emitError(err); + } finally { + d.exit(); + } + } + d.add = function (emitter) { emitter.on("error", emitError); }; @@ -35,14 +50,7 @@ domain.createDomain = domain.create = function () { }; d.bind = function (fn) { function runBound() { - d.enter(); - try { - return fn.$apply(this, arguments); - } catch (err) { - emitError(err); - } finally { - d.exit(); - } + return runInDomain(this, fn, arguments); } ObjectDefineProperty(runBound, "domain", { __proto__: null, @@ -69,26 +77,11 @@ domain.createDomain = domain.create = function () { d.emit("error", er); return; } - var args = Array.prototype.slice.$call(arguments, 1); - d.enter(); - try { - return fn.$apply(this, args); - } catch (err) { - emitError(err); - } finally { - d.exit(); - } + return runInDomain(this, fn, Array.prototype.slice.$call(arguments, 1)); }; }; d.run = function (fn, ...args) { - this.enter(); - try { - return fn.$apply(this, args); - } catch (err) { - emitError(err); - } finally { - this.exit(); - } + return runInDomain(this, fn, args); }; d.dispose = function () { this.removeAllListeners(); diff --git a/test/js/node/domain/domain.test.ts b/test/js/node/domain/domain.test.ts index f2823745f159..40aae9598419 100644 --- a/test/js/node/domain/domain.test.ts +++ b/test/js/node/domain/domain.test.ts @@ -35,20 +35,6 @@ describe("domain.bind()", () => { const bound = d.bind(() => {}); expect((bound as any).domain).toBe(d); }); - - test("routes a thrown error to the domain's 'error' listener", () => { - const d = domain.create(); - let caught; - d.on("error", (e: any) => { - caught = e; - }); - const bound = d.bind(() => { - throw new Error("boom"); - }); - expect(bound()).toBeUndefined(); - expect(caught).toBeInstanceOf(Error); - expect((caught as Error).message).toBe("boom"); - }); }); describe("domain.intercept()", () => { @@ -99,10 +85,12 @@ describe("domain.intercept()", () => { test("makes the domain active while the callback runs", () => { const d = domain.create(); let inside; + expect(process.domain == null).toBe(true); d.intercept(() => { inside = process.domain; })(null); expect(inside).toBe(d); + expect(process.domain == null).toBe(true); }); }); From c4e09f2688c44e8fc19f6d8b6d1136c705abc4c9 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 24 Jul 2026 23:16:07 +0000 Subject: [PATCH 4/4] ci: retrigger