Skip to content

feat(mobile): report network and app errors to Sentry with rich metadata - #6109

Open
iscekic wants to merge 1 commit into
mainfrom
kwf/surface-the-mobile-app-apps-mobile-9518
Open

feat(mobile): report network and app errors to Sentry with rich metadata#6109
iscekic wants to merge 1 commit into
mainfrom
kwf/surface-the-mobile-app-apps-mobile-9518

Conversation

@iscekic

@iscekic iscekic commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Changelog for users

  • Failed tRPC and other HTTP requests now reach Sentry at warning level with request metadata.
  • The app adds no new banners, toasts, or dialogs; existing error screens, retry controls, and mutation toasts are unchanged.
  • A failed call inside a mixed batch is recorded even when the screen keeps showing its successful results.
  • Pinned screen headers now reserve the offline banner's height, so the banner no longer covers titles such as Profile or a pull request.
  • Modal sheet headers now use the same fixed top clearance on Android and iOS instead of adding the app's status-bar inset, so Android sheets no longer sit too low.
  • Modal sheet headers no longer reserve space for the offline banner, so an offline connection does not push a sheet title down.
  • Reports to Sentry no longer carry token-shaped values nested inside an error's own properties.
  • App errors whose only code is a top-level code reach Sentry instead of being dropped as already reported.

Changelog for maintainers

  • apps/mobile/src/lib/telemetry/error-sink.ts is a transport-neutral captureTelemetry/setTelemetrySink layer that must never import @sentry/react-native; apps/mobile/src/lib/telemetry/install-error-reporting.ts attaches the Sentry adapter and wraps global fetch once with a guard on globalThis, and the root layout installs it at module scope.
  • observedFetch in apps/mobile/src/lib/trpc.ts wraps deadlineFetch and reports every rejection, HTTP >= 400, and batched HTTP 207 once at warning level; the global wrapper covers other http(s) requests and skips /api/trpc plus Sentry, PostHog, AppsFlyer, and Expo hosts. Network events tag error.subsystem, error.source, network.outcome, http.method, http.status, http.status_class, trpc.procedure, and trpc.code; contexts.network carries the query-stripped URL, duration, status, procedure, code, and timedOut. Bodies and headers are never captured; review the network-error fingerprint string first.
  • React Query query and mutation errors call reportAppError, which skips tRPC-named errors, a tRPC data.code or shape.data.code envelope, CancelledError, and RequestDeadlineError; uncaught JS errors, unhandled rejections, and render or route errors were already covered by the SDK handlers and Sentry.wrap. Review the app-error fingerprint string first.
  • extraErrorDataIntegration now registers in both consent branches, and scrubEvent redacts token-shaped values at any depth in extra, tags, and the exception-named context the integration attaches, with a cycle guard.
  • apps/mobile/src/components/screen-header.tsx no longer forks header padding on Platform.OS: modal alone selects a fixed 32 top padding (MODAL_HEADER_TOP_PADDING), and only a pinned header adds the reported safe-area inset plus 8; a native sheet is its own window that reports the inset it needs, so the fixed clearance holds on both platforms. Modal headers no longer reserve the offline banner height on either platform (replacing the iOS-only exception), while a pinned header still does through the leaf module in apps/mobile/src/components/offline-banner-space.tsx; apps/mobile/src/components/screen-header.mounted.test.tsx flips Platform.OS per assertion and pins iOS and Android to identical padding for modal and pinned headers, with and without the offline banner.
  • apps/mobile/src/lib/sentry-init.ts:86 — accepted: extraErrorDataIntegration attaches a thrown error's own properties under a context named after the exception, and scrubEvent redacted only top-level extra and tags, so token-shaped values in that context reached Sentry unredacted. scrubEvent now walks the named context at any depth with a cycle guard; tests cover a nested token and a cyclic value.
  • apps/mobile/src/lib/telemetry/install-error-reporting.ts:24 — accepted: the module-local installed guard reset when Fast Refresh re-evaluated the module, so globalThis.fetch was wrapped again and reports were duplicated. The guard now lives on globalThis and survives Fast Refresh; a test loads a fresh module instance, installs again, and asserts the fetch wrapper is unchanged.
  • apps/mobile/src/lib/telemetry/app-error-reporting.ts:49 — accepted: dedup treated any error with a top-level string code as already reported by the network layer, so unrelated errors such as a filesystem ENOENT were dropped. It now matches only a tRPC code envelope (data.code or shape.data.code) and reports an error whose only code is top-level.

E2E proof

p9-offline-nav.mp4
p3-rec.mp4
e1-toast-rec.mp4

p1.png

p2.png

p2-recovered.png

e1-toast-live.png

e1-immediate-toast.png

e1-after-6s.png

p3toast.png

p4.png

