Skip to content

Conversation

@ArielDemarco
Copy link
Collaborator

Overview

This PR fixes #113, where push notifications were sometimes not reaching the app when users did not implement UNUserNotificationCenterDelegate methods.

Description

We’ve introduced conditional logic for proxying UNUserNotificationCenterDelegate methods:

  • The SDK now only responds to a selector if the original delegate also implements it.
  • This prevents scenarios where the SDK intercepts notifications without forwarding them, ensuring they always reach the app’s implementation when it exists (or the user).

To account for cases where users don't implement UNUserNotificationCenterDelegate methods, we have added a fallback swizzling of UIApplicationDelegate.application(_:didReceiveRemoteNotification:fetchCompletionHandler:). This ensures that notifications are still captured by the SDK, even when no delegate is explicitly handling them.

Warning

This does not handle cases where an app receives push notifications but does not implement any handling logic (e.g., neither UNUserNotificationCenterDelegate methods nor UIApplicationDelegate.application(_:didReceiveRemoteNotification:fetchCompletionHandler:)). In such scenarios, push notifications will still be received by iOS but we'll not be able to capture them.

@github-actions
Copy link

github-actions bot commented Jan 29, 2025

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

OpenSSF Scorecard

PackageVersionScoreDetails

Scanned Files

@github-actions
Copy link

github-actions bot commented Jan 29, 2025

Warnings
⚠️ No CHANGELOG entry added.
⚠️ No tests added / modified.

Generated by 🚫 Danger Swift against ce705ae

@codecov
Copy link

codecov bot commented Jan 29, 2025

Codecov Report

❌ Patch coverage is 0% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.61%. Comparing base (97c27c4) to head (ce705ae).
⚠️ Report is 137 commits behind head on main.

Files with missing lines Patch % Lines
...Notifications/PushNotificationCaptureService.swift 0.00% 23 Missing ⚠️
...ations/UNUserNotificationCenterDelegateProxy.swift 0.00% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #168      +/-   ##
==========================================
- Coverage   89.69%   89.61%   -0.08%     
==========================================
  Files         439      439              
  Lines       30098    30123      +25     
==========================================
- Hits        26995    26994       -1     
- Misses       3103     3129      +26     
Files with missing lines Coverage Δ
...tions/PushNotificationCaptureService+Options.swift 50.00% <ø> (ø)
...ces/EmbraceIO/Capture/CaptureService+Helpers.swift 100.00% <ø> (ø)
...ations/UNUserNotificationCenterDelegateProxy.swift 4.54% <0.00%> (-0.15%) ⬇️
...Notifications/PushNotificationCaptureService.swift 12.32% <0.00%> (-5.68%) ⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Data loss issue with Push Notification Capture service

2 participants