[mobile] Fix data readiness JSON response errors - #1460
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
📝 WalkthroughWalkthrough
ChangestrpcFetch JSON diagnostics
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Storybook previews for This comment updates automatically on each PR push. |
Mobile PreviewScan to open on device:
To test on device:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/mobile/lib/trpc-fetch.ts`:
- Around line 12-15: The empty-body detection in responseHasEmptyBody is too
heuristic and misses responses that are truly empty but lack a 204/205/304
status or content-length header. Update parseJsonBody to read response.text()
once and check for an empty/whitespace-only body before JSON.parse, then use
that same single read path for both the empty and invalid response error cases.
Keep the existing helpers like getTrpcPath, buildStatusLabel, and
buildContentTypeLabel for the error messages, but remove the separate
body-heuristic branch in responseHasEmptyBody.
- Around line 50-65: The catch in withJsonParseDiagnostics currently drops the
original JSON parse error and does not report it to Sentry. Update the json
override in withJsonParseDiagnostics to catch the thrown error, call
captureException from ./telemetry with that original exception, and then rethrow
a new error that preserves the original as the cause if possible. Keep the
existing getTrpcPath, buildStatusLabel, and buildContentTypeLabel context in the
final message.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a980c333-51e9-4d2f-bd71-9f3ffdafa010
📒 Files selected for processing (2)
packages/mobile/lib/trpc-fetch.test.tspackages/mobile/lib/trpc-fetch.ts
There was a problem hiding this comment.
No issues found across 2 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Auto-approved: Adds diagnostic error messages for empty or invalid JSON responses in mobile tRPC fetch, with test coverage.
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Auto-approved: Improves error handling for empty/invalid JSON responses in mobile tRPC fetch with diagnostics and telemetry.
Re-trigger cubic
|
Addressed CodeRabbit title warning.
|
Summary
Unexpected end of JSON inputfrom surfacing in data readiness errors by including the tRPC path, status, and content type.sync.dataHealthresponses and invalid JSON response bodies.Validation
pnpm lintpnpm tsc --noEmitcd packages/server && pnpm tsc --noEmitcd packages/web && pnpm tsc --noEmitpnpm vitest run --project mobile packages/mobile/lib/trpc-fetch.test.tspnpm test:mobilepnpm vitest run --project integration packages/server/src/routers/router-data.integration.test.tsNote: full
pnpm testreached 705 passing files and then hit the local ClickHouse 3 GiB memory cap inrouter-data.integration.test.ts; that suite passed when rerun in isolation with.env.localexported.Summary by cubic
Improves mobile tRPC fetch diagnostics by replacing JSON parsing with clearer errors for empty or invalid bodies, including the tRPC path, HTTP status, and content type. Also reports JSON parse failures to telemetry.
Bug Fixes
response.json()to detect empty bodies (including whitespace-only) and throw an actionable error.cause, and reports it via telemetry.Tests
causeand telemetry capture).Written for commit c0df2f0. Summary will update on new commits.
Summary by CodeRabbit