Skip to content

fix(mobile): group Sentry issues by a stable network fingerprint - #6619

Merged
iscekic merged 1 commit into
mainfrom
kwf/req-fingerprint-b12f
Sep 23, 2026
Merged

iscekic merged 1 commit into
mainfrom
kwf/req-fingerprint-b12f

Conversation

@iscekic

@iscekic iscekic commented Sep 23, 2026

Copy link
Copy Markdown
Collaborator

Changelog for users

  • One network defect now reaches Sentry as one issue instead of one issue per ephemeral port, host, or build path.
  • Different HTTP outcomes of one procedure become separate issues, so a 401 and a 412 no longer merge.
  • Unhandled errors with no fingerprint group by exception class and a normalized message, so a new build path no longer creates a new issue.
  • Issue titles come from the error message instead of SDK frames such as Scope#captureException.

Changelog for maintainers

  • A URL is normalized before the message and the fingerprint: scheme, host, port, and query drop, the path stays; the normalized path replaces the raw URL in both.
  • The fingerprint outcome key is http.<status> when a status is set, else the tRPC code, else the transport outcome; statusClass is removed.
  • scrubEvent fills a fingerprint only when none is set: the exception class plus the message with URL origins, absolute paths, host:port, and query strings removed.
  • The fallback reads at most 512 characters and the host:port pattern is anchored, so a long message cannot make beforeSend quadratic.
  • A non-Error tRPC body now rides in the new network.body context, which scrubEvent token-scrubs; captureException always receives a real Error named NetworkError.
  • The payload context avoids contexts[error.name] because extraErrorDataIntegration replaces that key with {} before beforeSend runs.
  • Redaction is unchanged: query strings, user identity fields, and 20+ character token-shaped runs are still stripped.
  • Review the volatile-value patterns and the network.body contract first; the reporter opts out of max-lines to keep its helpers together.

E2E proof

[e1] is a host-only unit scenario (this host has no device tool); log file checks.log:

CHECK PASS (2s): pnpm --filter kilo-app exec vitest run src/lib/telemetry/sentry-scrub-fingerprint.test.ts src/lib/telemetry/fingerprint-policy.test.ts src/lib/telemetry/network-errors.test.ts src/lib/telemetry/sentry-scrub.test.ts src/lib/telemetry/sentry-scrub-identifier-paths.test.ts

  ✓ mobile-pure  src/lib/telemetry/fingerprint-policy.test.ts (5 tests) 27ms
  ✓ mobile-pure  src/lib/telemetry/sentry-scrub-fingerprint.test.ts (8 tests) 12ms

 Test Files  5 passed (5)
      Tests  101 passed (101)
Owner request

Surface: mobile-app

The mobile app sends one defect to Sentry as many issues, and it sends
different defects to Sentry as one issue. Both come from one cause: the app has
no fingerprint policy.

Fix the policy in the mobile telemetry path. Sentry must group one defect as one
issue, and two defects as two issues.

The evidence

A 24 hour pull of the kilo-app project found 616 issues with activity. Those 616
issues carry only 112 distinct messages. 497 of them sit in 15 clusters.

Three failures, all confirmed against live events:

  1. The fingerprint keeps the ephemeral port. apps/mobile/src/lib/telemetry/network-errors.ts
    builds fingerprint: ['network-error', context.source, procedure ?? pathOnly, statusClass ?? outcome]
    and pathOnly is the whole URL with the port. KILO-APP-26Y is
    POST http://127.0.0.1:10416/v1/latency -> 401. KILO-APP-287 is
    POST http://127.0.0.1:10216/v1/latency -> 401. They are one defect in two
    issues.

  2. The fingerprint is too coarse. The same line keys on statusClass, so
    4xx covers every client error. KILO-APP-C4 holds one procedure
    (activeSessions.createWebTicket) with two different outcomes: HTTP 412
    PRECONDITION_FAILED and HTTP 401 UNAUTHORIZED. Sentry merged two
    different root causes into one issue.

  3. The native path carries no fingerprint at all. An unhandled fetch failed
    error from expo-modules-core has no explicit fingerprint, so Sentry groups
    it by the stack. The stack holds the absolute build path of the worktree that
    built the app, so every worktree makes a new issue. 87 separate issues carry
    the message Error: fetch failed: java.net.ConnectException: Failed to connect to /<host>:<port>.
    KILO-APP-102, KILO-APP-B5, KILO-APP-2ER, KILO-APP-2GF and
    KILO-APP-2EZ differ only in the worktree path inside the culprit.

A fourth defect makes the issues unreadable. network-errors.ts passes a tRPC
plain object to Sentry.captureException when context.error is set. Sentry
then takes the title from an SDK frame, so 64 issues are titled
Scope#captureException, 17 are setTelemetrySink$argument_0, and 7 are
Scope.

