fix(android): replay a notification tap that lands before setup - #579
Open
turnipdabeets wants to merge 3 commits into
Open
fix(android): replay a notification tap that lands before setup#579turnipdabeets wants to merge 3 commits into
turnipdabeets wants to merge 3 commits into
Conversation
A tray tap delivered to Activity.onNewIntent before Dart reaches Posthog().setup() is dropped by PostHogAndroid, because the SDK is not set up yet. Android does not put that intent on Activity.getIntent() either, so the later read at setup finds the stale launch intent. The event survives today only because firebase_messaging's own NewIntentListener calls mainActivity.setIntent(intent) first. Any app whose FCM layer does not do that loses the tap. Remember the tap in the listener and prefer it at setup. Only an intent carrying google.message_id is kept, at most one, cleared on consume and on activity detach. PostHogAndroid dedupes by google.message_id, so the AUTO_INIT path — where the listener already captures — cannot double-count.
Contributor
posthog-flutter Compliance ReportDate: 2026-09-12 19:56:05 UTC ✅ All Tests Passed!45/45 tests passed Capture Tests✅ 29/29 tests passed View Details
Feature_Flags Tests✅ 16/16 tests passed View Details
|
turnipdabeets
marked this pull request as ready for review
September 12, 2026 16:06
This was referenced Sep 12, 2026
Open
Prompt To Fix All With AI### Issue 1
posthog_flutter/android/src/test/kotlin/com/posthog/flutter/PosthogFlutterPluginTest.kt:505-507
**Replay target is untested**
This test only verifies that `pendingPushIntent` is cleared. It does not verify that the remembered intent, rather than the stale Activity intent, reaches native capture. The core regression would therefore pass if the implementation cleared the pending intent without replaying it. Add a capture seam and assert both pending-intent precedence and the Activity-intent fallback.
### Issue 2
posthog_flutter/android/src/main/kotlin/com/posthog/flutter/PosthogFlutterPlugin.kt:856
**Fatal errors are suppressed**
Catching `Throwable` also suppresses fatal errors such as `OutOfMemoryError` and `LinkageError`, although the documented malformed-extras case only requires handling `BadParcelableException`. Catch that expected exception so unrecoverable failures are not misreported as unreadable notification extras.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "fix(android): replay a notification tap ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+505
to
+507
| plugin.capturePushNotificationOpenedFromLaunchIntent() | ||
|
|
||
| assertNull(plugin.pendingPushIntent) |
There was a problem hiding this comment.
This test only verifies that pendingPushIntent is cleared. It does not verify that the remembered intent, rather than the stale Activity intent, reaches native capture. The core regression would therefore pass if the implementation cleared the pending intent without replaying it. Add a capture seam and assert both pending-intent precedence and the Activity-intent fallback.
Prompt To Fix With AI
This is a comment left during a code review.
Path: posthog_flutter/android/src/test/kotlin/com/posthog/flutter/PosthogFlutterPluginTest.kt
Line: 505-507
Comment:
**Replay target is untested**
This test only verifies that `pendingPushIntent` is cleared. It does not verify that the remembered intent, rather than the stale Activity intent, reaches native capture. The core regression would therefore pass if the implementation cleared the pending intent without replaying it. Add a capture seam and assert both pending-intent precedence and the Activity-intent fallback.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| if (intent?.getStringExtra(GOOGLE_MESSAGE_ID) != null) { | ||
| pendingPushIntent = intent | ||
| } | ||
| } catch (e: Throwable) { |
There was a problem hiding this comment.
Catching Throwable also suppresses fatal errors such as OutOfMemoryError and LinkageError, although the documented malformed-extras case only requires handling BadParcelableException. Catch that expected exception so unrecoverable failures are not misreported as unreadable notification extras.
Prompt To Fix With AI
This is a comment left during a code review.
Path: posthog_flutter/android/src/main/kotlin/com/posthog/flutter/PosthogFlutterPlugin.kt
Line: 856
Comment:
**Fatal errors are suppressed**
Catching `Throwable` also suppresses fatal errors such as `OutOfMemoryError` and `LinkageError`, although the documented malformed-extras case only requires handling `BadParcelableException`. Catch that expected exception so unrecoverable failures are not misreported as unreadable notification extras.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.The clear-on-detach comment claimed a second engine could inherit the tap; pendingPushIntent is a plugin instance field, so it cannot. Name the real cost instead: a configuration change between the tap and setup() loses the event. Also record that the tray-tap filter guards the activity-intent fallback, not just memory, and move the FCM root cause out of the changeset.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
💡 Motivation and Context
On Android,
$push_notification_openedfor a tap that arrives beforePosthog().setup()runs is captured today only by accident — it depends onfirebase_messagingdoing us a favour.The scenario (measured on a device):
am killthe process — the task stays in recents.With
com.posthog.posthog.AUTO_INITdisabled (what our own example ships), the plugin'sPluginRegistry.NewIntentListenerfires before Dart reachesPosthog().setup(), soPostHogAndroid.capturePushNotificationOpened(intent)no-ops — the SDK isn't set up yet. Android does not put that intent onActivity.getIntent()either, so the later read at setup finds the stale launch intent.The event still lands today only because
firebase_messaging's ownNewIntentListenerruns first and callsmainActivity.setIntent(intent)(FlutterFirebaseMessagingPlugin.handleNotificationIntent), which makes our lateractivity?.intentread find the push intent. Suppressing that onesetIntentcall drops the captured count to 0.So any app whose FCM layer doesn't refresh the intent silently loses the tap: a hand-rolled
FirebaseMessagingService, another push wrapper, orfirebase_messagingitself when the message isn't in its store.The fix: the listener remembers the tap, and setup prefers it over
activity?.intent. Only an intent carryinggoogle.message_idis kept, at most one, cleared on consume and on activity detach. The listener still callsPostHogAndroid.capturePushNotificationOpened(intent)exactly as before, so the AUTO_INIT-enabled path is untouched — andPostHogAndroiddedupes bygoogle.message_id, so the two calls can't double-count.Related:
capturePushNotificationOpened(intent)entry point this uses.main.💚 How did you test it?
Kotlin unit tests (
PosthogFlutterPluginTest, all 42 green via./gradlew :posthog_flutter:testDebugUnitTest): a tray tap is remembered, a non-push intent isn't, a second tap supersedes the first, an intent whose extras throwBadParcelableExceptionis skipped without throwing, the replay consumes the remembered tap once, and detach drops it.Also green:
flutter test(351),dart analyze,dart format, ktlint, and the Dart public API snapshot.On an emulator (API 34,
google_apis), Flutter example app + realfirebase_messaging, a real tray tap viauiautomator, counting$push_notification_openedin the/batchpayloads posted to a local mock server.setIntentwas suppressed with a temporary, uncommitted override in the example'sMainActivity.setIntentAUTO_INITNo double counting anywhere: each run logged exactly one
Queued Event $push_notification_opened. Also checked on the fixed build — an ordinary cold start (app not running, tap launches it): 1; a warm tap (process alive,onNewIntent): 1.Falsification: with only the replay removed (
pendingPushIntent ?: activity?.intent→activity?.intent, listener still remembering, detach still clearing), the third row goes back to 0.📝 Checklist
If releasing new changes
pnpm changesetto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Written with Claude Code. The bug was already localised in a prior investigation; this PR implements the agreed fix and re-confirms it on hardware.
Decisions along the way:
setIntentourselves. Writing to the host Activity's intent is a side effect other plugins observe, and it would make us the second plugin racing over the same field. Keeping our own reference changes nothing outside the plugin.google.message_idare kept, so we never hold an arbitrary intent, and at most one — anIntentretains its extras, not the Activity.setup(); standard Flutter activities declareconfigChangesfor orientation, so that path is rare, and losing an event is the safer failure direction than mis-attributing one.try/catch— unmarshalling the Bundle throwsBadParcelableExceptionwhen it carries a class this app can't load, which the native SDK already guards against on its own path.Related PRs
One push-open capture effort across the mobile SDKs: count every notification tap exactly once, and stop losing taps the SDK starts too late to see.
firebase_messagingMerge order: posthog-android#783 and posthog-ios#828 first, then their releases. #4921, #4929 and #579 are independent and can go any time. #4919 and #578 go green once posthog-android 3.65.0 is published. Docs: #20114 can go now; #20102 last, after the releases.
Earlier work this builds on: PostHog/posthog-android#753, PostHog/posthog-ios#792, PostHog/posthog-js#4858, #556, #557, PostHog/posthog.com#19905.