Crash reporting for the bridge, and the two silent failures it uncovered - #79
Merged
Merged
Conversation
…s to The host process had no crash reporting at all. It now runs @sentry/bun behind three gates that must all hold: the user's telemetry consent, carried on the stdin bootstrap as `telemetryEnabled` and read once for the host's lifetime (absent means off, so a CLI or test host never reports); a SENTRY_DSN baked in by --define, exactly like LICENSE_API_URL; and a scrubber that strips paths, source lines, locals and the hostname, kept in lockstep with the app's. OnUncaughtException/OnUnhandledRejection are kept, not excluded: they are what stamps a fatal handled:false, which a hand-rolled captureException reports as generic/handled:true. They are re-added with options pinned rather than inherited, and the contract's other half lives in index.ts -- the SDK re-counts the OTHER uncaughtException listeners AT CRASH TIME, so it defers to our teardown only while one of ours is registered. Init also verifies getClient()?.getDsn() and logs at error when it is missing. Sentry.init never throws and never returns a status, so a DSN it refuses leaves a transport-less client on which every capture and even flush still succeed -- silently sending nothing. That is not hypothetical: the JS SDKs require a NUMERIC project id and errex issues slugs, so the DSN CI bakes in is refused outright. sentry-dart takes the last path segment as an opaque String, which is why only this side is affected.
sentry_flutter never assigns nativeDatabasePath, and sentry-native then falls back to `.sentry-native` relative to the CURRENT WORKING DIRECTORY. That is unwritable in exactly the configuration we ship: a Store-launched MSIX gets C:\Windows\System32 as its cwd, and its own install dir under WindowsApps is read-only. sentry_init fails, and native crash capture is absent with nothing to notice it by -- no crashpad handler, no database, and (auto-session-tracking being a native option) no release-health sessions either. Measured on the shipped 1.20698.1008 package: sentry.dll loaded, no .sentry-native anywhere on the machine, no crashpad_handler process. Dart-level reporting was unaffected throughout, which is why production still received Dart fatals while every native crash was lost. Also turns options.debug on outside release builds. The SDK reports its own init failures at debug level and nowhere else, which is the whole reason this went unnoticed: a broken native layer looks exactly like an app that never crashed. docs/release/build.md records that errex has no symbol-upload endpoint -- sentry-cli's chunk-upload and legacy dsyms paths both 404 while implemented routes answer 401 -- so desktop native frames arrive as module+offset and a sentry-cli upload step would only fail in CI.
Production fatal, 3 occurrences: ConcurrentModificationError: Concurrent modification during iteration: _Set len:4, culprit _CompactIterator.moveNext under FocusManager.applyFocusChangesIfNeeded. Every frame was in_app:false, which made it read as a framework bug; it is ours. applyFocusChangesIfNeeded notifies listeners with `for (final node in _dirtyNodes) node._notify()`, and FocusNode.dispose detaches, which makes FocusManager._markDetached do _dirtyNodes.remove(node) -- mutating the Set being iterated. The field's onFocusChange commits the rename, detached() runs that action through Future.sync, and _commitEdit calls _exitEdit BEFORE its first await, so the dispose landed inside the notification. _exitEdit now clears the fields first and disposes in a microtask, so the notification unwinds before the detach and nothing can reach a disposed node in between. Fixing it here rather than at the callback covers both commit triggers, Enter and blur. The regression test drives the real row: double-tap to rename, move focus away, assert no exception. Reverting the fix makes it throw the same ConcurrentModificationError.
bharathm03
added a commit
that referenced
this pull request
Sep 2, 2026
… path it enabled was unscrubbed (#80) * Bridge: refuse a DSN before the SDK installs anything, not after The gate ran after Sentry.init, which installs both top-level process handlers before it ever looks at the DSN — and nothing takes them off again, since Sentry.close() disables the client but leaves the listeners. A client that could never transmit therefore kept owning both fatal paths, with the warn-mode rejection handler printing raw unredacted reasons into the stderr teed to host.log. hasNumericProjectId now decides ahead of init, so a refusal installs no client and no listener; the test pins the listener counts. CI baked in secrets.SENTRY_DSN, the app's slug-project DSN the JS SDK refuses outright, so every desktop bridge shipped inert. The builds now read SENTRY_DSN_BRIDGE, which does not exist yet — until it does, reporting stays off loudly rather than silently. tracesSampleRate, spotlight and debug are pinned because getClientOptions fills each from the ambient environment, and the host inherits its environment from whatever spawned it. SENTRY_SPOTLIGHT would fan every envelope to a second loopback destination and SENTRY_TRACES_SAMPLE_RATE would emit transactions, which beforeSend never sees. Also: Modules excluded and event.modules dropped (it walks up from a cwd the host did not choose); redactNullable no longer throws on a null, which beforeSend would swallow into a dropped event; redactDeep uses fromEntries so a __proto__ key cannot silently delete its sibling; debug_meta code_file redacted; a failed first-project open is captured before its bare process.exit; startControlPlane moved below the handler registration, since host.json on disk lets the app drive project:open during the relay handshake. * App: scrub the breadcrumbs the native layer copies, and say what stays out of reach beforeSend is not the whole story where there is a native layer. sentry_flutter's C binding never calls sentry_options_set_before_send, so sentry-native writes and posts its own envelope for a native crash — and the nativeDatabasePath fix is precisely what turns that path on for the first time, taking it from broken-and-silent to working-and-leaking. beforeBreadcrumb runs before NativeScopeObserver mirrors the scope down, so it is what keeps a path out of the copy the native layer holds. Frames and contexts of a native crash stay unreachable from Dart; the comment now says so rather than implying coverage. The support-dir catch restored the exact behaviour the function exists to fix and had no symptom by construction: no handler process, no database, no release-health session. It now warns. Consent reads through telemetryEnabledProvider, and frame module/package are redacted alongside absPath — native frames carry an absolute path there. fileName deliberately is not: a Dart frame's is a package:/dart: URI, and _pathLike would eat it. The symbols note claimed native frames can be hand-symbolicated against the build's PDBs. No workflow archives them, and the toolchains are not bit-reproducible, so rebuilding the tag yields build ids that do not match. Recorded as the gap it is. * The rename field's disposal waits for the frame, not just the microtask Deferring to scheduleMicrotask unwound the focus notification but still landed inside the frame showing the TextField, so the field outlived the controller and focus node it is built against — any pointer, key or traversal event in that gap touches a disposed ChangeNotifier. A post-frame callback runs after the setState rebuild that takes the field down. The regression test also closed neither the CachedSessionsStore nor the ProjectSession; both own timers and subscriptions that would outlive the tree and fail some later test with a pending-timer assertion pointing nowhere near this file.
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
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.
Three fixes. The first was the request; the second and third are what looking for
evidence that the first worked turned up.
1. The bridge reports its own crashes
The host process had no crash reporting at all. It now runs
@sentry/bunbehindthree gates that must all hold: the user's telemetry consent, carried on the stdin
bootstrap as
telemetryEnabledand read once for the host's lifetime (absent meansoff, so a CLI or test host never reports); a
SENTRY_DSNbaked in by--define,exactly like
LICENSE_API_URL; and a scrubber that strips paths, source lines,locals and the hostname, kept in lockstep with the app's.
Every integration that reads request bodies or source off disk is excluded, with a
per-name reason in the file.
OnUncaughtException/OnUnhandledRejectionaredeliberately kept — they are what stamps a fatal
handled: false, and ahand-rolled
captureExceptionreports the same crash asgeneric/handled: true.Their contract has a second half in
index.ts: the SDK re-counts the otheruncaughtExceptionlisteners at crash time, so it defers to our teardown onlywhile one of ours is registered.
A DSN the SDK silently refuses
Sentry.initnever throws and never returns a status. A DSN it rejects leaves atransport-less client on which every
captureException— andflush— stillsucceeds, sending nothing.
That is measured, not hypothetical:
The JS SDKs require a numeric project id; errex issues slugs, so the DSN CI
bakes in is refused outright and this feature would have shipped completely inert.
Init now verifies
getClient()?.getDsn()and logs at error instead of trustingitself.
sentry-darttakes the last path segment as an opaqueString, which iswhy only this side is affected.
2. Windows shipped with no native crash capture
sentry_flutternever assignsnativeDatabasePath, and sentry-native then fallsback to
.sentry-nativerelative to the current working directory. That isunwritable in exactly the configuration we ship: a Store-launched MSIX gets
C:\Windows\System32as its cwd, and its own install dir underWindowsAppsisread-only.
sentry_initfails, and native capture is absent with nothing to noticeit by — no crashpad handler, no database, and (auto-session-tracking being a native
option) no release-health sessions either.
Measured on the shipped
1.20698.1008package:sentry.dllloaded, no.sentry-nativeanywhere on the machine, nocrashpad_handlerprocess. Theonly
.sentry-nativeon disk belonged to an unrelated app, sitting in%TEMP%—which is what confirmed the cwd-relative default.
Dart-level reporting was unaffected throughout, which is why production kept
receiving Dart fatals while every native crash was lost.
Also turns
options.debugon outside release builds. The SDK reports its own initfailures at debug level and nowhere else, which is the entire reason this went
unnoticed: a broken native layer looks exactly like an app that never crashed.
docs/release/build.mdrecords that errex has no symbol-upload endpoint —sentry-cli's chunk-upload and legacy dsyms paths both 404 while implemented routes
answer 401 — so a
sentry-cli upload-difstep would only fail in CI.3. A production fatal, traced to our code
Every frame was
in_app: false, which made it read as a framework bug. It is ours.applyFocusChangesIfNeedednotifies withfor (final node in _dirtyNodes) node._notify(),and
FocusNode.disposedetaches, which makesFocusManager._markDetacheddo_dirtyNodes.remove(node)— mutating the Set being iterated. The chain:_exitEditnow clears the fields first and disposes in a microtask, so thenotification unwinds before the detach. Fixed there rather than at the callback so
both commit triggers — Enter and blur — are covered.
Repro: rename a session, then click away to blur the field.
Testing
bun run --filter antgrid-bridge testtsc --noEmitflutter testflutter analyzedart formatsmoke-hook-binary.tsSMOKE=0The rename regression test drives the real row — double-tap to rename, move focus
away, assert no exception. Reverting the fix makes it throw the same
ConcurrentModificationError, verified.Still open
antgrid-app-stagingreturns a generic400 invalid envelopeto everything —an empty body, the string
not-an-envelope, and a byte-exactsentry.dart/9.22.0envelope captured off the wire and replayed. Production accepts real events, so this
looks like that project not being provisioned rather than a client problem. No test
event was ever delivered.
Separately: that production event has
timestamp 2026-08-29T06:14:18Zbutreceived_at 2026-09-02T04:37:01Z. Ifreceived_atis genuine ingest time, reportsare arriving four days late and are not usable for monitoring.