Skip to content

fix: [SDK-5538] never crash on invalid init or calls before init - #2803

Draft
abdulraqeeb33 wants to merge 1 commit into
never-crash/3-deferred-managersfrom
never-crash/4-never-crash-init
Draft

abdulraqeeb33 wants to merge 1 commit into
never-crash/3-deferred-managersfrom
never-crash/4-never-crash-init

Conversation

@abdulraqeeb33

@abdulraqeeb33 abdulraqeeb33 commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Stack (base feat/never-crash, merged to main as one unit; split out of #2793):

  1. refactor: [SDK-5538] add SdkOperation names for init-gated entry points #2800 SdkOperation names
  2. feat: [SDK-5538] remote kill switch with a placeholder OneSignalImpV2 #2801 Kill switch with a placeholder OneSignalImpV2
  3. feat: [SDK-5538] deferred managers that stand in before init #2802 Deferred managers
  4. fix: [SDK-5538] never crash on invalid init or calls before init #2803 Never-crash init (this PR)

Description

One Line Summary

OneSignalImpV2 becomes the never-crash implementation: an invalid init or a call before init logs instead of crashing the app.

Details

Motivation

SDK-5538 (doc): never crash the app, log instead. Today a null appId throws a Kotlin NullPointerException, an empty appId with nothing cached still starts the SDK, and any accessor or login before init throws. Wrappers (.NET, RN, Flutter, Unity) add their own guards for this; the native SDK should own it.

How to read this diff

OneSignalImpV2.kt starts as a copy of OneSignalImp (PR 2), so its diff here is the behavior change. OneSignalImp only takes the nullable signatures (8 lines) and is what the kill switch falls back to. The base tests still run against OneSignalImp; the new tests are the *V2Tests files.

Scope

Init

  • initWithContext(context: Context?, appId: String?) and initWithContextSuspend accept null.
  • An empty, null or NUL appId falls back to the cached appId. Cached appIds (config model and legacy prefs) are validated too. With nothing usable, init is skipped and logged (WARN for null, ERROR otherwise). A fallback that succeeds logs one WARN.
  • A null context, or ensureApplicationServiceStarted throwing: log and return false.
  • Init state lives in InitGate. Waiters follow a retry that starts while they wait. A cancelled initWithContextSuspend ends in FAILED, so a later init can still succeed.
  • A retry after a failed init skips services that already bootstrapped (StartupService), so subscriptions are not duplicated.
  • Consent or GMS settings changed while init is finishing reach the config model, and consent granted then wakes the operation queue.

Calls before init (NOT_STARTED or FAILED)

  • login, logout and updateUserJwt log and return. Composite login returns NOT_INITIALIZED.
  • Accessors return the deferred managers from PR 3. Queued registrations replay before services start and before SUCCESS.
  • Every other call logs one ERROR naming the call; the init failure trace is attached once per attempt.

Unchanged

  • During init in progress, accessors still block and return the real manager.
  • Misconfigured*Manager (missing Gradle module) still throws.

Kill switch on (OneSignalImp)
Measured against main with the same calls: identical, except a null context now throws IllegalArgumentException (was a Kotlin NPE), and a Java null appId reaches the old resolution logic instead of an NPE at the call.

Public API
String to String? and Context to Context? on initWithContext. Same JVM descriptor, binary compatible, apiCheck passes.

Testing

Unit testing

  • InitGateTests: readiness per state, tryStart, waiting, a waiter following a retry.
  • SDKInitV2Tests, SDKInitSuspendV2Tests: null context; empty or null appId with and without a cache; accessors and login before init; captured references forward after init and after a retry; no duplicate operations on retry; replay before SUCCESS; one error per call with the trace once per attempt; consent kept; cancelled suspend init can retry; JWT listeners queued; cache fallback logs WARN.
  • AppIdHelperV2Tests, OneSignalImpV2Tests, StartupServiceTests.
  • The base SDKInitTests, SDKInitSuspendTests, OneSignalImpTests and AppIdHelperTests pass against OneSignalImp.
  • Each fix was reverted to confirm its test fails. core 1353, location 46, in-app-messages 172, notifications 246 pass; spotless, detekt, apiCheck pass.

Manual testing

Not yet tested on a device.

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

OneSignalImpV2 becomes the never-crash implementation (the default; the
kill switch still selects OneSignalImp):
- initWithContext accepts a null context or appId. An empty, null or NUL
  appId falls back to a validated cached appId; with none, init is
  skipped and logged.
- Init state lives in InitGate: waiters follow retries, a cancelled
  suspend init ends in FAILED, a retry skips services that already
  bootstrapped, and consent set while init finishes is kept.
- Calls before init log and return a default. Accessors hand out the
  deferred managers, whose queued registrations replay before services
  start and before SUCCESS.

OneSignalImp only takes the nullable signatures; a null context still
throws there.

Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions

Copy link
Copy Markdown
Contributor

📊 Diff Coverage Report

Diff Coverage Report (Changed Lines Only)

Gate: aggregate coverage on changed executable lines must be ≥ 80% (JaCoCo line data for lines touched in the diff).

Changed Files Coverage

  • StartupService.kt: 6/6 touched executable lines (100.0%) (9 touched lines in diff)
  • InitGate.kt: 59/62 touched executable lines (95.2%) (132 touched lines in diff)
  • OneSignalImp.kt: 2/2 touched executable lines (100.0%) (5 touched lines in diff)
  • OneSignalImpV2.kt: 119/133 touched executable lines (89.5%) (210 touched lines in diff)
  • AppIdResolutionV2.kt: 11/11 touched executable lines (100.0%) (37 touched lines in diff)

Overall (aggregate gate)

197/214 touched executable lines covered (92.1% — requires ≥ 80%)

📥 View workflow run

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.

1 participant