p4s.png

p5-down.png

p5-recover.png

p6-first.png

offlineerr.png

recerr.png

p8-error.png

p8-retry.png

p8-interactive.png

e1-offline-profile.png

e-online-profile.png

e1-recover.png

[e-boot] android app boot with module-scope installErrorReporting() -> pass :: android emulator-5554; e-boot.log:1 'SCENE e-boot OK' and the Home digest shows the app rendered after the root-layout telemetry install, so the new module-scope side effect does not break bootstrap.
[e1] Open a modal sheet that renders ScreenHeader on the host platform; confirm the header keeps the fixed 32dp top clearance and is not pushed down by the status-bar inset. -> pass :: android/emulator-5554: modal-presented Kilo Pass screen shows the ScreenHeader at content-top 84px = 32.0dp (density 420, e1-density.log) — e1.log spans `Go back tappable [37,84][138,185]` and `Kilo Pass tappable [138,102][944,167]` — while the non-modal control screen renders at 149px = insets.top(48.8dp)+8dp (e1-control-preferences.log span `Go back tappable [0,149][101,250]`), so the modal header keeps the fixed 32dp clearance and is not pushed down by the status-bar inset; the named session page sheet renders its ScreenHeader inside the sheet (e1-session-page-sheet.log); no UX-DEFECT.
[e2] Go offline on a pinned non-modal screen (Profile); confirm the offline banner does not cover the title because the header still reserves the banner height. -> pass :: android/emulator-5554: with wifi+data off the banner paints at the safe-area top (e2.log span `No internet connection tappable [0,128][1080,223]`) and the pinned Profile title sits below it (e2.log span `Profile tappable [37,244][1045,350]`), 95px = 36dp lower than the online baseline 149px (e2-baseline-online.log span `Profile tappable [37,149][1045,255]`), i.e. the header reserves OFFLINE_BANNER_HEIGHT(36) so the banner cannot cover the title; after network restore the banner is gone (e2-recovered.log); no UX-DEFECT.

The new [e1] modal-header-clearance and [e2] pinned-header-reservation checks replace the earlier pinned-header reservation and post-restore [e1]/[e1-recover] checks; the pinned-header behavior is re-measured on the branch that removes the platform fork, and its captures stay valid because the repair does not change that behavior. Both checks ran on the Android emulator; the change removes the platform branch, so iOS shares the same modal rule. The remaining captures are unchanged. The mutation-toast captures are visual-only: the toast text is not present in the accessibility hierarchy or logcat.

Owner request

Surface: the mobile app (apps/mobile).

Enrich Sentry observability massively in two aspects:

  • Log all network errors (trpc or otherwise) with rich metadata, at warning level.
  • Log all errors in general (we may already do this; confirm and close any gaps).
  • If there are any other reasonable observability gaps with Sentry, fix them.

@iscekic
iscekic marked this pull request as draft September 13, 2026 05:42
Comment thread apps/mobile/src/lib/sentry-init.ts
Comment thread apps/mobile/src/lib/telemetry/install-error-reporting.ts Outdated
Comment thread apps/mobile/src/lib/telemetry/app-error-reporting.ts Outdated
@kilo-code-bot

kilo-code-bot Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (2 files)
  • apps/mobile/src/lib/telemetry/sentry-scrub.ts
  • apps/mobile/src/lib/telemetry/sentry-scrub.test.ts
Previous Review Summaries (2 snapshots, latest commit 145bc86)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 145bc86)

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
apps/mobile/src/lib/telemetry/sentry-scrub.ts 49 The cycle guard returns the original object, so token-shaped values reachable through a repeated/aliased reference still reach Sentry unredacted
Files Reviewed (12 files)
  • .kwf-keep-device
  • apps/mobile/src/components/screen-header.mounted.test.tsx
  • apps/mobile/src/components/screen-header.tsx
  • apps/mobile/src/lib/sentry-init.ts
  • apps/mobile/src/lib/telemetry/app-error-reporting.test.ts
  • apps/mobile/src/lib/telemetry/app-error-reporting.ts
  • apps/mobile/src/lib/telemetry/install-error-reporting.test.ts
  • apps/mobile/src/lib/telemetry/install-error-reporting.ts
  • apps/mobile/src/lib/telemetry/network-errors.test.ts
  • apps/mobile/src/lib/telemetry/sentry-scrub.test.ts
  • apps/mobile/src/lib/telemetry/sentry-scrub.ts - 1 issue
  • apps/mobile/src/lib/trpc.test.ts

Fix these issues in Kilo Cloud

Previous review (commit 9a94526)

Status: 3 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 2
Issue Details (click to expand)

WARNING

File Line Issue
apps/mobile/src/lib/sentry-init.ts 86 extraErrorDataIntegration (also line 68) attaches nested error properties that scrubEvent never walks/redacts, contradicting the comment's redaction guarantee

