You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
feat(exceptions): JS exception payload, handlers, and ErrorBoundary - #16
Builds a privacy-safe JS exception payload (signature, bounded stacks, debug IDs) and the logException / logUnhandledException facade. Native methods on this branch are still stubs.
Installs ErrorUtils and unhandled-rejection handlers, and adds <ErrorBoundary> plus a root reporter inside Bugsee.wrap that reports a render error and then rethrows it.
Not in this pull request: the Android and iOS bridges (Tasks 7.1c and 7.1d) and the device exception suites (Task 7.5).
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Deep code review: JS exceptions (payload, handlers, ErrorBoundary)
This is the Phase 7 JS slice: privacy-safe payload + SHA-1 signature, ErrorUtils/rejection handlers, logException / logUnhandledException, debug IDs, <ErrorBoundary>, and RootErrorReporter inside Bugsee.wrap. Native bridges remain stubs (Tasks 7.1c/7.1d), so nothing here actually files a report yet — but the JS contracts those bridges will consume are what this review is about.
I did not re-run yarn test / mutate. Findings below are from reading the new modules against launch/attach/wrap, reportHandled's swallow-all catch, R7 (detect.crash), R8/R10, and RN 0.81/0.87 ErrorHandlers.js / ExceptionsManager.handleException (confirmed: render errors do not go through ErrorUtils, so the root reporter is the right hook).
Findings
Sev
Issue
P1
attach() installs handlers with capture default-on before reading native detect.crash — R7 hole on the Android auto-launch path
P1
<ErrorBoundary> claims the error in markReported then reportHandled swallows invalid options — report is dropped forever
P2
installExceptionHandlers sets installed = true before install finishes — a throw permanently skips handlers
P2
R10 boundary node is appended pastEXCEPTION_MAX_CAUSE_DEPTH (documented as Android's nesting cap)
1. Overall risk: Medium
The payload/stack work is unusually well-hardened (ReDoS budgets, header-strip guards, never-throw builder, non-Error privacy). The issues that matter are lifecycle/R7 ordering and ErrorBoundary's interaction with the shared WeakSet — not parser correctness.
2. Merge recommendation: Request changes
Fix the two P1s before merging. They are small edits and they become live the moment 7.1c/7.1d land. P2s should land in the same pass if cheap; they can otherwise be tracked against 7.1c.
d0b1b8b lands the three code fixes from the previous pass. I re-read attach(), ErrorBoundary.componentDidCatch, installExceptionHandlers, and the new tests against the original failure scenarios.
Prior findings
Thread
Sev
Verdict
attach() installed handlers with capture default-on across getLaunchOptions()
P1
fully_addressed — fail-closes capture, applies detect.crash, then installs; reject path does not install
<ErrorBoundary> claimed via markReported then reportHandled swallowed bad options
P1
fully_addressed — validates first; rejected options warn and the error is still reported with options omitted
installed = true before ErrorUtils / tracker ran
P2
fully_addressed — split flags, each set only after that half returns; retries are tested
R10 boundary node appended past EXCEPTION_MAX_CAUSE_DEPTH
P2
partially_addressed — left as spec'd (cause chain cap, then the R10 node). Credible for a Java Throwable nest; not for 7.1c as planned (ReactNativeWebException(payloadJson) is one throwable, JSON cause lives in reason). Residual is on 7.1c: do not explode that JSON into nested Throwables
New findings
No new P0–P3 on this head. The fail-closed window in attach() (capture off until options return) is the intended trade-off: a default-on detect.crash can miss an error during that one round-trip; a false option can no longer leak one.
1. Overall risk: Low
JS payload, handlers, and boundaries are in good shape. Native logException / logUnhandledException are still stubs, so this slice does not yet file reports.
2. Merge recommendation: safe for this JS slice
Land it; 7.1c/7.1d remain the next real risk (keep-rule, iOS simulator completion, Android double-report R13).
3. Most important leftovers
7.1c must keep the payload as oneThrowable message (JSON), not a Java cause chain, or the declined depth finding becomes real.
Device Task 7.5 still has to prove ErrorUtils / rejection / boundary / root-reporter routes on hardware.
4. Positive observations
The three fixes match the suggested shapes, with tests for order, getLaunchOptions rejection, bad domain: '', and partial install retry.
Payload/stack hardening from the earlier commits (ReDoS budgets, header-strip, never-throw builder, R9) is unchanged and still looks solid.
I did not run yarn test here (no node_modules in this environment).
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Deep code review (re-review at d0b1b8b)
This is the Phase 7 JS slice: privacy-safe payload + SHA-1 signature, ErrorUtils/rejection handlers, logException / logUnhandledException, debug IDs, <ErrorBoundary>, and RootErrorReporter inside Bugsee.wrap. Native bridges remain stubs (Tasks 7.1c / 7.1d).
Re-checked the four findings from the previous run against d0b1b8b, then re-read the payload/stack/handler/report/boundary path against launch / relaunch / attach, wrap, R7–R11 / R14, and the plan’s “natives forward JSON verbatim” rule. I did not re-run yarn test / mutate locally.
Prior findings
Sev
Issue
Status
P1
attach() installed handlers with capture default-on before getLaunchOptions()
Fixed — capture is fail-closed, options applied, then handlers installed; reject path does not install
P1
<ErrorBoundary> claimed via markReported then reportHandled swallowed bad options
Fixed — validates first; rejected options warn and still report with options omitted
P2
installed = true before install finished
Fixed — split flags, each set only after that half returns
P2
R10 boundary node past EXCEPTION_MAX_CAUSE_DEPTH
Not a JS-builder defect — see residual below
The cause-depth note is withdrawn as a blocker for this PR. EXCEPTION_MAX_CAUSE_DEPTH names Android’s Throwable nesting cap, but 7.x sends one ReactNativeWebException whose reason is the JSON blob (plan: natives never parse/truncate/re-encode). Nested JSON cause objects are not nested Throwables, so Android will not drop the R10 ErrorBoundary Error node on a deep error.cause chain. Residual for 7.1c: keep that contract — put the JSON in getMessage() / equivalent; do not rehydrate it into a Java/ObjC cause chain.
Findings
No new P0–P3. I looked for remaining R7 gaps (launch still installs then applies, but that is the same synchronous tick with no await), RootErrorReporter getting stuck in reporting (the only plausible throw was WeakSet on a revoked Proxy; it does not throw on current JS), unbounded stripKnownHeader work (caught by buildExceptionPayload’s outer try), and ErrorBoundary vs detect.crash (R7 explicitly gates handlers / tracker / root reporter only; explicit logException and <ErrorBoundary> always forward).
1. Overall risk: Low
The two P1 lifecycle holes that would have become live when 7.1c/7.1d land are closed and tested. What remains is a 7.1c implementation constraint, not a bug in this JS.
2. Merge recommendation: Safe to merge this JS slice
Native exception bridges and device suites are still out of scope. Do not treat stubbed logException / logUnhandledException as “reports file.”
3. Most important leftover
Task 7.1c: one Throwable, JSON in reason. Do not nest deserialized cause nodes as platform exceptions (or the withdrawn P2 becomes real).
4. Positive observations
attach() R7 order and the ErrorBoundary validate-before-claim path match the facade and are locked by tests (lifecycle.test.ts, ErrorBoundary.test.tsx, handlers.test.ts).
Payload builder still does not serialise a thrown non-Error; property reads go through throwing-getter guards (R9).
Stack parse work is bounded across the whole cause chain, not only per node.
RootErrorReporter still rethrows after a bounded wait so RN’s onUncaughtError → handleException(error, true) path runs (R8). check-rn-compat.sh guards that premise.
CI on this head (at review): android, iOS e2e simulator, CocoaPods, SPM, and rn-compat 0.81/0.83/0.86/0.87 passed. lint, typecheck, unit, mutation was still pending.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Deep code review (head d0b1b8b)
This is the Phase 7 JS slice: privacy-safe payload + SHA-1 signature, logException / logUnhandledException, ErrorUtils + rejection handlers, <ErrorBoundary>, and RootErrorReporter inside Bugsee.wrap. Native 7.1c/7.1d are still stubs; device suites (7.5) are not in this PR.
I re-read the exception modules against the Phase 7 plan (R6–R14), the facade/attach/launch paths, wrap/isWrapper, and RN 0.81.4 / 0.87.1 ErrorHandlers.js (both call ExceptionsManager.handleException directly — not ErrorUtils — for onCaughtError/onUncaughtError). I did not run yarn test locally.
Prior findings at this head
Severity
Topic
Status
P1
attach() vs R7: capture stayed default-on across getLaunchOptions()
Fixed. Fail-closes capture, applies detect.crash, then installs. Reject path does not install.
P1
ErrorBoundary claimed via markReported before options validation
Fixed. Encodes first; on throw, warns and reports with options=undefined.
P2
Single installed flag set before both halves returned
Not a JS-builder defect. 7.1c must keep one Throwable with JSON-in-reason; do not re-parse that JSON into nested Throwables.
New findings
No new P0–P3. Scenarios checked and not reported: onCaughtError stealing the WeakSet claim (it does not go through ErrorUtils), RootErrorReporter blanking the tree before native capture (specified R8 sequence), launch()/relaunch() install-then-apply (sync, plan-specified), ErrorBoundary ignoring detect.crash (explicit API; R7 only names handlers, tracker, and the root reporter).
CI (d0b1b8b, run 36740431551)
Passing at review time: android, ios e2e simulator, cocoapods, spm, rn-compat 0.81/0.83/0.86/0.87. Still in progress: lint, typecheck, unit, mutation and ios unit (BugseeRNSupport).
Overall risk: Low
Merge recommendation: Safe for this JS slice. Reports still do not file until 7.1c/7.1d.
Most important to fix: Nothing on this JS head. Carry into 7.1c: one Throwable, JSON payload as reason, no nested Java causes.
Positives: Hostile-stack parse budget (64 KiB / 2 KiB line / 128 KiB chain); R9 never serializes extra properties; shared markReported WeakSet across facade, handlers, boundary, and root reporter; bounded 1.5s wait on the fatal path; RootErrorReporter is excluded from the view-tree walk.
…ture
Task 7.1a: pure-JS stack parsing, SHA-1 and the exception payload builder
that Phase 7's facade, native bridges and debug-id work build on.
- src/exceptions/stack.ts: parseStack (Chrome/V8, Hermes -- a Metro URL or
a release "address at" frame --, JSC, and a React componentStack line),
fileKey and cleanSource ("The payload (exact)", steps 1-2 and 1-5).
Adapted from 6.x's stack-trace-parse.ts (stacktrace-parser, MIT); the
notice is carried over verbatim.
- src/exceptions/sha1.ts: a minimal, dependency-free SHA-1 over a string's
UTF-8 bytes (React Native has no crypto module).
- src/exceptions/payload.ts: buildExceptionPayload and describeThrown
(R9/R10). Never throws: every read of a thrown value's name, message,
stack or cause goes through a helper that treats a throwing getter (a
hostile Proxy, a revoked Proxy) as absent. A non-Error is described, not
serialised, so nothing else it carries can leak into the payload. The
cause chain is capped at depth 10 and guarded against cycles with a
WeakSet; the signature is sha1 of the chain's names and frame traces
only, so it does not change with the reason.
yarn test, typecheck and lint are green. yarn mutate:src is green at
95.87% overall (threshold 95%); the new exceptions/ files score 96.27%
(stack.ts 98.60%, payload.ts 96.02%, sha1.ts 94.29%). The ~19 remaining
survivors there are equivalent mutants given the code's own safety nets
(e.g. a try/catch's catch body falling through to the same implicit
`undefined` return; a WeakSet.has() on a non-object already returning
false without the guard; an unread extra loop iteration; a >>>0 already
masking a corrupted byte to the same bits).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
…stack (C1)
Review finding C1: three regexes in stack.ts backtracked super-linearly on
an adversarial error.stack (which is arbitrary, attacker-reachable data --
a server body, user input, a pretty-printed dump), and nothing bounded the
work. One crafted message line could hang the JS thread on the fatal path
for minutes to potentially longer.
- STACK_MAX_INPUT_LENGTH (64 KiB) bounds the whole stack before it is split
into lines; STACK_MAX_LINE_LENGTH (2 KiB) bounds each line before any
pattern runs on it. These are the guaranteed defence: even a quadratic
scan on a 2 KiB line finishes in low single-digit milliseconds.
- parseStack takes a maxFrames bound and stops as soon as it is reached,
so a stack with millions of matching lines does not keep parsing past
the caller's own cap.
- Every line is trim()-med once, centrally, removing the need for each
pattern's own leading `^\s*`/trailing `\s*$` -- the specific shape (one
flexible quantifier retried at every position a different one
backtracks to) that made the reported cases super-linear.
- NODE_RE (6.x's `nodeRe` parity, review M3: a V8/Hermes anonymous frame,
or a relative bundle file CHROME_RE's scheme whitelist rejects) is split
into a parenthesised and a bare form; a first-draft combined form (with
an optional name group ending in a single literal space) still hung past
3 s on a 1 MB adversarial line, directly measured -- documented in the
pattern's own comment.
- COMPONENT_STACK_RE folds its name/parenthesis separator into the name
group itself (trimmed off in code), for the same reason.
- stack.test.ts adds timing tests (a generous 100 ms CI-safe budget,
measured with performance.now()) for the review's literal adversarial
shapes and for padding sized to survive both length caps intact, so it
reaches the target pattern rather than trim()-ing away to nothing first.
Manually verified: restoring the old COMPONENT_STACK_RE (this file's own
git history) makes the corresponding new test fail at ~1.0-1.5 s.
- Review M1: two of the report's "equivalent mutant" claims for `^` were
wrong (corrected in a follow-up commit); this adds the garbage-prefixed
regression cases the review demonstrated (`"xin A (at f.js:1)"`,
`"Within A (at f.js:1)"`, a JSC line with a `\r` where the file must
start) so they stay caught.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
… header
Review findings I1 and I2.
I1: reason was capped at EXCEPTION_MAX_REASON_LENGTH (8192), but name,
traceRaw, data.source and data.member were not. A hostile stack (many long
frame lines) produced a 76.8 MB JSON payload; a 1 MB error.name produced a
1 MB name. Both natives forward this verbatim as the crash reason/message
(payload.ts's own header comment), so the cap the constant exists to
enforce was bypassed for everything except reason itself.
- EXCEPTION_MAX_NAME_LENGTH (256) and EXCEPTION_MAX_FIELD_LENGTH (1024) cap
name and traceRaw/source/member respectively, with the same surrogate-safe
cut reason already used (now a shared `truncate` helper).
I2: the "Name: message" header was "dropped" only because the parsing
regexes usually failed to match it -- not stripped. A message shaped like
`connect ECONNREFUSED 10.0.0.1:5432` parsed as a fake top-level frame, the
signature varied with the message (breaking the property the "does not
change with the reason" test means to pin), and the worker's own
`_is_system_frame` would mark that fake frame the top user frame.
- stripKnownHeader removes a stack's own `name` or `name: message` prefix
before it reaches parseStack, using the same name/message already read
through safeGetString (no new property reads). message is matched as one
literal string, so a multi-line message -- including a later line that
looks like a frame -- is stripped in one step. JSC never prepends this
header, so the strip is a no-op there.
Also, review M6: `user` is now computed on the frame's own `file`, matching
the payload table's wording ("its file"), not the cleaned `source` (no
behaviour change: cleanSource's five steps never remove `node_modules`,
`native code` or `(native)`).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
…ymous-frame bug they found (M2)
Review M2: every stack.test.ts input was a hand-written single line. Adds
real, engine-captured multi-line stacks:
- V8: `node sample.js` (Node v24.15.0) -- a real stack with named, "node:"
scheme, `<anonymous>` and no-name frames.
- JSC: `jsc sample.js`, this machine's system JavaScriptCore (macOS
26.6.2's `/System/Library/Frameworks/JavaScriptCore.framework/.../jsc`)
-- named, anonymous (`@file:line:col`), a name containing a space
("global code") and a `forEach@[native code]` frame.
Both are the engines' own verbatim output (see the fixture file's header
for the exact script and commands), with only the capture directory's path
replaced for a stable fixture. Hermes stays hand-written: the only Hermes
binary found locally, `hermes-compiler`'s `hermesc`, refuses to run a
script ("hermesc does not support -exec"; it only emits bytecode), and no
separate `hermes` interpreter exists in this tree -- documented in the
fixture file rather than downloading one, per the task's instruction.
The real JSC fixture immediately found a bug: `@file:line:col` (JSC's own
anonymous-frame shape, an empty name before `@`) fell through JSC_RE's
name group entirely and left a stray leading `@` in the parsed file
(`"@sample.js"` instead of `"sample.js"`). JSC_RE's name group is now
optional as a whole, not just absent-or-one-or-more, so an empty name
before a real `@` is consumed correctly; verified this keeps the same
ReDoS bound (ranges from 0-3 ms on 1-2 MB adversarial inputs, matching the
version this replaces).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
… redundant
Mutation testing after C1/I1/I2 (fix round 1) surfaced that HERMES_ADDRESS_RE
had become fully redundant: NODE_PAREN_RE's file group is unrestricted, so
for "at name (address at file:line:col)" it already captures "address at
file" as one string, byte-identical to HERMES_ADDRESS_RE's own capture
(`fileKey`/`cleanSource` strip the "address at " prefix from either the
same way). Every one of HERMES_ADDRESS_RE's own regex-literal mutants
survived for exactly this reason -- NODE_PAREN_RE's fallback produced the
same ParsedFrame regardless. Removed the pattern and its parse function;
NODE_PAREN_RE now handles that shape too, and every existing "Hermes
address" test still passes unchanged (it was testing the input shape, not
which pattern handled it).
Also, from the same mutation run:
- `stripKnownHeader` no longer strips the header's trailing newline itself:
`parseStack` already trim()s and skips a blank line, so a leftover `\n`
or `\r\n` (a lone `\r` trims to empty) was already silently absorbed
there. The extra handling was untestable dead weight.
- Dropped a redundant `candidate.length > 0` guard: `name` is always
non-empty by the time it reaches here, so neither candidate can be ''.
- Added regression tests: NODE_PAREN_RE's optional column and NODE_BARE_RE's
anchor/multi-digit cases; STACK_MAX_INPUT_LENGTH/STACK_MAX_LINE_LENGTH
exact-boundary cases (a JSC-shaped line/stack sized to lose exactly its
final digit one byte past each cap); a stack whose own *name* is
frame-shaped, so an unstripped header would misparse as an extra frame
(a bare "Error" header can't show this, since it never parses as a frame
either way); and a CRLF stack.
yarn test/typecheck/lint green. yarn mutate:src green at 95.26% overall
(threshold 95%); exceptions/ at 93.53% (stack.ts 95.43%, sha1.ts 94.29%,
payload.ts 91.83% -- the remaining survivors are documented equivalents,
mostly the same classes as before: a `catch {}` matching JS's own
fall-through, `WeakSet.has()` on a non-object, and `buildFallbackPayload`'s
'ios' branch coinciding with its own default).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
…rity notes
Review M4, M5, M7 (docs-only; no code change).
- Constants table: STACK_MAX_INPUT_LENGTH, STACK_MAX_LINE_LENGTH,
EXCEPTION_MAX_NAME_LENGTH, EXCEPTION_MAX_FIELD_LENGTH (review C1/I1); a
note that a truncated reason is 8193 units, and that DEBUG_IDS_MAX is
Task 7.3's to enforce, not payload.ts's (review M7).
- The payload table: `frames` now says the header is stripped, not just
skipped when nothing happens to match it; `traceRaw`'s privacy note
(a legitimate absolute path still reaches the backend verbatim, by
spec -- review M5); `user`'s wording now matches the code, which checks
the frame's own file (review M6).
- R2: names 6.x's algorithm precisely (`concatExceptionsRecursiveForSignature`)
and the specific ways 7.x's signature differs from it (R3's `build_id`
drop, `cleanSource`'s source cleaning, `error.name` verbatim, no
`<unknown> () ()` for an unparsed line), and states plainly that client-
signature continuity across a 6.x-to-7.x upgrade is not expected --
6.x's own signature already changed with every app version and bundle.
- "To raise": whether `cleanSource` should strip through the last
`CodePush/` segment so a CodePush update does not change `source`/
`trace`/the signature (6.x did; 7.x currently does not) -- left as a
decision for the controller rather than changed silently in a fix round.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
…e Hermes-address parens (N1, N3, N2's JSC guard)
Re-review round 1, N1 (Critical). Two of the four patterns stack.ts rewrote
in fix round 1 were still quadratic, so the per-line length cap bounded
each line but not a whole 64 KiB stack or a full cause chain:
- CHROME_RE's name group `(.*?)` could contain `(`, so every `(`/` (`
followed by a whitelisted scheme was a fresh candidate split, each
costing its own O(line length) scan to fail. Excluding `(` from the name
group removes the ambiguity (a real name is never expected to contain
one; a frame's own `(` is always in the *file* half, untouched by this).
- COMPONENT_STACK_RE's file group allowed spaces, so `\(at\s+` and the file
group could both keep re-trying the same run of spaces (the same shape
fix round 1 already fixed for the name/parenthesis separator, missed
here). Requiring the file's first character to be non-space closes it.
Measured (this file's own comments have the numbers): the crafted 2 KiB
lines the review used dropped from ~4-5 ms to under 1 ms; a full 64 KiB
stack of either worst-case line is now under 1 ms (was 126 ms/1.3 s
respectively, per the review, once multiplied by a stack/a chain). Timing
tests cover a full-size stack of each pattern's own worst-case line.
N3: restores Hermes-address frames round 1's `HERMES_ADDRESS_RE` removal
(as redundant with `NODE_PAREN_RE`) had silently dropped -- a file
containing parentheses (an iOS app bundle such as "My App (Beta).app") and
an empty name -- via a dedicated `HERMES_ADDRESS_PAREN_RE`, safe because
the literal "address at " marker leaves no other flexible quantifier for
its file group to compete with once matched (measured linear). A name
containing `(` stays unsupported and is now documented as a known,
accepted limitation in the plan: allowing it back would reintroduce N1's
own class of bug.
N2 (part): a bare (no "@") JSC-shaped match whose file contains ": " is
now dropped rather than kept as a frame -- a second guard, in the parser
itself, for a "Name: message" header `stripKnownHeader` could not identify
(see the next commit) and therefore left unparsed.
Also replaces the invented `COMPONENT_STACK_SAMPLE` fixture (review M2/N4)
with the real algorithm in this machine's bundled React 19.2 renderer
(`node_modules/react-native/.../ReactFabric-dev.js`, `describeBuiltInComponentFrame`
and `getStackByFiberInDevAndProd`): a built-in host component's frame is
`"... (<anonymous>)"` on an engine whose stacks say "at" or `"...@unknown:0:0"`
on one that does not, and a user component's frame is a real
"at NAME (file:line:col)"/"NAME@file:line:col" frame like any other.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
… header strip consuming real frame text (N1's shared budget, N2, N4)
Re-review round 1.
N1 (shared budget): each node's own stack was already capped at
STACK_MAX_INPUT_LENGTH (64 KiB), but nothing capped the *sum* across an
11-node cause chain plus componentStack, which could still reach 12x one
node's own worst case (measured: 1.3 s at the caps before this). A single
EXCEPTION_MAX_TOTAL_STACK_LENGTH (128 KiB), shared via one mutable counter
threaded through the whole node tree, bounds the total instead: once it is
spent, a later node's frames are `[]`, same as having no stack at all.
Measured: the 11-node-chain-plus-componentStack case, every node at its
own cap, is 1-2 ms now (was 1.3 s); 7 ms under `node --regexp-interpret-all`
(a rough proxy for an interpreted engine such as Hermes').
N2: `stripKnownHeader` accepted a bare `name` prefix match with nothing
checking what followed, so a real frame whose own function name starts
with the error's name as plain text lost its own leading text -- captured
with system jsc, `new TypeError('boom')` thrown from a function named
`TypeErrorFactory` gave a stack whose only frame, `TypeErrorFactory@file:2:22`,
became `Factory@file:2:22`. A candidate is now accepted only when followed
by the end of the string, `\n` or `\r`. Also: `name` passed in is now the
*raw* value (`''` when absent), not the display fallback `'Error'` -- V8
and Hermes write no name at all, only `message`, when the raw name is `''`,
and the fallback masked that case entirely.
N4: React 19's `describeBuiltInComponentFrame` frame for a built-in host
component (`View`, `Text`, ...) is `file: "unknown"`, `line: 0`, `column: 0`
on JSC (see the previous commit's fixture) -- 0 is a real number, not
`null`, so without an explicit check this parsed as `user: true`. Excluded
by name via a dedicated, narrow check (only when file is exactly "unknown"
and both line and column are exactly 0), so a real file happening to be
named "unknown" at a real location is unaffected.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
…he N3/N4 exceptions
Review N1, N2, N3, N4 (docs-only; no code change beyond what the two
preceding commits already made).
- Constants table: EXCEPTION_MAX_TOTAL_STACK_LENGTH, and STACK_MAX_LINE_LENGTH's
row corrected -- fix round 1 claimed the cap alone made a quadratic
pattern's cost negligible, which a full 64 KiB stack and a cause chain
both disproved (N1).
- The payload table's `frames` row now describes the header-strip's
end/\n/\r boundary check and the bare-JSC-match ": " guard (N2).
- The `user` row now names React 19's built-in-component sentinel
(file "unknown", line 0, column 0 on JSC) as the reason `user` also
checks for it, not just line/column/node_modules (N4).
- A new "Known limitation" paragraph after the `source` steps: a Hermes
frame whose *name* contains `(` still does not parse -- allowing it
would reintroduce N1's own class of super-linear scan -- while a file
containing parentheses, the more realistic case, is now restored (N3).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Round 2's re-run of yarn mutate:src dropped to 94.87% overall (under the
95% break threshold) with the new stripKnownHeader guard, the shared parse
budget and isBuiltInComponentSentinel adding mutants faster than round 2's
own tests killed them. Adds targeted cases:
- stripKnownHeader: a non-matching candidate whose length coincidentally
lands on a newline is still not treated as a match (kills the
`startsWith` check itself being short-circuited); a bare `\r` (not
`\r\n`) header, matching the existing `\r\n` and `\n` cases.
- isBuiltInComponentSentinel: the three ways exactly two of
file==="unknown"/line===0/column===0 can hold without the third,
confirming the check needs all three, not any two (a mutant surviving
on only the existing 0/0-together and 1/2-together cases).
- The shared parse budget: a case where the remaining budget is smaller
than STACK_MAX_INPUT_LENGTH itself, so a node's own pre-slice to the
budget -- not parseStack's own per-call cap, which would still allow up
to 64 KiB -- is what has to do the cutting.
yarn mutate:src back to 95.25% overall; exceptions/ at 93.71% (stack.ts
94.66%, sha1.ts 94.29%, payload.ts 92.81%).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
…undary options before claiming
attach() held the default-on capture flag across the launch-options read, and ErrorBoundary claimed an error before a rejected options object could drop the report.
Co-authored-by: Cursor <cursoragent@cursor.com>
The reason will be displayed to describe this comment to others. Learn more.
Deep code review (head 5d4fd8a)
This is the Phase 7 JS slice: privacy-safe payload + SHA-1 signature, logException / logUnhandledException, ErrorUtils + rejection handlers, <ErrorBoundary>, and RootErrorReporter inside Bugsee.wrap. Native 7.1c/7.1d remain stubs; device suites (7.5) are out of scope.
5d4fd8a is a rebase of the previously reviewed fix (d0b1b8b). I re-read the exception modules against launch / relaunch / attach, wrap / isWrapper, R6–R14, and RN’s onUncaughtError → handleException(error, true) premise (pinned by check-rn-compat.sh). I did not run yarn test locally (node_modules is not present).
Prior findings
Severity
Topic
Status
P1
attach() installed handlers with capture default-on across getLaunchOptions()
Fixed. Fail-closes capture, applies detect.crash, then installs. Reject path does not install.
P1
<ErrorBoundary> claimed via markReported then reportHandled swallowed bad options
Fixed. Validates first; rejected options warn and still report with options omitted.
P2
Single installed flag set before both halves returned
Not a JS-builder defect. 7.1c must keep one Throwable with JSON-in-reason; do not rehydrate nested Throwables.
New findings
No new P0–P3. Scenarios checked and not reported:
launch() / relaunch() still install then apply: same synchronous tick, no await, so R7 cannot leak between those two calls.
onCaughtError / onUncaughtError still go to ExceptionsManager.handleException directly, not ErrorUtils, so the shared WeakSet is not stolen by RN’s soft render path.
RootErrorReporter always leaves reporting ( reportUnhandled never rejects and races UNHANDLED_REPORT_WAIT_MS).
ErrorBoundary ignoring detect.crash matches R7 (explicit API; only handlers, tracker, and the root reporter are gated).
Pre-launch logException / boundary reports being dropped is the documented native contract, not a JS miss.
CI (5d4fd8a, run 36745927733)
Passing at review time: rn-compat 0.81/0.83/0.86/0.87. Still pending: android, ios e2e / unit / cocoapods / spm, and lint, typecheck, unit, mutation.
Overall risk: Low
Merge recommendation: Safe for this JS slice. Reports still do not file until 7.1c/7.1d.
Most important leftovers: Nothing on this JS head. Carry into 7.1c: one Throwable, JSON payload as reason, no nested Java causes. Task 7.5 still has to prove the routes on hardware.
Positives: Hostile-stack parse budget (64 KiB / 2 KiB line / 128 KiB chain); R9 never serializes extra properties; shared markReported WeakSet across facade, handlers, boundary, and root reporter; bounded 1.5s wait on the fatal path; RootErrorReporter is excluded from the view-tree walk.
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
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.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Summary
logException/logUnhandledExceptionfacade. Native methods on this branch are still stubs.ErrorUtilsand unhandled-rejection handlers, and adds<ErrorBoundary>plus a root reporter insideBugsee.wrapthat reports a render error and then rethrows it.Test plan
514867b)eab747d)c5bdf56)485cf64)yarn teston the 7.4 checkout: 84 suites, 1598 passedyarn mutate:src: 95.16%Made with Cursor