Repository navigation
fix: [SDK-5538] never crash on invalid init or calls before init - #2803
Draft
abdulraqeeb33 wants to merge 1 commit into
Draft
abdulraqeeb33 wants to merge 1 commit into
abdulraqeeb33 wants to merge 1 commit into
Conversation
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>
Contributor
📊 Diff Coverage ReportDiff 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
Overall (aggregate gate)197/214 touched executable lines covered (92.1% — requires ≥ 80%) |
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
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.
Stack (base
feat/never-crash, merged tomainas one unit; split out of #2793):Description
One Line Summary
OneSignalImpV2becomes 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 orloginbefore 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.ktstarts as a copy ofOneSignalImp(PR 2), so its diff here is the behavior change.OneSignalImponly takes the nullable signatures (8 lines) and is what the kill switch falls back to. The base tests still run againstOneSignalImp; the new tests are the*V2Testsfiles.Scope
Init
initWithContext(context: Context?, appId: String?)andinitWithContextSuspendaccept null.ensureApplicationServiceStartedthrowing: log and returnfalse.InitGate. Waiters follow a retry that starts while they wait. A cancelledinitWithContextSuspendends in FAILED, so a later init can still succeed.StartupService), so subscriptions are not duplicated.Calls before init (NOT_STARTED or FAILED)
login,logoutandupdateUserJwtlog and return. CompositeloginreturnsNOT_INITIALIZED.Unchanged
Misconfigured*Manager(missing Gradle module) still throws.Kill switch on (
OneSignalImp)Measured against
mainwith the same calls: identical, except a null context now throwsIllegalArgumentException(was a Kotlin NPE), and a Java null appId reaches the old resolution logic instead of an NPE at the call.Public API
StringtoString?andContexttoContext?oninitWithContext. Same JVM descriptor, binary compatible,apiCheckpasses.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.SDKInitTests,SDKInitSuspendTests,OneSignalImpTestsandAppIdHelperTestspass againstOneSignalImp.Manual testing
Not yet tested on a device.
Affected code checklist
Checklist
Overview
Testing
Final pass