fix(mobile): stop reporting expected conditions to Sentry - #4831
Conversation
…ly skip new tests A test file added in a directory no include glob covers passes CI without ever running: vitest.config.ts composes two projects with enumerated includes, and a path outside every glob matches no project and is skipped with no warning. Symptom/cause/fix recorded; the fix side lands with the identity-step test in this section.
A failed logEvent delivery is a transport failure (offline, DNS-blocked,
ad-blocked, proxied) that the AppsFlyer SDK queues and retries itself;
the JS error callback carries a bare platform message string with no
structured discriminator, so no developer action is possible and every
report was noise (KILO-APP-1Z). Both logEvent error callbacks - the
trackEvent site and the pending-event drain - are now the existing noop.
Actionable integration failures still report: initSdk's error callback
("AppsFlyer init failed") and the purchase connector's are untouched.
Tests: transport failure not reported, queued-drain failure not
reported, and init failure still reported with its message intact.
…Sentry
Everything reaching the outer catch of handleGpsPress is an expected
user-environment condition - the app's own 10s GPS timeout (KILO-APP-27),
location services off, unsatisfied device settings (KILO-APP-2Z) - and
the UI already tells the user exactly what happened via locationFeedback
("Could not get your location. Enter it manually."). Nothing there is
developer-actionable, so the captureException is removed; the feedback
message, status, and loading handling are byte-identical.
The inner catch's validateWeatherLocation capture stays: that procedure
is first-party, so its failures are actionable and must keep reporting.
Tests: positive control (validateWeatherLocation rejection still reports
the exact error object) plus both production expected-condition messages
asserting no report, no backend mutation, and that the handler ran. The
new suite needs a mobile-pure include for src/components/kiloclaw -
added as the narrowest matching glob (see the committed learning).
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummarySince the last review, only two new workflow-learning markdown files were added (E2E slot ownership and iOS simulator location-fix notes); the previously reviewed production files ( Files Reviewed (2 new files)
Previous Review Summary (commit c0ce401)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit c0ce401)Status: No Issues Found | Recommendation: Merge Executive SummaryReviewed the three production callback/catch-block removals ( Files Reviewed (6 files)
Reviewed by claude-sonnet-5 · Input: 32 · Output: 7.4K · Cached: 832.1K Review guidance: REVIEW.md from base branch |
1 participant
Summary
What: stop reporting two classes of expected user-environment failures to Sentry from
apps/mobile— AppsFlyer event-delivery transport failures, and the KiloClaw onboarding identity step's GPS lookup failures. The production change is three lines in two files (two error callbacks swapped for the existingnoop, onecaptureExceptiondeleted), plus unit tests and one vitest include line. No UI, copy, state, or control flow changes.Why: production Sentry noise. Governing rule applied throughout: report only what a developer could act on.
How: per-issue dispositions, each traced against the live code path on
main:AppsFlyer event "…" failed: Failed to connect …:443logEventerror callbacks (trackEventand the pending-event drain inappsflyer.ts) are nownoop. A delivery failure is a transport condition (offline, DNS-blocked, ad-blocker, corporate proxy) that the SDK queues and retries itself; the JS error callback carries an untyped bare message string with no structured discriminator, so a transport/config split is not reliably possible.Error: timeoutGPS_TIMEOUT_MSrace rejects into the outercatchofhandleGpsPress; itsSentry.captureExceptionis removed. The user is told exactly what happened vialocationFeedback("Could not get your location. Enter it manually."), unchanged.Location request failed due to unsatisfied device settingsexpo-location's rejection lands in the same outercatch; the same one removal covers it.mainNotifications.getExpoPushTokenAsyncis called from exactly two places, both awaited insidetryblocks:notifications-step.tsx(catch renders an inline message with Try again / Skip for now) andnotifications-screen.tsx(catch shows a "Could not enable notifications" toast).getDevicePushTokenrejections are caught by React Query and rendered viaisError. NocaptureExceptionexists innotifications.ts,notifications-screen.tsx, ornotifications-step.tsx, and the global TanStack error hooks never touch Sentry.WebBrowser's auth session is in an invalid state…mainopenAuthSessionAsynccall sites catch and none reports to Sentry. The error only originates inexpo-web-browser's Android polyfill, which was already removed from the device-auth path that produced these events in #4376.No actionable report was removed. Deliberately retained:
AppsFlyer init failed(initSdk error callback — catches a bad dev key / app id / SDK start failure),AppsFlyer purchase connector failed, and the identity step's innercaptureException(validateError), which reports failures of the first-partykiloclaw.validateWeatherLocationtRPC procedure.Observed but not fixed (KILO-APP-22 nuance): one
openAuthSessionAsyncsite — theOpen GitHubbutton atnew-session-repository-section.tsx:79viaagent-chat/new.tsx— has no in-flight re-entry guard, so an Android double-tap there can still throw and show its (caught, recoverable) toast. Left alone deliberately: that site emits zero Sentry events, so it sits outside this PR's Sentry-noise mandate; the observable outcome is already a recoverable toast plus retry; and adding button state would be a user-visible behaviour change in a PR whose stated invariant is that user-visible behaviour does not change.Acceptance criterion 3 (push-token failures surface handled; registration behaviour unchanged apart from reporting) is discharged by the code evidence above on
main, not by a device observation — no push-token code changed in this PR.Unit coverage: the new negative tests were verified red-before / green-after against the production edits — against unmodified code they fail with
captureException … called 1 timescarrying the exact production errors, proving they reach the code under test rather than passing vacuously. Fullapps/mobilechecks green: format, typecheck, lint (993 files, 0 errors), knip, and 278 test files / 2343 tests; the newidentity-step.test.tsxruns in themobile-pureproject (its include line is part of this PR — without it the suite would silently never execute, now recorded as a workflow learning).Verification
apps/mobilegreen:pnpm format,pnpm typecheck,pnpm lint(993 files, 0 errors),pnpm check:unused,pnpm test(278 files / 2343 tests —identity-step.test.tsxexecuting inmobile-pure),git diff --checke2e-verifierround against the final head; byte-identical git baseline before/after:catchwas driven deterministically (iOS 26.5 simulators cannot starvegetCurrentPositionAsync—simctl location clearis non-functional, so Location Services were toggled off to reach the identical catch; recorded as a workflow learning). Feedback line exactlyCould not get your location. Enter it manually., no red box/crash/toast, input still editable,Continueenabled. Screenshots:r1-feedback-line-final.png,r1-spinner-mid.png.52.37,4.90→ input filledAmsterdam, The Netherlandswith the weather feedback line. Screenshot:r2-happy-path.png.mobilepane (one pre-existing benign RN-bridge WARN that confirmsinitSdkran behind consent). Screenshot:r3-cold-launch-identity.png.Enable notifications→ permission allowed → token fetch succeeded →Push notifications are on this device.No crash or unhandled rejection. Screenshot:r4-enabled-state.png.Visual Changes
N/A
Reviewer Notes
Feature-state matrix: not applicable — this PR adds no user-facing feature; it removes error-reporting side effects from existing flows and changes no UI, copy, state, or control flow.
Focus areas: the two retained AppsFlyer reports and the retained
validateWeatherLocationcapture are the signal this PR deliberately keeps; the identity-step diff should be exactly "one capture removed, everything else byte-identical".