Fix mobile native bridge crash - #1226
Conversation
|
Warning Review limit reached
More reviews will be available in 3 minutes and 49 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
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 |
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? |
Automated Checks (advisory, non-blocking)✅ All checks passed. Surmado Code Review — Free tier limit reachedYou've used all 10 free reviews this month. Deterministic checks (secrets, model strings) still ran above. Upgrade to the Paid plan for 100 reviews/month + $15 per additional 100: https://app.surmado.com/checkout?plan=pr_review_starter Or wait until your next monthly window for 10 more free reviews. Surmado Code Review (v1.2-mt) |
|
Storybook previews for This comment updates automatically on each PR push. |
Mobile PreviewScan to open on device:
To test on device:
|
|
Review app deployment was skipped for PR #1226. Hetzner could not allocate the configured review app server type in the configured location. This is provider capacity/placement availability, not a code failure in this PR. |
There was a problem hiding this comment.
1 issue found across 7 files
Confidence score: 3/5
- There is a concrete reliability risk in
packages/mobile/modules/health-kit/ios/HealthKitModule.swift:HKObserverQuerycompletion may run before the main-thread event dispatch, which can cause background HealthKit update events to be missed. - Given the medium severity (6/10) and solid confidence (7/10), this is more than a minor housekeeping issue and could create user-visible data sync gaps.
- This is likely fixable with ordering/synchronization in the observer callback flow, but it adds meaningful merge risk until addressed.
- Pay close attention to
packages/mobile/modules/health-kit/ios/HealthKitModule.swift- ensure observer completion happens only after the event send is safely executed.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…2337 # Conflicts: # docs/production-incident-baseline.md
…2337 # Conflicts: # docs/production-incident-baseline.md
There was a problem hiding this comment.
0 issues found across 3 files (changes from recent commits).
Auto-approved: This PR fixes a production crash by ensuring bridge events are dispatched on the main thread and nil payloads are stripped; the changes are isolated to two iOS modules, include unit tests, and do not affect core business logic.
Re-trigger cubic
Fixes a production mobile crash where WHOOP BLE and HealthKit native events could cross the Expo/React Native bridge from background callback queues. WHOOP connection/orientation events and HealthKit observer events now emit on the main thread, and optional nil payload values are stripped before bridging. The production incident baseline documents Sentry issue DOFEK-MOBILE-P, the root cause evidence, and remaining risk. Validated with pnpm lint, root/server/web typechecks, focused mobile Vitest tests, and Swift package tests for both native modules.
Summary by cubic
Fixes a production crash by forcing native
whoop-bleandhealth-kitevents to emit on the main thread, stripping nil payloads, and acknowledging HealthKit observer queries after emission. PreventsEXC_BAD_ACCESSduring foreground sync when background callbacks crossed the Expo/React Native bridge and avoids re-fired observer updates.MainThreadEventEmitterinwhoop-bleandhealth-kitiOS modules to dispatch on the main thread and drop nil payloads.WhoopBleModule.swift(connect/disconnect/orientation) andHealthKitModule.swift(observer events) to use the helper; HealthKit now calls the observer completion after sending the event; cleaned up SwiftLint closure style.DOFEK-MOBILE-P.Written for commit 9e05a46. Summary will update on new commits.