Drop the frames instead of aborting when a stack trace passes the string length limit - #42314
Merged
Jarred-Sumner merged 2 commits intoSep 14, 2026
Merged
Conversation
…ing length limit
`formatStackTrace` and `formatStackTraceToJSValue` build the `.stack`
text with a default-constructed `WTF::StringBuilder`, which calls
`CRASH()` on the append that passes `String::MaxLength` (2^31 - 1
characters). The message and each frame's source URL come from JS, so
`new Error("q".repeat(2**31-10)).stack` aborted the process. The default
`Error.prepareStackTrace`, which also runs before a user-defined one,
aborted the same way.
Both builders now record the overflow. A trace that did not fit becomes
the `name: message` header without the frames, or the message alone when
the header does not fit either. It does not throw, because
`formatStackTrace` also runs from `ErrorInstance::finalizeUnconditionally`
inside a GC.
Contributor
|
Warning Review limit reached
On-demand reviews are free for the next 9 days. After that, they cost $0.25 per reviewed file. Or wait 21 minutes for your next included review. View limit detailsLimit details: You’ve used all 10 included reviews currently available. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Essentials Run ID: 📒 Files selected for processing (2)
Comment |
Collaborator
Author
|
Status: fix pushed. The diff is green in CI, see the last paragraph. Reproduced on 1.4.3 and on main. Each command aborts with With this branch each one returns the Test: Split out of #42202. Open question for a maintainer: this drops the frames and does not throw, because the same text is also produced inside a GC. Node throws CI (build 114249): the new test passes on every lane checked (Linux x64-asan, macOS aarch64, Windows x64). Two red tests on the x64-asan lane are not touched by this diff. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Collaborator
Author
|
Updated 5:08 AM PT - Sep 11th, 2026
❌ @robobun, your commit 20e1856 has 2 failures in
🧪 To try this PR locally: bunx bun-pr 42314That installs a local version of the PR into your bun-42314 --bun |
Jarred-Sumner
pushed a commit
that referenced
this pull request
Sep 14, 2026
…ng length limit (#42202) ### Problem - An error message that embeds a string from JS has a user-controlled length. Building one past the string length limit aborts the process, also inside `try`/`catch`: `panic(main thread): abort() called`, exit code 134. Example: `bun -e 'try { Buffer.from("x", "q".repeat(2**31-10)) } catch {}'`. - The cause: `WTF::makeString` and a default-constructed `WTF::StringBuilder` call `CRASH()` when the result passes `String::MaxLength`. Every message builder in `src/jsc/bindings/ErrorCode.cpp` is one of the two. ### Fix - `Bun::MessageBuilder` (`ErrorCode.h`) records the overflow. Every builder in `ErrorCode.cpp` uses it, and `determineSpecificType` and `JSValueToStringSafe` accept only that type. `createError` and `throwError` report a message that did not fit as `RangeError: Out of memory`. - Five sinks outside `ErrorCode.cpp` build with `tryMakeString`: the `performance.measure` mark, the `ReadableStream` source `type`, three `MessageEvent` constructor messages. - Correct because JSC reports `RangeError: Out of memory` for a string it cannot create. Node v26.3.0 reports the same inputs as `RangeError: Invalid string length`. A message under the limit is unchanged. - Verified: `test/js/bun/util/error-message-string-length-limit.test.ts`, 8 cases in one child, which aborts on main. Also the buffer, streams, webcrypto and MessageEvent suites. Self-reviewed: 7 concerns raised, 6 addressed, 1 is a question for a maintainer (Notes). ### Background - `String::MaxLength` is 2^31 - 1 code units. A JS string can be that long. - `StringBuilder` takes an `OverflowPolicy`. The default crashes. `RecordOverflow` sets a flag that `hasOverflowed()` reports, and later appends do nothing. - `determineSpecificType` renders the `Received ...` part of `ERR_INVALID_ARG_TYPE`. It appends `constructor.name` with no bound. - JSC treats its own messages this way in `ExceptionHelpers.cpp`. <details><summary>Notes</summary> **The type guards its own reads.** `StringBuilder::toString()` and `length()` assert that the builder did not overflow, so on `MessageBuilder` they are private. `tryToString()` returns a null string on overflow, and `finish(globalObject, scope)` throws. No `hasOverflowed()` check is written by hand in `ErrorCode.cpp`. **Question for a maintainer.** This PR reports an over-long message as `RangeError: Out of memory` with no `.code`, like JSC and like Node. #39481 (open) takes the other policy for the Rust-side error constructor: it keeps the error type and properties and uses a stand-in message. If that policy is preferred here too, the change is in two functions: `createError(ErrorCode, MessageBuilder&)` and `MessageBuilder::finish`. **Moved out of this PR.** The first revision also changed `process.execve` and `Error.stack`. Neither uses `MessageBuilder`, and each has its own question, so each has its own PR: - `process.execve`: #42308 - `Error.stack` and the default `Error.prepareStackTrace`: #42314 **Cases in the test.** All abort on main and on 1.4.2 with exit code 134. With this branch each one throws `RangeError: Out of memory`. ```js const long = "q".repeat(2 ** 31 - 10); class C {} Object.defineProperty(C, "name", { value: long }); Buffer.from("x", long); // UNKNOWN_ENCODING Buffer.from(new C()); // determineSpecificType Buffer.alloc(1).indexOf(new C()); // JSBuffer.cpp message new SocketAddress({ address: "1.2.3.4", port: new C() }); // Bun__ErrorCode__determineSpecificType, the entry Rust validators call performance.measure("m", long); // PerformanceUserTiming.cpp crypto.subtle.importKey(long, new Uint8Array(8), "AES-GCM", false, ["encrypt"]); // JSSubtleCrypto.cpp new ReadableStream({ type: long }); // JSReadableStream.cpp ReadableStream.from(Symbol(long)); // throwNotIterable ``` **Verified by hand, not in the test.** - The three `MessageEvent` messages: `new MessageEvent("m", { ports: long })`, `{ ports: [long] }`, `{ source: long }`. Each aborts on main and throws with this branch. They render the value through the inspector first, which takes about 20 s in a release build and about 9 minutes in a debug ASAN build for a 2 GiB string, so they cannot be a test. - The quoted `ERR_INVALID_ARG_VALUE` path (`Buffer.alloc(4).fill(long, "hex")`), `ERR_OUT_OF_RANGE` with a long name, and `NodeError.prototype.toString` with a long message throw with this branch. The quoted path appends one character at a time, about 8 minutes in a debug build. - The doors reported in the comments below (`Buffer.byteLength`, `fs.mkdirSync` mode, `process.kill` signal name, `child_process.spawn`) all go through `MessageBuilder`. - The whole fixture also passes under `BUN_JSC_validateExceptionChecks=1`. **Scope.** This does not cover every `makeString` call that can reach an error sink. A census at `4ff91937` counted 276 `makeString(` calls and 110 default-constructed `StringBuilder` locals under `src/jsc`. The `makeString` calls left in `ErrorCode.cpp` take an `ASCIILiteral` or an internal string, so the binary bounds their length. A different mechanism is also out of scope: `String::utf8()` aborts for a Latin-1 string of more than about 2^30 characters, at many call sites. **Cost of the test.** The length is what is under test, so the input is about 2 GiB. One child runs all 8 cases, so the string is allocated once. Peak RSS is 4.4 GB. The file takes 2 to 4 s in a debug ASAN build. It skips below 10 GiB of total memory, the gate `test/js/bun/transpiler/source-too-large.test.ts` uses. The one test keeps a 30 s ceiling, because the default 5 s is too close on a loaded machine. **Pre-existing failures** seen in the surrounding suites, which also fail with `src/` at `origin/main`: `error-gc-test.test.js` (timeouts in a debug build), `socket.test.ts > should not call drain before handshake`, and `process.test.js > process` (this container has no `USER` in the environment). </details>
Jarred-Sumner
deleted the
robobun/fe09dde8/stack-trace-string-length-limit
branch
September 14, 2026 22:24
usrbinkat
pushed a commit
to usrbinkat/bun
that referenced
this pull request
Sep 15, 2026
…ng length limit (oven-sh#42202) ### Problem - An error message that embeds a string from JS has a user-controlled length. Building one past the string length limit aborts the process, also inside `try`/`catch`: `panic(main thread): abort() called`, exit code 134. Example: `bun -e 'try { Buffer.from("x", "q".repeat(2**31-10)) } catch {}'`. - The cause: `WTF::makeString` and a default-constructed `WTF::StringBuilder` call `CRASH()` when the result passes `String::MaxLength`. Every message builder in `src/jsc/bindings/ErrorCode.cpp` is one of the two. ### Fix - `Bun::MessageBuilder` (`ErrorCode.h`) records the overflow. Every builder in `ErrorCode.cpp` uses it, and `determineSpecificType` and `JSValueToStringSafe` accept only that type. `createError` and `throwError` report a message that did not fit as `RangeError: Out of memory`. - Five sinks outside `ErrorCode.cpp` build with `tryMakeString`: the `performance.measure` mark, the `ReadableStream` source `type`, three `MessageEvent` constructor messages. - Correct because JSC reports `RangeError: Out of memory` for a string it cannot create. Node v26.3.0 reports the same inputs as `RangeError: Invalid string length`. A message under the limit is unchanged. - Verified: `test/js/bun/util/error-message-string-length-limit.test.ts`, 8 cases in one child, which aborts on main. Also the buffer, streams, webcrypto and MessageEvent suites. Self-reviewed: 7 concerns raised, 6 addressed, 1 is a question for a maintainer (Notes). ### Background - `String::MaxLength` is 2^31 - 1 code units. A JS string can be that long. - `StringBuilder` takes an `OverflowPolicy`. The default crashes. `RecordOverflow` sets a flag that `hasOverflowed()` reports, and later appends do nothing. - `determineSpecificType` renders the `Received ...` part of `ERR_INVALID_ARG_TYPE`. It appends `constructor.name` with no bound. - JSC treats its own messages this way in `ExceptionHelpers.cpp`. <details><summary>Notes</summary> **The type guards its own reads.** `StringBuilder::toString()` and `length()` assert that the builder did not overflow, so on `MessageBuilder` they are private. `tryToString()` returns a null string on overflow, and `finish(globalObject, scope)` throws. No `hasOverflowed()` check is written by hand in `ErrorCode.cpp`. **Question for a maintainer.** This PR reports an over-long message as `RangeError: Out of memory` with no `.code`, like JSC and like Node. oven-sh#39481 (open) takes the other policy for the Rust-side error constructor: it keeps the error type and properties and uses a stand-in message. If that policy is preferred here too, the change is in two functions: `createError(ErrorCode, MessageBuilder&)` and `MessageBuilder::finish`. **Moved out of this PR.** The first revision also changed `process.execve` and `Error.stack`. Neither uses `MessageBuilder`, and each has its own question, so each has its own PR: - `process.execve`: oven-sh#42308 - `Error.stack` and the default `Error.prepareStackTrace`: oven-sh#42314 **Cases in the test.** All abort on main and on 1.4.2 with exit code 134. With this branch each one throws `RangeError: Out of memory`. ```js const long = "q".repeat(2 ** 31 - 10); class C {} Object.defineProperty(C, "name", { value: long }); Buffer.from("x", long); // UNKNOWN_ENCODING Buffer.from(new C()); // determineSpecificType Buffer.alloc(1).indexOf(new C()); // JSBuffer.cpp message new SocketAddress({ address: "1.2.3.4", port: new C() }); // Bun__ErrorCode__determineSpecificType, the entry Rust validators call performance.measure("m", long); // PerformanceUserTiming.cpp crypto.subtle.importKey(long, new Uint8Array(8), "AES-GCM", false, ["encrypt"]); // JSSubtleCrypto.cpp new ReadableStream({ type: long }); // JSReadableStream.cpp ReadableStream.from(Symbol(long)); // throwNotIterable ``` **Verified by hand, not in the test.** - The three `MessageEvent` messages: `new MessageEvent("m", { ports: long })`, `{ ports: [long] }`, `{ source: long }`. Each aborts on main and throws with this branch. They render the value through the inspector first, which takes about 20 s in a release build and about 9 minutes in a debug ASAN build for a 2 GiB string, so they cannot be a test. - The quoted `ERR_INVALID_ARG_VALUE` path (`Buffer.alloc(4).fill(long, "hex")`), `ERR_OUT_OF_RANGE` with a long name, and `NodeError.prototype.toString` with a long message throw with this branch. The quoted path appends one character at a time, about 8 minutes in a debug build. - The doors reported in the comments below (`Buffer.byteLength`, `fs.mkdirSync` mode, `process.kill` signal name, `child_process.spawn`) all go through `MessageBuilder`. - The whole fixture also passes under `BUN_JSC_validateExceptionChecks=1`. **Scope.** This does not cover every `makeString` call that can reach an error sink. A census at `4ff91937` counted 276 `makeString(` calls and 110 default-constructed `StringBuilder` locals under `src/jsc`. The `makeString` calls left in `ErrorCode.cpp` take an `ASCIILiteral` or an internal string, so the binary bounds their length. A different mechanism is also out of scope: `String::utf8()` aborts for a Latin-1 string of more than about 2^30 characters, at many call sites. **Cost of the test.** The length is what is under test, so the input is about 2 GiB. One child runs all 8 cases, so the string is allocated once. Peak RSS is 4.4 GB. The file takes 2 to 4 s in a debug ASAN build. It skips below 10 GiB of total memory, the gate `test/js/bun/transpiler/source-too-large.test.ts` uses. The one test keeps a 30 s ceiling, because the default 5 s is too close on a loaded machine. **Pre-existing failures** seen in the surrounding suites, which also fail with `src/` at `origin/main`: `error-gc-test.test.js` (timeouts in a debug build), `socket.test.ts > should not call drain before handshake`, and `process.test.js > process` (this container has no `USER` in the environment). </details>
usrbinkat
pushed a commit
to usrbinkat/bun
that referenced
this pull request
Sep 15, 2026
…ing length limit (oven-sh#42314) ### Problem - Reading `.stack` of an error with a very long message aborts the process: `panic(main thread): abort() called`, exit code 134. Example: `bun -e 'new Error("q".repeat(2**31-10)).stack'`. The default `Error.prepareStackTrace`, which also runs before a user-defined one, aborts the same way. - Both formatters in `src/jsc/bindings/FormatStackTraceForJS.cpp` (`formatStackTrace` at :154, `formatStackTraceToJSValue` at :42) use a default-constructed `WTF::StringBuilder`. It calls `CRASH()` on the append that passes `String::MaxLength`. The message and each frame's source URL come from JS. ### Fix - Both builders record the overflow (`OverflowPolicy::RecordOverflow`). A trace that did not fit becomes the `name: message` header without the frames, or the message alone if the header does not fit either. - It degrades and does not throw, because `formatStackTrace` also runs from `ErrorInstance::finalizeUnconditionally`, inside a GC, where nothing can throw. `.stack` is then the same whether the GC or the getter computes it. - A trace under the limit is unchanged. A user `Error.prepareStackTrace` still runs and still gets the call sites. - Verified: `test/js/node/v8/capture-stack-trace.test.js`, one new test with both paths in one child. Each path aborts alone on 1.4.3. Also `stack.test.ts`, three stack-trace regression tests and `test-error-prepare-stack-trace.js`. ### Background - `String::MaxLength` is 2^31 - 1 code units. An error message can be that long. - `.stack` is computed lazily: `ErrorInstance::materializeErrorInfoIfNeeded` calls Bun's hook on the first read. If the GC finds dead frames first, `finalizeUnconditionally` calls the string variant of the same hook. - `RecordOverflow` makes the builder set a flag that `hasOverflowed()` reports. Later appends do nothing. <details><summary>Notes</summary> This is split out of oven-sh#42202, which is now only about error messages. Other policies for a trace that does not fit, and why this PR does not take them: - Throw, like Node. Node v26.3.0 throws `RangeError: Invalid string length` from the `.stack` read for the same input. In Bun the same text is also produced inside a GC, where a throw is not possible, so the two paths would differ. The getter runs under many operations on the error object (`Object.keys`, a property write), which would all start to throw. - Keep the frames and cut the message. That keeps more information, because the frames are small. It needs the header and the frames built apart, which is a larger change to a function that oven-sh#40354 and oven-sh#33584 also edit. The change is small on purpose: the builder policy and three return sites. It does not touch the parts of the file that oven-sh#40354 and oven-sh#33584 change, so any merge order works. Seen while testing, not caused by this change: `stack.test.ts > Async functions frame should be included in stack trace` fails when it runs in one process after the three regression files (two extra frames). It fails the same way with `src/` at `origin/main`, and it passes alone. Cost of the test: the length is what is under test, so the child needs a string of about 2 GiB. Both paths run in one child, so the string is allocated once. Each path copies the header once, so the child touches about 6 GB of pages. It takes about 5 s in a debug ASAN build, so it carries a 30 s ceiling. It skips below 10 GiB of total memory, the gate `test/js/bun/transpiler/source-too-large.test.ts` uses. </details>
Jarred-Sumner
pushed a commit
that referenced
this pull request
Sep 16, 2026
#42795) ### Problem - `node:vm` aborts when a string it builds from JS text passes `String::MaxLength`: `panic(main thread): abort() called`, exit code 134, also inside try/catch. Example: `const s = "a".repeat(2 ** 30); vm.compileFunction("", [s, s])`. - Five joins in `src/jsc/bindings/` call `CRASH()` past the limit, through a default `StringBuilder` or `makeString`: the `compileFunction` params (`NodeVM.cpp:424`), the arrow header (`:533`, `:535`), the `import()` message (`:1666`), `setExport` (`NodeVMSyntheticModule.cpp:216`), the link failure message (`NodeVMSourceTextModule.cpp:459`). - The arrow header is the next abort after #42314: `vm.runInNewContext("throw new Error(m)", { m: "q".repeat(2 ** 31 - 40) })`. ### Fix - A program text or a message that does not fit is `RangeError: Out of memory`, like `new Function`. - An error whose arrow header does not fit keeps its stack. The header is decoration, so the caller still gets the script's own error. - Verified: `test/js/node/vm/vm.test.ts`, one new test, six cases. Each aborts alone on main. The link failure case takes 79 s in a debug build, so it has a hand check only. Also `test/js/node/vm/` and Node's `test-vm-*` files. - Self-reviewed: 14 concerns raised, 11 addressed. The other 3 are separate work (Notes). ### Background - `String::MaxLength` is 2^31 - 1 code units. `RecordOverflow`, `tryMakeString` and `Bun::MessageBuilder` (#42202) report an overflow to the caller. - The arrow header is what Node puts in front of `.stack` for an error from a vm script: `<filename>:<line>`, the source line, a caret line. - Found by reading the bindings. No issue reports it. <details><summary>Notes</summary> This is the same family as #42202 (ERR_* messages) and #42314 (stack frames), in files they did not touch. Line numbers above are at main 7e56b40. **Each case alone**, debug ASAN build of main at 90fba66, exit code 134 every time. `long` is `"q".repeat(2 ** 31 - 10)`: - `vm.compileFunction("", [half, half])`, two params of 2**30 characters. - `new vm.SyntheticModule([], () => {}).setExport(long, 1)`. - `vm.runInNewContext("Function")("s", "return import(s)")(long)`. With a short specifier this rejects with `Could not import the module 'x'.`, which is the message at `NodeVM.cpp:1666`. It now rejects with the `RangeError`. It does not throw. - `throw error` in a `vm.Script`, where `error.stack` is `long`. This is the first `makeString` in `writeArrowHeaderStack`, through `handleException`. - The same with a source line of over 1024 characters. `nthSourceLineForArrowHeader` leaves such a line out, so the second `makeString` builds the header. - `new vm.Script("%%")` while `Error.prepareStackTrace` returns `long`. This is `decorateParseErrorStack`. - Not in the test: the `runInNewContext` example above (4.6 s, 6.4 GB). With the fix it throws the script's `Error`, and `.stack` is `"Error: " + m`, which is what #42314 produces. **The link failure message** (`request for '<specifier>' is not in cache`) needs a specifier of 2**31 - 20 characters in the source of a `SourceTextModule`, so the lexer has to read 2 GiB. That is 5.6 s in a release build, and 79 s and 6.5 GB in a debug ASAN build. By hand: `b = new vm.SourceTextModule('import "' + specifier + '";')`, `a = new vm.SourceTextModule('import "b";')`, `a.linkRequests([b])`, `a.instantiate()`. 1.4.3 exits 134. This branch throws `RangeError: Out of memory`. The message under the limit has a test already (`vm.test.ts`, `request for 'b' is not in cache`). **The filename examples** abort in the same `makeString` (lldb: `WTF::makeString` <- `writeArrowHeaderStack` <- `decorateParseErrorStack` <- `constructScript`). With the fix, `new vm.Script("%%", { filename: "a".repeat(2 ** 30) })` throws the `SyntaxError` without the header. They are not in the test: a filename of 2**30 characters costs about 2 minutes in a debug build before the join is reached (1.85 s for 2**24 characters, for a `new vm.Script("1", { filename })` that compiles fine). The test reaches the same joins through the `stack` argument, which costs nothing. `vm.compileFunction("%%")` with the long `Error.prepareStackTrace` result does not overflow: the default filename is empty, so the header is 9 characters and the result is exactly `String::MaxLength`. The compile-time case uses `new vm.Script`, whose default filename is `evalmachine.<anonymous>`. **Why the error keeps its stack** and no `RangeError` replaces it: the header is decoration on top of an error that the script or the parser already produced. A caller that catches around `runInContext` wants that error. #42314 made the same choice for frames that do not fit. In Node the same step is `err.stack = arrow + err.stack` in `decorateErrorStack`, where a result past V8's limit throws `RangeError: Invalid string length`. That is a side effect of the concatenation. The error is not marked as decorated when the header is left out. **Left as it is:** - `StringBuilder caretLine` in `writeArrowHeaderStack`. `nthSourceLineForArrowHeader` returns at most 1024 characters and the caret column is at most one past the line, so the builder holds at most 1025 characters. - `makeString("Unexpected token '", tokenView, '\'')` at `NodeVM.cpp:185`. `stringifyAnonymousFunction` runs first and checks the wrapped program, which is at least 18 characters longer than the source. This message is 19 characters longer than one token of that source. So it fits when the program fits, except for a source that is one token of exactly 2**31 - 19 characters. I found no input that gets there: for every token kind, JSC's parser first builds its own, longer message from the same token (`printUnexpectedTokenText`). - The other `makeString` calls in `NodeVM.cpp` join literals only. - A real allocation failure, not the length limit, inside `paramString.toString()` (it shrinks the buffer first) still asserts. It called `CRASH()` before. `MessageBuilder::tryToString` from #42202 has the same property. **Not in this PR, because the abort is inside WebKit:** - A `filename` or a module `identifier` whose percent-encoded form does not fit: `RELEASE_ASSERT` in `percentEncodeCharacters` (`URL.cpp:674`) <- `WTF::URL::fileURLWithFileSystemPath`. Example: `new vm.Script("1", { filename: " ".repeat(2 ** 30) })`. oven-sh/WebKit#643 is open for the URL code. - A module `identifier` near the limit, when another module imports a name that it does not export: `makeString` for `Export named 'x' not found in module '<identifier>'` in `CyclicModuleRecord::initializeEnvironment`. - A parse error at a token near the limit: the parser's own message, see `NodeVM.cpp:185` above. #38239, #38240 and #38245 edit the `tryMakeString` statement after the params loop. This PR does not touch that statement, so any merge order works. **Self-review.** It asked for one PR for every join in the `node:vm` bindings. This is now that PR: the first version had only the params and the arrow header. It also asked to say that no issue reports this, and to name the aborts inside WebKit. Not done here, because each is its own PR: - A test-only way to lower the limit, so that tests like this one can run with small strings on every lane. It needs a wrapper for `tryMakeString` and the builders across the bindings. - A lint that counts `makeString(` and default `StringBuilder` uses in `src/jsc/bindings` (2 concerns). **Cost of the test:** the child holds one string of 2**31 - 10 characters. The two params are `slice()`s of it. Peak RSS is 3.5 GB and it takes 2.5 s in a debug ASAN build, so it carries a 30 s ceiling. It skips below 10 GiB of memory. The gate is the one in `test/js/web/fetch/blob-oom.test.ts`: it also reads `process.constrainedMemory()`, because inside a container `totalmem()` reports the host's RAM. The string comes from a one-character `repeat`: 1.15 s and 2.4 GB in a debug ASAN build, against 2.55 s and 4.4 GB for `Buffer.alloc(n, "q").toString()`. **Other suites**, debug ASAN build with the fix: `test/js/node/vm/vm.test.ts` (300 pass), the other files in `test/js/node/vm/` (`script-leak.test.ts` needs more than the default 5 s in a debug build, with or without this change), 95 `test-vm-*.js` and 2 `test-vm-*.mjs` from `test/js/node/test/parallel`, 3 from `sequential`, `test-repl-syntax-error-*.js`, `test-error-prepare-stack-trace.js`. The six cases also pass with `BUN_JSC_validateExceptionChecks=1`. </details> <!-- robobun:evidence:begin --> --- **no test proof** · iteration 1 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/node/vm/vm.test.ts <!-- robobun:evidence:end -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
.stackof an error with a very long message aborts the process:panic(main thread): abort() called, exit code 134. Example:bun -e 'new Error("q".repeat(2**31-10)).stack'. The defaultError.prepareStackTrace, which also runs before a user-defined one, aborts the same way.src/jsc/bindings/FormatStackTraceForJS.cpp(formatStackTraceat :154,formatStackTraceToJSValueat :42) use a default-constructedWTF::StringBuilder. It callsCRASH()on the append that passesString::MaxLength. The message and each frame's source URL come from JS.Fix
OverflowPolicy::RecordOverflow). A trace that did not fit becomes thename: messageheader without the frames, or the message alone if the header does not fit either.formatStackTracealso runs fromErrorInstance::finalizeUnconditionally, inside a GC, where nothing can throw..stackis then the same whether the GC or the getter computes it.Error.prepareStackTracestill runs and still gets the call sites.test/js/node/v8/capture-stack-trace.test.js, one new test with both paths in one child. Each path aborts alone on 1.4.3. Alsostack.test.ts, three stack-trace regression tests andtest-error-prepare-stack-trace.js.Background
String::MaxLengthis 2^31 - 1 code units. An error message can be that long..stackis computed lazily:ErrorInstance::materializeErrorInfoIfNeededcalls Bun's hook on the first read. If the GC finds dead frames first,finalizeUnconditionallycalls the string variant of the same hook.RecordOverflowmakes the builder set a flag thathasOverflowed()reports. Later appends do nothing.Notes
This is split out of #42202, which is now only about error messages.
Other policies for a trace that does not fit, and why this PR does not take them:
RangeError: Invalid string lengthfrom the.stackread for the same input. In Bun the same text is also produced inside a GC, where a throw is not possible, so the two paths would differ. The getter runs under many operations on the error object (Object.keys, a property write), which would all start to throw.The change is small on purpose: the builder policy and three return sites. It does not touch the parts of the file that #40354 and #33584 change, so any merge order works.
Seen while testing, not caused by this change:
stack.test.ts > Async functions frame should be included in stack tracefails when it runs in one process after the three regression files (two extra frames). It fails the same way withsrc/atorigin/main, and it passes alone.Cost of the test: the length is what is under test, so the child needs a string of about 2 GiB. Both paths run in one child, so the string is allocated once. Each path copies the header once, so the child touches about 6 GB of pages. It takes about 5 s in a debug ASAN build, so it carries a 30 s ceiling. It skips below 10 GiB of total memory, the gate
test/js/bun/transpiler/source-too-large.test.tsuses.