mirror of
https://github.com/oven-sh/bun
synced 2026-02-02 15:08:46 +00:00
Fix infinite recursion when error.stack is a circular reference (#22863)
## Summary This PR fixes infinite recursion and stack overflow crashes when error objects have circular references in their properties, particularly when `error.stack = error`. ### The Problem When an error object's stack property references itself or creates a circular reference chain, Bun would enter infinite recursion and crash. Common patterns that triggered this: ```javascript const error = new Error(); error.stack = error; // Crash! console.log(error); // Or circular cause chains: error1.cause = error2; error2.cause = error1; // Crash! ``` ### The Solution Added proper circular reference detection at three levels: 1. **C++ bindings layer** (`bindings.cpp`): Skip processing if `stack` property equals the error object itself 2. **VirtualMachine layer** (`VirtualMachine.zig`): Track visited errors when printing error instances and their causes 3. **ConsoleObject layer** (`ConsoleObject.zig`): Properly coordinate visited map between formatters Circular references are now safely detected and printed as `[Circular]` instead of causing crashes. ## Test plan Added comprehensive tests in `test/regression/issue/circular-error-stack.test.ts`: - ✅ `error.stack = error` circular reference - ✅ Nested circular references via error properties - ✅ Circular cause chains (`error1.cause = error2; error2.cause = error1`) All tests pass: ``` bun test circular-error-stack.test.ts ✓ error with circular stack reference should not cause infinite recursion ✓ error with nested circular references should not cause infinite recursion ✓ error with circular reference in cause chain ``` Manual testing: ```javascript // Before: Stack overflow crash // After: Prints error normally const error = new Error("Test"); error.stack = error; console.log(error); // error: Test ``` 🤖 Generated with [Claude Code](https://claude.ai/code) --------- Co-authored-by: Claude Bot <claude-bot@bun.sh> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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("<r><cyan>[Circular]<r>", 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);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -4888,6 +4888,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]] {
|
||||
|
||||
@@ -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");
|
||||
});
|
||||
84
test/regression/issue/circular-error-stack.test.ts
Normal file
84
test/regression/issue/circular-error-stack.test.ts
Normal file
@@ -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");
|
||||
});
|
||||
Reference in New Issue
Block a user