Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| await implementation.didReceive(request, withContentHandler: contentHandler) | ||
| } | ||
|
|
||
| return true |
There was a problem hiding this comment.
Bug: SDK Notification Handling Breaks API Contract
The public didReceive method now always returns true and dispatches its work to an unawaited Task. This breaks the API contract where the return value indicates if the SDK handled the notification, potentially causing Notification Service Extensions to terminate before completing push processing.
Additional Locations (1)
| pushLogger.logTrackingPushMessageDelivered(deliveryId: pushCioDeliveryInfo.id) | ||
|
|
||
| trackMetricFromNSE(deliveryID: pushCioDeliveryInfo.id, event: .delivered, deviceToken: pushCioDeliveryInfo.token) | ||
| await trackMetricFromNSE(deliveryID: pushCioDeliveryInfo.id, event: .delivered, deviceToken: pushCioDeliveryInfo.token) |
There was a problem hiding this comment.
Bug: Async Refactor Breaks Parallelism, Affects Performance
The async/await refactor changed previously parallel operations, like metric tracking and rich push processing, into serial execution. This goes against the explicit comment in the code and may cause a performance regression in notification service extension processing.
| ) | ||
|
|
||
| MessagingPush.shared.didReceive(request, withContentHandler: contentHandler) | ||
| Task { await MessagingPush.shared.didReceive(request, withContentHandler: contentHandler) } |
There was a problem hiding this comment.
Bug: Async Task Not Awaited in Notification Extension
The NotificationServiceExtension creates a Task for rich push processing but doesn't await its completion. Given the extension's limited execution time, this can lead to premature termination before the async work finishes, resulting in incomplete notification processing.
Shahroz16
left a comment
Shahroz16
left a comment
There was a problem hiding this comment.
can we put this in draft?
mahmoud-elmorabea
changed the title
mahmoud-elmorabea
changed the title
2 participants
TBA
Note
Adopts Swift 5.9 with strict concurrency across the SDK, refactors networking and rich push to async/await, enforces Sendable/@mainactor, improves DI/thread-safety, and bumps minimum iOS to 15 with updated podspecs/projects/mocks/tests.
SWIFT_STRICT_CONCURRENCY=complete) and upgrade to Swift 5.9 across projects; SPM tools to 5.9.Sendable,@MainActor,@preconcurrency, and@Sendableannotations throughout.HttpClient/HttpRequestRunnerand rich push flows to async/await; update call sites and mocks.@Sendable; many methods nowasync(e.g., rich pushdidReceive, metric tracking).EnhancedSynchronizedfor safe shared state; reworkDIGraphSharedto thread-safe singleton creation/storage.@MainActorwhere needed.TaskBag; adjust Atomic internals.15.0; update all Cocoapods podspecs to Swift 5.9 and iOS 15.Written by Cursor Bugbot for commit d5c1ee7. This will update automatically on new commits. Configure here.