[fadi-george]

Description

One Line Summary

Release non-OneSignal FCM broadcasts immediately instead of holding them for the 8s deadline.

Details

Motivation

Fixes #2831. Since 5.11.0, an FCM data message without a custom key makes getOSNotificationIdFromJson pass null into JSONObject, which throws a NullPointerException on Android. The helper only catches JSONException, so suspendifyOnIngress swallows the NPE and skips onSuccess, and FCMBroadcastReceiver holds the ordered broadcast until the 8s BroadcastCompletion deadline. Apps with their own FirebaseMessagingService (e.g. VoIP call pushes) receive their messages 8s late. 5.10.2 avoided this with a null-safe isOneSignalBundle check that #2712 removed.

Scope

• getOSNotificationIdFromJsonString returns null for a null string, so persistFcm returns false and the receiver finishes right away without aborting.
• Also covers the other callers of the helper (notification generation, lifecycle service).
• Not changed: the policy from fix: [SDK-4995] durable ingress lane for background notification receivers #2712 that a failed handoff stays open until the deadline. A missing custom key is no longer a failure, so that policy doesn't apply.

Testing

Unit testing

• NotificationFormatHelperTests: id from custom, missing custom, non-JSON custom, isOneSignalBundle without custom.
• NotificationIngressTests: FCM input without a OneSignal payload is rejected without journaling or scheduling.
• Both missing-custom tests fail with the reported NPE without the fix.

Manual testing

Not tested on device. The change is a null guard covered by unit tests.

Affected code checklist

• Notifications
  • Display
  • Open
  • Push Processing
  • Confirm Deliveries
• Outcomes
• Sessions
• In-App Messaging
• REST API requests
• Public API changes

Checklist

Overview

• I have filled out all REQUIRED sections above
• PR does one thing
• Any Public API changes are explained in the PR details and conform to existing APIs

Testing

• I have included test coverage for these changes, or explained why they are not needed
• All automated tests pass, or I explained why that is not possible
• I have personally tested this on my device, or explained why that is not possible

Final pass

• Code is as readable as possible.
• I have reviewed this PR myself, ensuring it meets each checklist item

🤖 Generated with Claude Code

[github-actions]

📊 Diff Coverage Report

Diff Coverage Report (Changed Lines Only)

Gate: aggregate coverage on changed executable lines must be ≥ 80% (JaCoCo line data for lines touched in the diff).

Changed Files Coverage

• NotificationFormatHelper.kt: 3/3 touched executable lines (100.0%) (4 touched lines in diff)

Overall (aggregate gate)

3/3 touched executable lines covered (100.0% — requires ≥ 80%)

📥 View workflow run

[abdulraqeeb33]

approve-with-nits

The null guard is the right place, and the missing-custom path now finishes the ordered broadcast instead of waiting out the deadline.

[abdulraqeeb33]

The debug log is in 6bf7920. A missing custom field now finishes the broadcast and leaves a debug line.

Approved.