chore: test foreground push notifications#32100
Conversation
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
PR template — items to address before "Ready for review"Blocking — these items fail the workflow until fixed:
Warnings — informational, address before merging:
See docs/readme/ready-for-review.md for the full Definition of Ready for Review. |
| }, | ||
| ]} | ||
| /> | ||
| <PushDebugPanel /> |
There was a problem hiding this comment.
Debug panel ships in production
Medium Severity
PushDebugPanel is always rendered on the notifications screen with no __DEV__, feature flag, or internal-only guard. Production users get a developer push debug strip (Refresh/Clear, internal FCM event types) above the notification list, which matches temporary test tooling rather than end-user UI.
Reviewed by Cursor Bugbot for commit 3c1f9a3. Configure here.
| dataKeys: Object.keys(remoteMessage.data ?? {}), | ||
| }, | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Background handler drops debug writes
Medium Severity
The new setBackgroundMessageHandler calls logPushEvent without await. In headless/background Android contexts the handler promise can finish before AsyncStorage read/write completes, so FCM_BACKGROUND_RECEIVED entries may never persist—the main signal this PR adds for killed/background FCM.
Reviewed by Cursor Bugbot for commit 3c1f9a3. Configure here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4eb4ec4. Configure here.
| logPushEvent( | ||
| 'FCM_FOREGROUND_PROCESSING', | ||
| `Foreground notification parsed: ${notification?.id}`, | ||
| { id: notification?.id, type: (notification as { type?: string })?.type }, |
There was a problem hiding this comment.
Handler runs without notification
Medium Severity
Optional chaining on the foreground debug logs stops a throw when processNotification returns a nullish value. The flow still calls handler with that value afterward, whereas before the log line’s notification.id access threw and skipped the handler inside the existing catch path.
Reviewed by Cursor Bugbot for commit 4eb4ec4. Configure here.
🔍 Smart E2E Test Selection
click to see 🤖 AI reasoning detailsE2E Test Selection:
Why no E2E tags are selected:
The risk is low as these are debug instrumentation additions that don't alter any functional behavior. Performance Test Selection: |
|




Description
Changelog
CHANGELOG entry:
Related issues
Fixes:
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Performance checks (if applicable)
trace()for usage andaddTokenfor an exampleFor performance guidelines and tooling, see the Performance Guide.
Pre-merge reviewer checklist
Note
Medium Risk
Changes FCM bootstrap (
setBackgroundMessageHandlerinindex.js) and core notification handling paths; the visible debug UI may ship to all users unless removed or gated before merge.Overview
Adds local FCM push instrumentation so engineers can trace token registration, foreground handling, background/headless delivery, and notification-open flows while testing pushes.
A new
pushDebugLoghelper persists up to 100 typed events in AsyncStorage vialogPushEvent, with safe no-op error handling.FCMServiceandindex.jsnow emit events at key points (e.g.onMessage, parse/display/drop paths, token creation, opened-from-killed/background). APushDebugPanelon the Notifications screen lists entries with refresh/clear and expandable JSON detail.Note: The debug panel is wired unconditionally into the Notifications view (no
__DEV__gate in this diff).Reviewed by Cursor Bugbot for commit 4eb4ec4. Bugbot is set up for automated code reviews on this repo. Configure here.