Skip to content

Conversation

nan-li
Copy link
Contributor

@nan-li nan-li commented Aug 13, 2025

Description

One Line Summary

Fix null pointer exception when PermissionsActivity is being created.

Details

  • This should never happen as the SDK does send extras for this Activity, and no other entity should be creating PermissionsActivity.
  • However, we have seen crash reports from our users that primarily seems to affect Nexus 5 and Pixel 3, in low numbers.
  • One theory is that automatic testing done by the google play store is explicitly driving the PermissionsActivity.

Motivation

Scope

  • This appears to be a crash in very low numbers. If we detect extras is null, then we return early, which is safe to do as it doesn't make sense to show the Activity anyway.

Testing

Unit testing

None

Manual testing

None, unable to repro

From Josh:

The only odd case I can think of is if the end-user backgrounds the app when they see the permission prompt. Then if the app is kicked out of memory and the end-user decides to come back to the app the Activity will be recreated. This is not a flow we normally test, but I just did so now and not seeing any issues.

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

Testing

  • I have included test coverage for these changes, or explained why they are not needed
  • 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

This change is Reviewable

* This should never happen as the SDK does send extras for this Activity, and no other entity should be creating PermissionsActivity.
* However, we have seen crash reports from our users that primarily seems to affect Nexus 5 and Pixel 3, in low numbers.
* One theory is that automatic testing done by the google play store is explicitly driving the PermissionsActivity.

if (intent.extras == null) {
// This should never happen, but extras is null in rare crash reports
return
Copy link
Contributor

Choose a reason for hiding this comment

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

Returning early keeps the Activity alive (it will still hit onStart/onResume), leaves it on the back stack, and can cause a blank/flickery frame. Even though this was most likely triggered by tests passing null extras, we should harden production code.

I would recommend reusing the existing exit path at line 49 so behavior is deterministic.

Copy link
Contributor Author

@nan-li nan-li Aug 13, 2025

Choose a reason for hiding this comment

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

This is because we already called super.onCreate() so we need to call finish()?
Should the initWithContext check do that as well? I think it is more common for initWithContext to return false, than for the extras to be null?

Copy link
Contributor

Choose a reason for hiding this comment

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

In practice, I suspect the initWithContext check never fail: showing the permission prompt requires OneSignal.getNotification() or OneSignal.getLocation(), and if initWithContext wasn’t invoked earlier, the “must call initWithContext before use” exception would already be thrown.

For the case of the automated tests, initializing OneSignal by calling initWithContext in onCreate() here seems to work, so was continued to handleBundleParams().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's call finish on all early returns, updated the PR.

* Prevent flicker or blocking, finish activity before returning early
@nan-li nan-li merged commit f84f0e4 into main Aug 14, 2025
2 checks passed
@nan-li nan-li deleted the fix/PermissionsActivity_rare_crash branch August 14, 2025 19:41
@jinliu9508 jinliu9508 mentioned this pull request Aug 14, 2025
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.

3 participants