SUGGESTION

File Line Issue
apps/mobile/src/lib/telemetry/install-error-reporting.ts 24 Module-local installed guard resets on Fast Refresh, re-wrapping globalThis.fetch and stacking wrappers (duplicate reports + retained closures)
apps/mobile/src/lib/telemetry/app-error-reporting.ts 49 Dedup keys on any top-level string code, so unrelated app errors can be silently dropped
Files Reviewed (22 files)
  • .kwf-keep-device
  • apps/mobile/src/app/_layout.tsx
  • apps/mobile/src/components/offline-banner-space.tsx
  • apps/mobile/src/components/offline-banner.tsx
  • apps/mobile/src/components/pr-review/pr-review-screen.test.tsx
  • apps/mobile/src/components/pr-review/pr-review-screen.tsx
  • apps/mobile/src/components/screen-header.mounted.test.tsx
  • apps/mobile/src/components/screen-header.tsx
  • apps/mobile/src/lib/query-client.test.ts
  • apps/mobile/src/lib/query-client.ts
  • apps/mobile/src/lib/sentry-init.ts - 1 issue
  • apps/mobile/src/lib/telemetry/app-error-reporting.test.ts
  • apps/mobile/src/lib/telemetry/app-error-reporting.ts - 1 issue
  • apps/mobile/src/lib/telemetry/error-sink.test.ts
  • apps/mobile/src/lib/telemetry/error-sink.ts
  • apps/mobile/src/lib/telemetry/install-error-reporting.test.ts
  • apps/mobile/src/lib/telemetry/install-error-reporting.ts - 1 issue
  • apps/mobile/src/lib/telemetry/network-errors.test.ts
  • apps/mobile/src/lib/telemetry/network-errors.ts
  • apps/mobile/src/lib/telemetry/transport-spy.test.ts
  • apps/mobile/src/lib/trpc.test.ts
  • apps/mobile/src/lib/trpc.ts

Fix these issues in Kilo Cloud


Reviewed by deepseek-v4.1-flash · Input: 0 · Output: 0 · Cached: 0

Review guidance: REVIEW.md from base branch main

@iscekic
iscekic force-pushed the kwf/surface-the-mobile-app-apps-mobile-9518 branch from 796d71c to 0c639ef Compare September 13, 2026 06:18
@iscekic
iscekic force-pushed the kwf/surface-the-mobile-app-apps-mobile-9518 branch from 0c639ef to 2a0ba24 Compare September 13, 2026 07:18
@iscekic
iscekic force-pushed the kwf/surface-the-mobile-app-apps-mobile-9518 branch from 2a0ba24 to 90933f5 Compare September 13, 2026 08:50
@iscekic
iscekic force-pushed the kwf/surface-the-mobile-app-apps-mobile-9518 branch from 90933f5 to b73b73a Compare September 13, 2026 09:11
@iscekic
iscekic force-pushed the kwf/surface-the-mobile-app-apps-mobile-9518 branch from b73b73a to 145bc86 Compare September 13, 2026 09:15
@iscekic

iscekic commented Sep 13, 2026

Copy link
Copy Markdown
Contributor Author

bot: Cross-platform fix. Pushed b73b73a (kwf kwf-fix-platform-6a59).

What changed:

  • apps/mobile/src/components/screen-header.tsx no longer reads Platform.OS: modal alone selects a fixed 32 top padding (MODAL_HEADER_TOP_PADDING), and only a pinned header adds the reported safe-area inset plus 8. A native sheet is its own window that reports the inset it needs, so the fixed clearance holds on both platforms; read the constant's comment before changing it.
  • Modal headers no longer reserve the offline banner height on either platform, replacing the iOS-only exception; a pinned header still reserves it.
  • apps/mobile/src/components/screen-header.mounted.test.tsx flips Platform.OS per assertion and pins iOS and Android to identical padding for modal and pinned headers, with and without the offline banner.

@iscekic
iscekic marked this pull request as ready for review September 13, 2026 09:40
Comment thread apps/mobile/src/lib/telemetry/sentry-scrub.ts Outdated
@iscekic
iscekic marked this pull request as draft September 13, 2026 09:44
@iscekic
iscekic force-pushed the kwf/surface-the-mobile-app-apps-mobile-9518 branch from 145bc86 to de8c387 Compare September 13, 2026 10:03
@iscekic
iscekic marked this pull request as ready for review September 13, 2026 10:12
@iscekic iscekic added the human-ready The PR is ready for human review. label Sep 13, 2026
@iscekic
iscekic requested a review from eshurakov September 13, 2026 10:20
@iscekic iscekic self-assigned this Sep 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

human-ready The PR is ready for human review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant