Skip to content

Conversation

@jkasten2
Copy link
Member

@jkasten2 jkasten2 commented Nov 18, 2025

Description

One Line Summary

Revert throwing if things wait on init take too long that was introduced in 5.4.0.

Details

Starting in 5.4.0 calls such as OneSignal.Notification or OneSignal.Location would throw if they waited on the SDK into to long. This PR changes this to wait forever until init is complete, and log instead if the time is taking longer than expected. If these calls were done from the main thread an ANR is the lesser of two evils and the app can recover, where an uncaught throw it can not.

This commit will bring the behavior similar to 5.1.x. The only difference is you will now never see an ANR with initWithContext, however if you call other parts of the SDK from the main thread, such as OneSignal.Notification, you will continue to see ANRs, just at a different stacktrace. To avoid these ANRs completely call all other SDK methods outside of the main thread.

Related Issues

Motivation

A behavior change wasn't intended in a non-major release.

Scope

Removes throwing and timeout state when things are waiting on SDK initialization.

Testing

Unit testing

Updated existing unit tests to expect waiting without a timeout.

Manual testing

Tested on an Android 14 emulator. Added an artificial 10 second delay with Thread.Sleep just before latch.countDown() to ensure the SDK still initializes correctly.

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
    • If it is hard to explain how any codes changes are related to each other then it most likely needs to be more than one PR
  • 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.
    • Simplify with less code, followed by splitting up code into well named functions and variables, followed by adding comments to the code.
  • I have reviewed this PR myself, ensuring it meets each checklist item
    • WIP (Work In Progress) is ok, but explain what is still in progress and what you would like feedback on. Start the PR title with "WIP" to indicate this.

This change is Reviewable

Starting in 5.4.0 calls such as OneSignal.Notification or
OneSignal.Location would throw if they waited on the SDK into to long.
This comment changes this to wait forever until init is complete, and
log instead if the time is taking longer than expected.
If these calls were done from the main thread an ANR is the better of
two evils and the app can recover, where an uncaught throw it can not.

This commit will bring the behavior the similar behavior in 5.1.x.
The only difference is you will now never see an ANR with
initWithContext, however if you call other parts of the SDK from the
main thread, such as OneSignal.Notification, you will continue to see
ANRs, just at a different stacktrace. To avoid these ANRs completely
call all other SDK methods outside of the main thread.
@jkasten2 jkasten2 force-pushed the fix/throwing_init_timeout branch from c386c85 to 00d3380 Compare November 18, 2025 01:27
@alberto-derodrigo-spacego

A comment regarding this, at the SDK level, it's difficult to know that something as simple as Notifications.addClickListener(this) needs to be launched on a different thread than the main one; ultimately, the complexity is transferred to the developer. For example, this code also triggered the error:

initWithContext(context, ONESIGNAL_APP_ID)
if (OneSignal.isInitialized) {
    Notifications.addClickListener(this)
}

Given my lack of context when using the SDK, I initially assume that adding a listener shouldn't require the SDK to be initialized, nor should it need to be done on a non-main thread. And I least expected a failure when there was confirmation that it was initialized.

I understand that the fix solves a problem but not the cause. In my opinion, the SDK should either provide an initialization event, perhaps with a listener or something similar, to indicate when it's a good time to subscribe to things. And if certain operations must be done off the main thread, that should be the SDK's default behavior internally, providing options for advanced users to change said behavior.

Having said all this, perhaps none of this is necessary if the documentation is supplemented with examples of how to work better with the SDK following these changes.

In any case, thank you for working on the fix and constantly improving the SDK.

@michael-winkler
Copy link

Starting in 5.4.0 calls such as OneSignal.Notification or OneSignal.Location would throw if they waited on the SDK into to long. This comment changes this to wait forever until init is complete, and log instead if the time is taking longer than expected. If these calls were done from the main thread an ANR is the better of two evils and the app can recover, where an uncaught throw it can not.

This commit will bring the behavior the similar behavior in 5.1.x. The only difference is you will now never see an ANR with initWithContext, however if you call other parts of the SDK from the main thread, such as OneSignal.Notification, you will continue to see ANRs, just at a different stacktrace. To avoid these ANRs completely call all other SDK methods outside of the main thread.

Please mark these functions with suspend so that we, as developers, can always see which functions need to be called as suspend functions.

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.

4 participants