Skip to content

fix: [SDK-4722] guard getNotifications() in onDetachedFromEngine (#1149)#1150

Merged
fadi-george merged 2 commits into
mainfrom
fadi/sdk-4722
May 28, 2026
Merged

fix: [SDK-4722] guard getNotifications() in onDetachedFromEngine (#1149)#1150
fadi-george merged 2 commits into
mainfrom
fadi/sdk-4722

Conversation

@fadi-george
Copy link
Copy Markdown
Collaborator

Description

One Line Summary

Guard OneSignal.getNotifications() in onDetachedFromEngine so the Flutter plugin no longer crashes on Android when the engine detaches before OneSignal.initialize() ran.

Details

Motivation

Fixes #1149. On Android, when the app is closed (Activity#onDestroyFlutterPlugin#onDetachedFromEngine) without OneSignal.initialize() having been called from Dart, the plugin's OneSignalNotifications.onDetachedFromEngine() calls OneSignal.getNotifications().removeClickListener(this). The native SDK throws:

java.lang.IllegalStateException: Must call 'initWithContext' before use
    at com.onesignal.OneSignal.getNotifications(OneSignal.kt)
    at com.onesignal.flutter.OneSignalNotifications.onDetachedFromEngine(...)

This happens for users who include the plugin via pubspec.yaml but defer or skip OneSignal.initialize() (cold start, conditional init, fast finish, etc.). The plugin still auto-attaches to the engine, so detach still fires.

Scope

  • Only adds an OneSignal.isInitialized() guard at the top of OneSignalNotifications.onDetachedFromEngine().
  • When the SDK is initialized, behavior is unchanged: the click listener is still removed so clicks while the engine is dead get queued by the native SDK instead of dispatched on a detached channel.
  • When the SDK is not initialized, the method becomes a no-op (there is no registered listener to remove anyway).
  • No public API changes. No Dart-side changes. iOS untouched.

Testing

Manual testing

Reproduced and verified on a Pixel emulator (Android 16) using a stripped-down repro app on branch repro/issue-1149-android-on-detach-crash (no OneSignal.initialize() call, a button that calls Activity#finish() via a native MethodChannel).

Steps:

  1. adb uninstall com.onesignal.example (so the native SyncJobService from a prior install does not auto-call initWithContext in the background and mask the bug).
  2. flutter run from examples/demo.
  3. Tap the "Finish activity" button.
  4. Observe adb logcat.

Results:

  • Before fix (repro/issue-1149-android-on-detach-crash): FATAL EXCEPTION ... IllegalStateException: Must call 'initWithContext' before use at OneSignalNotifications.onDetachedFromEngine.
  • After fix (repro/issue-1149-fix, same repro app + this patch): MainActivity.onDestroy logs, no exception, process exits cleanly.

Also re-ran the normal demo flow with OneSignal.initialize() enabled to confirm the click listener is still removed on detach and no regression in normal init paths.

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 (no public API changes)

Testing

  • I have included test coverage for these changes, or explained why they are not needed
    • No new unit test: the crash requires a real Android FlutterPlugin#onDetachedFromEngine lifecycle plus the native OneSignal SDK's uninitialized state, which isn't exercisable from the existing Dart test surface. Verified via the repro app described above.
  • 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

Made with Cursor

@fadi-george fadi-george requested a review from a team as a code owner May 27, 2026 23:08
Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The native-side fix looks correct and well-scoped, but this PR also deletes .github/pull_request_template.md entirely — which isn't mentioned in the description and looks unintentional (likely the template content was pulled into the PR body and the file ended up removed). Could you confirm whether that deletion is intentional before merge?

Extended reasoning...

Overview

The functional change is a single guard added at the top of OneSignalNotifications.onDetachedFromEngine() (android/src/main/java/com/onesignal/flutter/OneSignalNotifications.java:231-234): an early return when OneSignal.isInitialized() is false, with a comment pointing to issue #1149. This is a small, targeted Android-only fix for an IllegalStateException thrown by the native SDK when the Flutter engine detaches before OneSignal.initialize() ran. Behavior when initialized is unchanged.

Security risks

None. The change adds a defensive guard around an SDK call and reduces a crash surface; it does not touch auth, crypto, permissions, networking, or data handling.

Level of scrutiny

The Java change itself is low-risk and approve-worthy on its own — it follows the existing pattern (OneSignal.isInitialized() is the documented native check) and the reasoning is correct: if the SDK was never initialized, there cannot be a click listener registered for it to remove, so the early return is safe. The recent commit 05e3d8d fix(android): guard getNotifications() before init suggests the same pattern was already applied elsewhere, so this is consistent.

Other factors

What pushed this to a deferral is the second file change: the PR removes .github/pull_request_template.md entirely. The PR description does not mention this, the title/scope are explicitly about the Android guard, and the deletion looks like an editor/Cursor artifact (the template content appears verbatim inside the PR body, suggesting the author drafted on top of the file and the original got deleted). A human should confirm whether the template removal is intentional — if not, it should be restored in a follow-up commit before merge. No bugs were reported by the bug-hunting pass.

@fadi-george fadi-george merged commit aaa03d8 into main May 28, 2026
5 checks passed
@fadi-george fadi-george deleted the fadi/sdk-4722 branch May 28, 2026 16:53
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.

[Bug]: Android Crash on app destroy: IllegalStateException: Must call 'initWithContext' before use (Regression in OneSignal-Android-SDK 5.9.2)

2 participants