Report every handled mobile error to Sentry - #1896
Conversation
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
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? |
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
Warning Review limit reached
Next review available in: 31 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (13)
📝 WalkthroughWalkthroughThe mobile package now reports handled unexpected errors through structured telemetry, preserves explicit cancellation and optional-haptic behavior, migrates WHOOP sync paths to the local helper, and adds an AST-based lint policy with tests. ChangesMobile telemetry policy
Estimated code review effort: 4 (Complex) | ~45 minutes Assessment against linked issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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 |
|
Overall, great PR! Standardizing error handling and routing exceptions through the centralized I've left one small inline suggestion in 🤖 Reviewed by codereviewbot.ai - Catch bugs before your team does. |
Mobile PreviewScan to open on device:
To test on device:
|
![codereviewbot-ai[bot]](https://avatars.githubusercontent.com/in/309638?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/mobile/lib/inertial-measurement-unit-service.ts (1)
117-123: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse a distinct source for WHOOP streaming-start failures.
This catch covers both
findAndConnect()andstartStreaming(), but every failure is taggedactivity-recording-whoop-connect. A successful connection followed by a failed stream start will be misclassified. Split the handlers or use a source describing the complete setup, and add a regression test forstartStreaming()rejection.🤖 Prompt for 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. In `@packages/mobile/lib/inertial-measurement-unit-service.ts` around lines 117 - 123, Update the WHOOP setup flow around findAndConnect and startStreaming so startStreaming rejection is captured with a distinct streaming-start source instead of activity-recording-whoop-connect; split the try/catch handlers while preserving connection failure classification, and add a regression test covering startStreaming rejection.
🤖 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/app/imu-visualization.tsx`:
- Around line 84-87: Add representative IMU visualization tests covering both
native streaming/connection error catch paths around the start-streaming and
connection handling logic. Mock the native failures, verify errors are caught
and consumed, and assert captureException is called with the stable sources
"imu-visualization-start-streaming" and the corresponding connection source.
In `@packages/mobile/app/providers/auth-modals.test.tsx`:
- Around line 1-59: Extend the auth-modals test suite beyond CredentialAuthModal
to cover GarminAuthModal sign-in telemetry and WhoopAuthModal sign-in and
verification telemetry. Mock each relevant mutation, trigger the corresponding
UI flows, and assert mockCaptureException receives the original error with the
expected source and providerId metadata while preserving the actionable error
message.
In `@packages/mobile/lib/background-watch-inertial-measurement-unit-sync.ts`:
- Around line 112-113: Update the shared telemetry helper in
packages/mobile/lib/telemetry.ts so the context’s source value is sent to Sentry
as a tag rather than only as extra data; this root-cause fix should cover the
source callers in
packages/mobile/lib/background-watch-inertial-measurement-unit-sync.ts
(112-113), packages/mobile/lib/background-whoop-ble-sync.ts (153-157, 170-174,
210-214, 236-240, 285-291, 308-313, 475-480),
packages/mobile/lib/useWhoopBleSync.ts (30-30), and
packages/mobile/lib/trpc-fetch.ts (28-32, 45-47), which require no direct
changes.
In `@packages/mobile/lib/useHaptic.ts`:
- Around line 14-23: Update performOptionalHaptic in
packages/mobile/lib/useHaptic.ts:14-23 to identify and suppress only the
documented unavailable-haptics error, call captureException() for every
unexpected rejection, and preserve the unavailable result for the documented
case. Update packages/mobile/lib/useHaptic.test.ts:47-57 to retain coverage for
the documented unavailable condition and add coverage asserting unexpected
rejections trigger telemetry.
In `@scripts/mobile-catch-telemetry-policy.ts`:
- Around line 13-37: Update containsCanonicalCaptureOrThrow to evaluate
reachability rather than merely finding any throw or captureException
descendant. Ensure unreachable branches such as if (false) do not satisfy the
policy, while reporting or propagating the actual catch-path error still does;
add a regression test in mobile-catch-telemetry-policy.test.ts covering an
unreachable throw alongside a swallowing path.
- Around line 39-108: Update hasOptionalHapticResultReturnType to resolve and
validate the referenced OptionalHapticResult type in the current source context
instead of matching only its annotation text. Confirm the local alias represents
the expected union from useHaptic.ts: a performed status variant and an
unavailable status variant carrying cause; return false for same-named types
with any other shape, while preserving the existing exemption flow in
isExplicitOptionalHapticUnavailableResult.
---
Outside diff comments:
In `@packages/mobile/lib/inertial-measurement-unit-service.ts`:
- Around line 117-123: Update the WHOOP setup flow around findAndConnect and
startStreaming so startStreaming rejection is captured with a distinct
streaming-start source instead of activity-recording-whoop-connect; split the
try/catch handlers while preserving connection failure classification, and add a
regression test covering startStreaming rejection.
🪄 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: 1fff4b27-63ad-4e1b-9a7d-f38a149366e7
📒 Files selected for processing (31)
package.jsonpackages/mobile/app/ble-probe.test.tsxpackages/mobile/app/ble-probe.tsxpackages/mobile/app/heart-rate-visualization.test.tsxpackages/mobile/app/heart-rate-visualization.tsxpackages/mobile/app/imu-visualization.tsxpackages/mobile/app/login.test.tsxpackages/mobile/app/login.tsxpackages/mobile/app/providers/auth-modals.test.tsxpackages/mobile/app/providers/auth-modals.tsxpackages/mobile/components/DataExportSection.tsxpackages/mobile/lib/apple-health-provider.tspackages/mobile/lib/auth.test.tspackages/mobile/lib/auth.tspackages/mobile/lib/background-watch-inertial-measurement-unit-sync.tspackages/mobile/lib/background-whoop-ble-sync.test.tspackages/mobile/lib/background-whoop-ble-sync.tspackages/mobile/lib/health-kit-sync.test.tspackages/mobile/lib/health-kit-sync.tspackages/mobile/lib/inertial-measurement-unit-service.test.tspackages/mobile/lib/inertial-measurement-unit-service.tspackages/mobile/lib/open-external-url.test.tspackages/mobile/lib/open-external-url.tspackages/mobile/lib/trpc-fetch.tspackages/mobile/lib/useHaptic.test.tspackages/mobile/lib/useHaptic.tspackages/mobile/lib/useWhoopBleSync.tspackages/mobile/package.jsonpackages/mobile/test-setup.tsscripts/mobile-catch-telemetry-policy.test.tsscripts/mobile-catch-telemetry-policy.ts
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
|
Fixed the outside-diff WHOOP streaming finding from #1896 (review).
|
|
Storybook previews for This comment updates automatically on each PR push. |
Summary
Validation
pnpm lintpnpm tsc --noEmitcd packages/server && pnpm tsc --noEmitcd packages/web && pnpm tsc --noEmitpnpm test(765 files, 13,242 tests)Closes #1795