Skip to content

Fix crash in Error.captureStackTrace when stackTraceLimit is nullopt#28719

Open
robobun wants to merge 1 commit intomainfrom
farm/509f03ca/fix-captureStackTrace-nullopt
Open

Fix crash in Error.captureStackTrace when stackTraceLimit is nullopt#28719
robobun wants to merge 1 commit intomainfrom
farm/509f03ca/fix-captureStackTrace-nullopt

Conversation

@robobun
Copy link
Copy Markdown
Collaborator

@robobun robobun commented Mar 31, 2026

Fixes a crash (SIGABRT) in Error.captureStackTrace when Error.stackTraceLimit has been deleted or set to a non-number value.

Root Cause

In FormatStackTraceForJS.cpp, line 685 calls .value() on the std::optional<unsigned> returned by globalObject->stackTraceLimit():

size_t stackTraceLimit = globalObject->stackTraceLimit().value();

When Error.stackTraceLimit is set to a non-number (e.g. a string, undefined) or deleted, ErrorConstructor::put / ErrorConstructor::deleteProperty in WebKit sets the internal optional to std::nullopt. Calling .value() on an empty optional throws std::bad_optional_access, which terminates the process with SIGABRT.

Fix

Use .value_or(DEFAULT_ERROR_STACK_TRACE_LIMIT) instead of .value(), matching the pattern used in WebKit's own ErrorConstructor.cpp and other call sites in Bun.

Repro

delete Error.stackTraceLimit;
Error.captureStackTrace({}); // SIGABRT

Copy link
Copy Markdown
Contributor

@claude claude bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Code review skipped — your organization's overage spend limit has been reached.

Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit at claude.ai/admin-settings/claude-code.

Once credits are available, push a new commit or reopen this pull request to trigger a review.

@robobun
Copy link
Copy Markdown
Collaborator Author

robobun commented Mar 31, 2026

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 31, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: de5f8c80-d66c-41e6-95bf-1b077b6e5b74

📥 Commits

Reviewing files that changed from the base of the PR and between 7d31b80 and 1c2c593.

📒 Files selected for processing (2)
  • src/bun.js/bindings/FormatStackTraceForJS.cpp
  • test/js/node/v8/capture-stack-trace.test.js

Walkthrough

Switches stack-trace limit retrieval to use a safe fallback (value_or(DEFAULT_ERROR_STACK_TRACE_LIMIT)) and keeps mapping explicit 0 to the default. Adds a test ensuring Error.captureStackTrace({}) does not throw when Error.stackTraceLimit is non-numeric, deleted, or undefined.

Changes

Cohort / File(s) Summary
Stack trace handling
src/bun.js/bindings/FormatStackTraceForJS.cpp
Use globalObject->stackTraceLimit().value_or(DEFAULT_ERROR_STACK_TRACE_LIMIT) instead of unconditionally calling .value(); retain normalization that maps explicit 0 to the default.
Tests
test/js/node/v8/capture-stack-trace.test.js
Add test that temporarily mutates Error.stackTraceLimit (non-numeric string, deleted, undefined) and verifies Error.captureStackTrace({}) does not throw, restoring the original value in finally.
🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main fix: preventing crashes in Error.captureStackTrace when stackTraceLimit is nullopt, which matches the primary change in the changeset.
Description check ✅ Passed The description covers what the PR does and includes verification through a provided repro case, but lacks explicit details on how the code was tested beyond the repro steps.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
⚔️ Resolve merge conflicts
  • Resolve merge conflict in branch farm/509f03ca/fix-captureStackTrace-nullopt

Comment @coderabbitai help to get the list of available commands and usage tips.

@robobun robobun force-pushed the farm/509f03ca/fix-captureStackTrace-nullopt branch from 3c11bc4 to 7d31b80 Compare March 31, 2026 17:51
Use value_or(DEFAULT_ERROR_STACK_TRACE_LIMIT) instead of value() on the
std::optional<unsigned> returned by stackTraceLimit(). When
Error.stackTraceLimit is set to a non-number or deleted, the optional
becomes nullopt and calling .value() triggers std::bad_optional_access
which aborts the process.
@robobun robobun force-pushed the farm/509f03ca/fix-captureStackTrace-nullopt branch from 7d31b80 to 1c2c593 Compare March 31, 2026 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant