Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
4 changes: 2 additions & 2 deletions src/jsc/bindings/FormatStackTraceForJS.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -668,11 +668,11 @@ JSC_DEFINE_HOST_FUNCTION(errorConstructorFuncAppendStackTrace, (JSC::JSGlobalObj
return {};
}

if (!destination->stackTrace()) {
if (!destination->stackTrace() && globalObject->stackTraceLimit()) {
destination->captureStackTrace(vm, globalObject, 1);
}

if (source->stackTrace()) {
if (source->stackTrace() && destination->stackTrace()) {
destination->stackTrace()->appendVector(*source->stackTrace());
source->stackTrace()->clear();
}
Expand Down
24 changes: 24 additions & 0 deletions test/js/node/v8/capture-stack-trace.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1121,3 +1121,27 @@
});
expect(exitCode).toBe(0);
});

test("Error.appendStackTrace does not abort when stackTraceLimit is not a number", async () => {
const src = `
Error.stackTraceLimit = undefined;
const a = new Error();
const b = new Error();
Error.appendStackTrace(a, b);

const c = new Error();
delete Error.stackTraceLimit;
const d = new Error();
Error.appendStackTrace(c, d);

process.stdout.write("ok");
`;
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", src],
env: bunEnv,
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect({ stdout, stderr, signalCode: proc.signalCode }).toEqual({ stdout: "ok", stderr: "", signalCode: null });
expect(exitCode).toBe(0);

Check failure on line 1146 in test/js/node/v8/capture-stack-trace.test.js

View check run for this annotation

Claude / Claude Code Review

Test does not exercise the destination->stackTrace() null guard

The new test never exercises the second added guard (`&& destination->stackTrace()`). In both scenarios the *source* error is created while `stackTraceLimit` is already unset (scenario 2's `c` is created while it's still `undefined` from scenario 1), so `source->stackTrace()` is null and the second `if` short-circuits before the new operand is evaluated — deleting that guard would not break this test. Add a case where the source is created *before* unsetting the limit and the destination after,
Comment thread
claude[bot] marked this conversation as resolved.
});
Loading