Repository navigation
fix(react-native): stop dropping native crash reports for fatals JS did not see - #5271
Conversation
…id not see React render errors reach ExceptionsManager without the ErrorUtils global handler, so JS never captures them. The native plugins dropped every fatal JS crash report on the assumption that JS already had it, so these crashes were lost. The JS fatal handler now marks the crash as handled through the plugin before it forwards to React Native. Native drops its fatal JS report only when that marker is set: a file read at the next setup on iOS, an in-process flag on Android. An older JS layer sends no marker flag, so native keeps the old unconditional drop. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: 4a1a2a69-9aa2-4278-a49f-7588cc68a55f
…etup Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: 4a1a2a69-9aa2-4278-a49f-7588cc68a55f
PostHog Review · Deep · reviewed
|
| Step | Status | Result |
|---|---|---|
| Prepare the diff | done | 8 files, 1 chunk |
| Pick perspectives | done | 3 perspectives |
| Run review passes | done | 6 issues (+1 blind-spot) |
| Merge overlapping findings | done | 7 → 6 |
| Validate findings | done | 2 kept, 4 dismissed |
| Publish | done | 2 posted (view review) |
![posthog[bot]](https://avatars.githubusercontent.com/in/1203049?s=60&v=4){kind=small}
… error Native drops a fatal JS report only when it carries the marked message, from a launch that used the marker protocol, so early, recovered or pre-upgrade fatals no longer cause losses or duplicates.
…k in present terms
Coherence4 spec problems · 0 failing chokepoints · 2 spec gaps · 0 guard failures 44 components, 7 bullets (5 invariants, 2 requirements), 4 problems Spec problems (spec --check)
Chokepoints: none declared. Spec gaps: 2 entrances with outside or unknown trust and no traced control, and 10 more held by the adoption baseline
Close one with |
ioannisj
left a comment
ioannisj
left a comment
There was a problem hiding this comment.
Nothing blocking, but a few comments
- A short or empty fatal key can drop a later, unrelated crash on both platforms once a marked fatal is recovered from, and that crash is usually the render error this PR is meant to report.
- Apps with
uncaughtExceptions: falsenow get every JS fatal reported natively. Worth confirming that's intended and calling it out in the changeset. - PR description needs a sync
| guard let previous = previousLaunch else { return true } | ||
| previousLaunch = [:] | ||
| guard let message = previous[jsFatalMessageKey] as? String else { return false } | ||
| return message.isEmpty || texts.contains { $0.contains(message) } |
There was a problem hiding this comment.
🤖 An AI agent wrote this on behalf of @ioannisj.
[nit] I think a short or empty key can drop an unrelated crash here. isJsHandledFatal drops the report when any of texts contains the marked key, and there's no minimum length, so an empty key (throw {}) or a key like '1' matches nearly anything.
It only bites when the marked fatal doesn't become the crash (a custom or expo-updates fatal handler recovers, or RN reloads the bundle). The marker stays armed, and the next unmarked crash, usually the render error this PR is meant to catch, gets dropped. Android has the same match in dropJsHandledFatalCrash.
On iOS it also crosses launches: JsFatalMarkers.init consumes the previous launch's state on the first touch of .shared, so a fatal marked during startup before setup() replaces launch N's marker, and N's pending report is lost (empty key) or duplicated.
Would it be worth matching against the start of the message instead (after Unhandled JS Exception: on iOS, the start of the JavascriptException value on Android), letting an empty key match only an empty message, and rotating the previous launch's state only from setup?
There was a problem hiding this comment.
Good catch, confirmed on devices: a recovered new Error() or '1' marker dropped a later, different crash on both platforms (expo-updates' ErrorRecovery really does recover from startup fatals). Fixed in 2c12c32: JS now marks the <name>: <message> text RN reports, and native matches it at the start of the report's message (after Unhandled JS Exception: on iOS) and requires a shorter marker to end where the message ends. Re-verified on iOS RN 0.86/0.88 and Android RN 0.79/0.86: the later crash is now kept, and new Error() / throw {} fatals still dedupe.
On the cross-launch part: it reproduces, but the result is a duplicate, not a lost report, and it needs two crashes in a row with the second before setup(). Rotating only from setup would bring back the early-startup case fixed in 2fa760c, so I left it.
…out it Also declares fatalExceptionMarker on the plugin config type and names the uncaughtExceptions-off case in the changeset.
An empty or short marker no longer drops an unrelated crash, and every fatal marked before the crash is kept.
Problem
uncaughtExceptionsandnativeCrashesboth on.onUncaughtErrorcallsExceptionsManager.handleExceptiondirectly. The JS SDK hooks onlyErrorUtils.setGlobalHandler, so it never sees these crashes.beforeSenddrops everyRCTFatalException/ JS-error marker, and Android addsJavascriptExceptiontoignoredExceptionTypes. That filter (from #3824) assumes JS always captured the crash first.Origin
bd80c7cChanges
markFatalExceptionHandled(message), and waits for it. React Native then turns the error into a native crash. Native drops its fatal JS report only when a marker matches it. Fatals JS did not capture (render errors on React 19) are reported natively.<name>: <message>text React Native reports for the error ([object Object]for a thrown object). Native drops a report only when its message starts with that text and, for a shorter marker, ends there too, so an empty or short marker can't match a different crash. Native keeps up to 10 markers, so a second fatal before the crash doesn't hide the first one's marker.setup()ormarkFatalExceptionHandledtouches it, so an early-startup fatal can't be mistaken for the previous launch. The first matching fatal JS report consumes it.JavascriptExceptionis captured in the dying process.JavascriptExceptioninbeforeSendconsumes them all. A leftover marker from a fatal the host recovered from can't hide a later, different crash.errorTracking.fatalExceptionMarker: truein the plugin setup config. Without it (an older JS layer), native keeps the old unconditional drop, so a plugin-only upgrade does not double report. A new JS layer with an older plugin skips the mark (the method is missing), and the old plugin drops as before. On iOS, a crash report from a launch that left no state file (old JS, or an OTA update that applied after the crash) is also dropped the old way.nativeCrasheson butuncaughtExceptionsoff now get native reports for JS fatals, because JS captured none of them. The changeset covers this.console: ['error']autocapture, a render crash also yields anonconsoleerror event next to the native fatal. Handler fatals already produce anonconsoleevent on main.Device verification
Release builds, events checked in a test project. Each cell is the number of
$exceptionevents per crash. "Native" means the plugin's report (RCTFatalException/JavascriptException), "JS" means the SDK's fatal path. Same-uuid pairs at ingestion are counted once.Marker edge cases, iOS RN 0.86 unless noted:
new Error()) or short ('1') message, then a different crash (iOS RN 0.86, Android RN 0.79 and 0.86): the later crash is reported natively.new Error()andthrow {}handler fatals (iOS RN 0.86, Android RN 0.79 and 0.86): 1 JS event, native dropped.Combined with #5293 (RN 0.79 and 0.76 old arch, Expo old arch): exactly 1 event per crash, and render crashes arrive as JS events with JS frames.
Tests:
posthog-react-nativevitest, plugin vitest, Kotlin unit tests (26), iOS and Android Release builds. Swift has no test target.Not covered by device runs:
Related
capture-exception,exception-event-metadata,exception-steps).Release info Sub-libraries affected
Libraries affected
Checklist
If releasing new changes
pnpm changesetto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
posthog-react-nativevitest (all pass, 1 snapshot update for the new setup flag), build/typecheck, and lint;@posthog/react-native-pluginvitest,tsc, and lint. Skipped: Android unit tests (no JDK), iOS build (no Xcode).$exceptionevents with anRCTFatalExceptiontype should go from zero to non-zero. AndroidJavascriptExceptionevents should do the same.Created with PostHog Desktop from this inbox report.
🤖 Generated with Claude Code