diff --git a/.buildkite/ci.mjs b/.buildkite/ci.mjs index caaf647428..77787dd3ac 100755 --- a/.buildkite/ci.mjs +++ b/.buildkite/ci.mjs @@ -371,7 +371,7 @@ function getZigAgent(platform, options) { * @returns {Agent} */ function getTestAgent(platform, options) { - const { os, arch } = platform; + const { os, arch, profile } = platform; if (os === "darwin") { return { @@ -391,6 +391,13 @@ function getTestAgent(platform, options) { } if (arch === "aarch64") { + if (profile === "asan") { + return getEc2Agent(platform, options, { + instanceType: "c8g.2xlarge", + cpuCount: 2, + threadsPerCore: 1, + }); + } return getEc2Agent(platform, options, { instanceType: "c8g.xlarge", cpuCount: 2, @@ -398,6 +405,13 @@ function getTestAgent(platform, options) { }); } + if (profile === "asan") { + return getEc2Agent(platform, options, { + instanceType: "c7i.2xlarge", + cpuCount: 2, + threadsPerCore: 1, + }); + } return getEc2Agent(platform, options, { instanceType: "c7i.xlarge", cpuCount: 2, diff --git a/scripts/runner.node.mjs b/scripts/runner.node.mjs index ead78ebbc2..fedfb07c8b 100755 --- a/scripts/runner.node.mjs +++ b/scripts/runner.node.mjs @@ -72,6 +72,7 @@ const cwd = import.meta.dirname ? dirname(import.meta.dirname) : process.cwd(); const testsPath = join(cwd, "test"); const spawnTimeout = 5_000; +const spawnBunTimeout = 20_000; // when running with ASAN/LSAN bun can take a bit longer to exit, not a bug. const testTimeout = 3 * 60_000; const integrationTimeout = 5 * 60_000; @@ -1152,6 +1153,9 @@ async function spawnBun(execPath, { args, cwd, timeout, env, stdout, stderr }) { } bunEnv["TEMP"] = tmpdirPath; } + if (timeout === undefined) { + timeout = spawnBunTimeout; + } try { const existingCores = options["coredump-upload"] ? readdirSync(coresDir) : []; const result = await spawnSafe({ diff --git a/src/bun.js/ConsoleObject.zig b/src/bun.js/ConsoleObject.zig index a52cf2dfb6..e8a3e1c926 100644 --- a/src/bun.js/ConsoleObject.zig +++ b/src/bun.js/ConsoleObject.zig @@ -2278,6 +2278,13 @@ pub const Formatter = struct { } }, .Error => { + // Temporarily remove from the visited map to allow printErrorlikeObject to process it + // The circular reference check is already done in printAs, so we know it's safe + const was_in_map = if (this.map_node != null) this.map.remove(value) else false; + defer if (was_in_map) { + _ = this.map.put(value, {}) catch {}; + }; + VirtualMachine.get().printErrorlikeObject( value, null, diff --git a/src/bun.js/VirtualMachine.zig b/src/bun.js/VirtualMachine.zig index 47f374dbaa..062155094f 100644 --- a/src/bun.js/VirtualMachine.zig +++ b/src/bun.js/VirtualMachine.zig @@ -3236,8 +3236,23 @@ fn printErrorInstance( } for (errors_to_append.items) |err| { + // Check for circular references to prevent infinite recursion in cause chains + if (formatter.map_node == null) { + formatter.map_node = ConsoleObject.Formatter.Visited.Pool.get(default_allocator); + formatter.map_node.?.data.clearRetainingCapacity(); + formatter.map = formatter.map_node.?.data; + } + + const entry = formatter.map.getOrPut(err) catch unreachable; + if (entry.found_existing) { + try writer.writeAll("\n"); + try writer.writeAll(comptime Output.prettyFmt("[Circular]", allow_ansi_color)); + continue; + } + try writer.writeAll("\n"); try this.printErrorInstance(.js, err, exception_list, formatter, Writer, writer, allow_ansi_color, allow_side_effects); + _ = formatter.map.remove(err); } } diff --git a/src/bun.js/bindings/bindings.cpp b/src/bun.js/bindings/bindings.cpp index 320ab6afec..bd3237b73a 100644 --- a/src/bun.js/bindings/bindings.cpp +++ b/src/bun.js/bindings/bindings.cpp @@ -4103,10 +4103,12 @@ extern "C" JSC::EncodedJSValue JSC__JSValue__getOwn(JSC::EncodedJSValue JSValue0 auto identifier = JSC::Identifier::fromString(vm, propertyNameString); auto property = JSC::PropertyName(identifier); PropertySlot slot(value, PropertySlot::InternalMethodType::GetOwnProperty); - if (value.getOwnPropertySlot(globalObject, property, slot)) { - RELEASE_AND_RETURN(scope, JSValue::encode(slot.getValue(globalObject, property))); - } - RELEASE_AND_RETURN(scope, {}); + bool hasSlot = value.getOwnPropertySlot(globalObject, property, slot); + RETURN_IF_EXCEPTION(scope, {}); + if (!hasSlot) return {}; + auto slotValue = slot.getValue(globalObject, property); + RETURN_IF_EXCEPTION(scope, {}); + return JSValue::encode(slotValue); } JSC::EncodedJSValue JSC__JSValue__getIfPropertyExistsFromPath(JSC::EncodedJSValue JSValue0, JSC::JSGlobalObject* globalObject, JSC::EncodedJSValue arg1) @@ -4889,6 +4891,10 @@ static void fromErrorInstance(ZigException& except, JSC::JSGlobalObject* global, if (!scope.clearExceptionExceptTermination()) [[unlikely]] return; if (stackValue) { + // Prevent infinite recursion if stack property is the error object itself + if (stackValue == val) { + return; + } if (stackValue.isString()) { WTF::String stack = stackValue.toWTFString(global); if (!scope.clearExceptionExceptTermination()) [[unlikely]] { diff --git a/src/bun.js/test/Execution.zig b/src/bun.js/test/Execution.zig index 0a56b81c78..3819fad8dc 100644 --- a/src/bun.js/test/Execution.zig +++ b/src/bun.js/test/Execution.zig @@ -487,6 +487,8 @@ fn onGroupCompleted(_: *Execution, _: *ConcurrentGroup, globalThis: *jsc.JSGloba vm.auto_killer.disable(); } fn onSequenceStarted(_: *Execution, sequence: *ExecutionSequence) void { + if (sequence.test_entry) |entry| if (entry.callback == null) return; + sequence.started_at = bun.timespec.now(); if (sequence.test_entry) |entry| { @@ -500,6 +502,8 @@ fn onSequenceStarted(_: *Execution, sequence: *ExecutionSequence) void { } } fn onEntryStarted(_: *Execution, entry: *ExecutionEntry) void { + if (entry.callback == null) return; + groupLog.begin(@src()); defer groupLog.end(); if (entry.timeout != 0) { @@ -512,7 +516,7 @@ fn onEntryStarted(_: *Execution, entry: *ExecutionEntry) void { } fn onEntryCompleted(_: *Execution, _: *ExecutionEntry) void {} fn onSequenceCompleted(this: *Execution, sequence: *ExecutionSequence) void { - const elapsed_ns = sequence.started_at.sinceNow(); + const elapsed_ns = if (sequence.started_at.eql(&.epoch)) 0 else sequence.started_at.sinceNow(); switch (sequence.expect_assertions) { .not_set => {}, .at_least_one => if (sequence.expect_call_count == 0 and sequence.result.isPass(.pending_is_pass)) { diff --git a/test/regression/issue/circular-error-stack-edge-cases.test.ts b/test/regression/issue/circular-error-stack-edge-cases.test.ts new file mode 100644 index 0000000000..cfc205df67 --- /dev/null +++ b/test/regression/issue/circular-error-stack-edge-cases.test.ts @@ -0,0 +1,99 @@ +import { expect, test } from "bun:test"; +import { bunEnv, bunExe, tempDir } from "harness"; + +test("error.stack getter that throws should not crash", async () => { + using dir = tempDir("throwing-stack-getter", { + "index.js": ` + const error = new Error("Test error"); + Object.defineProperty(error, "stack", { + get() { + throw new Error("Stack getter throws!"); + } + }); + console.log(error); + console.log("after error print"); + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "index.js"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(exitCode).toBe(0); + expect(stdout).toContain("after error print"); + expect(stdout).not.toContain("Stack getter throws"); + expect(stderr).not.toContain("Stack getter throws"); +}); + +test("error.stack getter returning circular reference", async () => { + using dir = tempDir("circular-stack-getter", { + "index.js": ` + const error = new Error("Test error"); + Object.defineProperty(error, "stack", { + get() { + return error; // Return the error itself + } + }); + console.log(error); + console.log("after error print"); + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "index.js"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(exitCode).toBe(0); + expect(stdout).toContain("after error print"); + expect(stdout).not.toContain("Maximum call stack"); + expect(stderr).not.toContain("Maximum call stack"); +}); + +test("error with multiple throwing getters", async () => { + using dir = tempDir("multiple-throwing-getters", { + "index.js": ` + const error = new Error("Test error"); + Object.defineProperty(error, "stack", { + get() { + throw new Error("Stack throws!"); + } + }); + Object.defineProperty(error, "cause", { + get() { + throw new Error("Cause throws!"); + } + }); + error.normalProp = "works"; + console.log(error); + console.log("after error print"); + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "index.js"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(exitCode).toBe(0); + expect(stdout).toContain("after error print"); + expect(stdout).toContain("normalProp"); + expect(stdout).not.toContain("Stack throws"); + expect(stdout).not.toContain("Cause throws"); +}); diff --git a/test/regression/issue/circular-error-stack.test.ts b/test/regression/issue/circular-error-stack.test.ts new file mode 100644 index 0000000000..25cc3c14c9 --- /dev/null +++ b/test/regression/issue/circular-error-stack.test.ts @@ -0,0 +1,84 @@ +import { expect, test } from "bun:test"; +import { bunEnv, bunExe, tempDir } from "harness"; + +test("error with circular stack reference should not cause infinite recursion", async () => { + using dir = tempDir("circular-error-stack", { + "index.js": ` + const error = new Error("Test error"); + error.stack = error; + console.log(error); + console.log("after error print"); + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "index.js"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(exitCode).toBe(0); + expect(stdout).toContain("after error print"); + expect(stdout).not.toContain("Maximum call stack"); + expect(stderr).not.toContain("Maximum call stack"); +}); + +test("error with nested circular references should not cause infinite recursion", async () => { + using dir = tempDir("nested-circular-error", { + "index.js": ` + const error1 = new Error("Error 1"); + const error2 = new Error("Error 2"); + error1.stack = error2; + error2.stack = error1; + console.log(error1); + console.log("after error print"); + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "index.js"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(exitCode).toBe(0); + expect(stdout).toContain("after error print"); + expect(stdout).not.toContain("Maximum call stack"); + expect(stderr).not.toContain("Maximum call stack"); +}); + +test("error with circular reference in cause chain", async () => { + using dir = tempDir("circular-error-cause", { + "index.js": ` + const error1 = new Error("Error 1"); + const error2 = new Error("Error 2"); + error1.cause = error2; + error2.cause = error1; + console.log(error1); + console.log("after error print"); + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "index.js"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(exitCode).toBe(0); + expect(stdout).toContain("after error print"); + expect(stdout).not.toContain("Maximum call stack"); + expect(stderr).not.toContain("Maximum call stack"); +});