What to build

  1. In network-errors.ts, normalize the URL before it reaches the message and
    the fingerprint. Keep the path. Drop the scheme, the host and the port. Use
    the normalized path in fingerprint and in buildSyntheticError.

  2. In network-errors.ts, replace statusClass in the fingerprint with the
    specific outcome. Use http.status when it is set, otherwise trpc.code,
    otherwise outcome. A 401 and a 412 must not share a fingerprint.

  3. Add a fingerprint fallback in beforeSend in
    apps/mobile/src/lib/telemetry/sentry-scrub.ts (scrubEvent), so a capture
    site that sets no fingerprint cannot split a group by a volatile value. The
    fallback must use the exception class and the message with the volatile
    values removed: absolute build paths, absolute worktree paths, host and port.

  4. Always capture a real Error with a stable message. Do not pass a plain
    object to captureException. Keep the object in contexts, where
    scrubEvent already redacts it.

Do not weaken the redaction. scrubEvent must still strip query strings, user
identity, and token-shaped runs. A fingerprint must never carry a query string,
a token, or a user identifier.

Proof

One must-run scenario. Two events that differ only in a volatile value must land
in one Sentry group, and two events that differ in the outcome must land in two
groups.

Prove it with a unit test that drives the real sink and the real beforeSend:
one pair that differs only by port, one pair that differs only by build path,
one pair that differs only by HTTP status (401 against 412).

Quote the decisive log lines from the scenario run in the PR body. A log excerpt
is the proof for this change.

Open findings (not fixed here)

  • not fully verified: some optional checks did not run

@iscekic
iscekic marked this pull request as draft September 23, 2026 05:04
Comment thread apps/mobile/src/lib/telemetry/sentry-scrub.ts Outdated
Comment thread apps/mobile/src/lib/telemetry/sentry-scrub-fingerprint.test.ts Outdated
@kilo-code-bot

kilo-code-bot Bot commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (3 files)
  • apps/mobile/src/lib/telemetry/network-errors.ts
  • apps/mobile/src/lib/telemetry/network-errors.test.ts
  • apps/mobile/src/lib/telemetry/fingerprint-policy.test.ts
Previous Review Summaries (2 snapshots, latest commit 16f572b)

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

Previous review (commit 16f572b)

Status: 1 Issue Found | Recommendation: Address before merge

Executive Summary

Batched tRPC failures still key on the 207 multi-status envelope, discarding the specific inner status/code the fingerprint policy is meant to separate.

Overview

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

WARNING

File Line Issue
apps/mobile/src/lib/telemetry/network-errors.ts 274 Batched tRPC failures key on the 207 envelope, discarding the parsed inner trpc.httpStatus/trpc.code
Files Reviewed (6 files)
  • apps/mobile/src/lib/telemetry/network-errors.ts - 1 issue
  • apps/mobile/src/lib/telemetry/sentry-scrub.ts - 0 issues
  • apps/mobile/src/lib/telemetry/network-errors.test.ts - 0 issues
  • apps/mobile/src/lib/telemetry/sentry-scrub.test.ts - 0 issues
  • apps/mobile/src/lib/telemetry/fingerprint-policy.test.ts - 0 issues
  • apps/mobile/src/lib/telemetry/sentry-scrub-fingerprint.test.ts - 0 issues

Fix these issues in Kilo Cloud

Previous review (commit 11bf868)

Status: 2 Issues Found | Recommendation: Address before merge

Executive Summary

The fingerprint policy correctly fixes the reported port, worktree-path, and 401-vs-412 grouping cases; both findings are low-severity pattern/test issues in the new fallback fingerprint path.

Overview

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

SUGGESTION

File Line Issue
apps/mobile/src/lib/telemetry/sentry-scrub.ts 42 HOST_PORT_PATTERN also rewrites file:line/clock runs to <host>, which can merge distinct un-fingerprinted defects
apps/mobile/src/lib/telemetry/sentry-scrub-fingerprint.test.ts 99 Long-message test uses a space-separated string, so it does not reproduce the contiguous word run it claims to guard
Files Reviewed (6 files)
  • apps/mobile/src/lib/telemetry/network-errors.ts - 0 issues
  • apps/mobile/src/lib/telemetry/sentry-scrub.ts - 1 issue
  • apps/mobile/src/lib/telemetry/network-errors.test.ts - 0 issues
  • apps/mobile/src/lib/telemetry/sentry-scrub.test.ts - 0 issues
  • apps/mobile/src/lib/telemetry/fingerprint-policy.test.ts - 0 issues
  • apps/mobile/src/lib/telemetry/sentry-scrub-fingerprint.test.ts - 1 issue

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/req-fingerprint-b12f branch from 9ee0098 to 16f572b Compare September 23, 2026 06:25
@iscekic
iscekic marked this pull request as ready for review September 23, 2026 06:40
Comment thread apps/mobile/src/lib/telemetry/network-errors.ts Outdated
@iscekic
iscekic marked this pull request as draft September 23, 2026 07:01
@iscekic
iscekic force-pushed the kwf/req-fingerprint-b12f branch from 54895b2 to 63fb296 Compare September 23, 2026 07:50
@iscekic
iscekic marked this pull request as ready for review September 23, 2026 08:02
@iscekic iscekic added the human-ready The PR is ready for human review. label Sep 23, 2026
@iscekic iscekic self-assigned this Sep 23, 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.

2 participants