Skip to content

fix(ios): stop hijacking UNUserNotificationCenter.delegate - #3

Open
mr-vishwas wants to merge 1 commit into
mainfrom
fix/ios-do-not-hijack-notification-delegate
Open

fix(ios): stop hijacking UNUserNotificationCenter.delegate#3
mr-vishwas wants to merge 1 commit into
mainfrom
fix/ios-do-not-hijack-notification-delegate

Conversation

@mr-vishwas

Copy link
Copy Markdown
Collaborator

Problem

NotificationManager (iosMain) installed a NotificationDelegate wrapper on UNUserNotificationCenter.delegate so it could present the library notification while the host app is in the foreground:

private val notificationCenter = UNUserNotificationCenter.currentNotificationCenter().apply {
    setDelegate(NotificationDelegate(appNotificationDelegate = delegate()))
}

UNUserNotificationCenter.delegate is a weak property, and the wrapper was not retained anywhere. Kotlin/Native collected it shortly after the first monitored request, so the delegate silently became null.

From that point the host app received no notification callbacks at all:

  • push notifications were no longer presented while the app was in the foreground;
  • taps no longer reached userNotificationCenter(_:didReceive:withCompletionHandler:), so push deep links stopped working entirely.

Because the trigger is the first network call rather than anything notification-related, this is very hard to trace back to a network debugging library. It cost us a full day of investigation in a production app.

Why not just retain the wrapper

Retaining it fixes the null delegate, but the delegate belongs to the host app. Taking it over means competing with whatever the app and its other SDKs (Firebase, MoEngage, …) install there, in an order the library cannot control — and the forwarding chain only works if every participant plays along.

Change

The library no longer touches UNUserNotificationCenter.delegate, and NotificationDelegate is deleted.

The notification is still posted and still appears in Notification Center. It is no longer presented as a banner while the app is in the foreground, and tapping it no longer routes through the library. On iOS the monitor UI is opened from the host app anyway (KtorMonitorViewController), so the delegate bought very little.

Version bumped to 1.14.2.

Verification

Verified on iOS 26.4 Simulator in an app that uses FCM push with deep links:

  • before: UNUserNotificationCenter.current().delegate was the library's NotificationDelegate and push taps produced no callback in the app;
  • after: the delegate stays the app's own, willPresent and didReceive both fire, and the push payload arrives intact.

Android is unaffected by this change but should be sanity-checked before merge, since NotificationManager is a shared expect/actual and only the iOS actual changed.

NotificationManager installed a NotificationDelegate wrapper on
UNUserNotificationCenter.delegate to present the library notification while the
host app is in the foreground. That property is weak and the wrapper was not
retained anywhere, so Kotlin/Native collected it shortly after the first
monitored request and the delegate silently became null. From that point the
host app received no notification callbacks at all: push notifications stopped
being presented in the foreground, and taps no longer reached
userNotificationCenter(_:didReceive:withCompletionHandler:), which broke push
deep links in a way that is very hard to trace back to a network debugging
library.

Retaining the wrapper fixes the null delegate but not the underlying problem:
the delegate belongs to the host app, and taking it over means competing with
whatever the app and its other SDKs install there, in an order the library
cannot control.

The library no longer touches the delegate, and NotificationDelegate is
removed. The notification is still posted and still appears in Notification
Center; it is simply no longer presented as a banner while the app is in the
foreground, and tapping it no longer routes through the library.
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.

2 participants