[mobile] Diagnose mobile tRPC non-JSON responses - #1252
Merged
Merged
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? |
Contributor
Reviewer's GuideAdds a diagnostic tRPC fetch wrapper for the mobile app that intercepts non-JSON responses before JSON parsing, throws an error with path/status/body preview for Sentry visibility, wires it into the mobile tRPC client, and documents the related production incident and remaining risk. Sequence diagram for the new mobile tRPC fetch wrapper handling non-JSON responsessequenceDiagram
participant MobileApp
participant TrpcClient
participant TrpcFetchWrapper as createTrpcFetch
participant FetchImpl as fetchImpl
participant Server
MobileApp->>TrpcClient: tRPC request
TrpcClient->>TrpcFetchWrapper: fetch(input, init)
TrpcFetchWrapper->>FetchImpl: fetchImpl(input, init)
FetchImpl->>Server: HTTP request
Server-->>FetchImpl: HTTP response
FetchImpl-->>TrpcFetchWrapper: Response
alt JSON response
TrpcFetchWrapper-->>TrpcClient: Response (unchanged)
TrpcClient-->>MobileApp: tRPC result
else Non_JSON_response
TrpcFetchWrapper->>TrpcFetchWrapper: getTrpcPath(input)
TrpcFetchWrapper->>TrpcFetchWrapper: buildStatusLabel(Response)
TrpcFetchWrapper->>TrpcFetchWrapper: Response.clone().text()
TrpcFetchWrapper-->>TrpcClient: Error("Non-JSON tRPC response from ...")
TrpcClient-->>MobileApp: Surface error for Sentry
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
📝 WalkthroughWalkthroughAdds a mobile-side fetch wrapper that validates tRPC HTTP responses are JSON before parsing, integrates it into the tRPC client configuration in ChangesMobile tRPC JSON validation
🎯 1 (Trivial) | ⏱️ ~8 minutes
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Consider making
getTrpcPathresilient to relative URLs by either passing a base URL tonew URLor wrapping the construction in a try/catch so a malformed/relativeinputdoesn't throw and crash the fetch wrapper. - The JSON detection currently relies on
content-typeincludingjson; if some upstreams return JSON without a proper header, this wrapper will misclassify them as non-JSON—consider allowing a fallback path (e.g., attempting JSON parse on common 2xx responses with missing/ambiguous content types) or at least logging the unexpected header value.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider making `getTrpcPath` resilient to relative URLs by either passing a base URL to `new URL` or wrapping the construction in a try/catch so a malformed/relative `input` doesn't throw and crash the fetch wrapper.
- The JSON detection currently relies on `content-type` including `json`; if some upstreams return JSON without a proper header, this wrapper will misclassify them as non-JSON—consider allowing a fallback path (e.g., attempting JSON parse on common 2xx responses with missing/ambiguous content types) or at least logging the unexpected header value.
## Individual Comments
### Comment 1
<location path="packages/mobile/lib/trpc-fetch.ts" line_range="33-36" />
<code_context>
+ return response;
+ }
+
+ const bodyPreview = (await response.clone().text()).slice(0, BODY_PREVIEW_LIMIT);
+ throw new Error(
+ `Non-JSON tRPC response from ${getTrpcPath(input)}: ${buildStatusLabel(response)} ${bodyPreview}`,
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Guard against failures when reading the response body for the error preview.
If `response.clone().text()` throws (e.g. due to a read error or unsupported body), that error will replace the more informative "Non-JSON tRPC response" error. Wrap the body read in a try/catch and, on failure, fall back to a safe default (e.g. empty or generic preview) while still throwing the tRPC error with status and path.
```suggestion
let bodyPreview: string;
try {
const responseText = await response.clone().text();
bodyPreview = responseText.slice(0, BODY_PREVIEW_LIMIT);
} catch {
bodyPreview = "";
}
throw new Error(
`Non-JSON tRPC response from ${getTrpcPath(input)}: ${buildStatusLabel(response)} ${bodyPreview}`,
);
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
|
Storybook previews for This comment updates automatically on each PR push. |
Contributor
Mobile PreviewScan to open on device:
To test on device:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 3-9: getResponseContentType currently returns an empty string when
the header is absent; change its return type to string | null and return null
for missing content-type, then update isJsonResponse to narrow that nullable
return (e.g., check for non-null before calling includes or use a safe optional
check) so it treats absence as null rather than an empty string; update type
annotations for getResponseContentType and adjust isJsonResponse logic
accordingly (referencing getResponseContentType and isJsonResponse).
🪄 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: f8861397-f267-483d-bacb-b384492cc1ff
📒 Files selected for processing (4)
docs/production-incident-baseline.mdpackages/mobile/app/_layout.tsxpackages/mobile/lib/trpc-fetch.test.tspackages/mobile/lib/trpc-fetch.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 4 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Auto-approved: This PR adds a small fetch wrapper that detects non-JSON tRPC responses and throws a diagnostic error, along with corresponding tests and incident documentation, all of which are isolated to the mobile app's network layer with no changes to core business logic, security, or infrastructure.
Re-trigger cubic
Asherlc
changed the title
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Auto-approved: This PR adds a well-tested tRPC fetch wrapper that only throws on non-JSON responses (passing JSON through unchanged), improving diagnostic capability for mobile uploads without altering core business logic or infrastructure.
Re-trigger cubic
6 tasks
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.
Adds a mobile tRPC fetch wrapper that fails before JSON parsing when the server returns a non-JSON response. The thrown error includes the tRPC path, HTTP status, and a short body preview so Sentry can show the real upstream failure for WHOOP BLE background uploads. Documents the production incident baseline and current remaining risk.
Summary by Sourcery
Add a mobile tRPC fetch wrapper that surfaces non-JSON responses for WHOOP BLE uploads and document the related production incident and diagnostic mitigation.
New Features:
Enhancements:
Documentation:
Summary by cubic
Adds a mobile tRPC fetch wrapper that fails early on non-JSON responses and throws a clear error with the tRPC path, HTTP status, content type, and a short body preview. This lets Sentry show the real upstream error during WHOOP BLE background uploads.
createTrpcFetchinpackages/mobile/lib/trpc-fetch.ts; JSON responses pass through unchanged.content-type, and keeps diagnostics if the body preview can’t be read.packages/mobile/app/_layout.tsx, with unit tests inpackages/mobile/lib/trpc-fetch.test.ts, and incident details indocs/production-incident-baseline.md.Written for commit 5725970. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Documentation