feat: [SDK-4988] add OneSignalResult, OneSignalError, and the per-method payloads

[abdulraqeeb33]

Adds the public result model as a pure addition, wired to nothing. Nothing in the SDK returns these types yet — that starts in SDK-4990 — so the type design can be reviewed on its own before any API changes shape around it.

Part of SDK-4783. Ticket: SDK-4988.

Base

Targets ar/sdk-4986, which introduces the .api dump this PR extends. GitHub will retarget to feature/async-first-public-api once 4986 merges. The whole migration lands on that feature branch and merges to main once.

What's here

• OneSignalResult<T> — the envelope: isSuccess, the payload, the error, plus toMap/fromMap projections onto the cross-SDK wire schema.
• OneSignalError — a list of Detail plus the originating Throwable.
• ErrorCode / ErrorSource — the shared code catalog.
• InitData, LoginData, LogoutData, UpdateUserJwtData — the per-method payloads.

enum class ErrorCode(val source: ErrorSource) {
    NOT_INITIALIZED(CLIENT), STORAGE_LOCKED(CLIENT), INVALID_ARGUMENT(CLIENT),
    BACKEND_ERROR(BACKEND), UNKNOWN(CLIENT),
}

class OneSignalError internal constructor(error: List<Detail>, val cause: Throwable? = null) {
    class Detail internal constructor(
        val code: ErrorCode,
        val backendCode: Int? = null,
        val message: String? = null,
    )
    val error: List<Detail> = error.toList()
    val first: Detail get() = error.first()
}

Decisions worth your attention

An enum, not a sealed hierarchy. A sealed ErrorCode.Client.StorageLocked reads well in Kotlin and badly everywhere else — from Java it's ErrorCode.Client.StorageLocked.INSTANCE and can't be switched on, only chained through instanceof. An enum compiles to a real java.lang.Enum, so Java gets a native switch and the wrapper bridges get a name() marshal. The exhaustiveness argument for sealed doesn't apply: adding an enum constant breaks a Kotlin when in exactly the same way.

Backend codes stay a raw Int. The backend adds catalog codes on its own schedule, and an SDK release must not be what unblocks recognizing one. BACKEND_ERROR plus Detail.backendCode keeps that half open-ended.

Detail is nested rather than a top-level Error. A top-level com.onesignal.Error would shadow kotlin.Error, which is auto-imported into every Kotlin file, and java.lang.Error in any Java file that imports it.

No retryable or httpStatus. Both would be guesses the SDK can't currently back — nothing below the entry points reports a status code yet (SDK-4989), and retryability would be hardcoded per call site rather than derived from anything. A field customers branch on, populated by a guess, is worse than no field.

A list, not one reason. One request can fail for several reasons at once. Everything the SDK raises locally has exactly one, which first reads without the indexing ceremony.

cause is off the wire. A stack trace can't cross a wrapper bridge and the schema has to stay identical across SDKs, so cause exists only for native Kotlin and Java callers.

Wire shape

{ "success": false, "data": null,
  "error": [ { "code": "STORAGE_LOCKED", "source": "CLIENT", "backendCode": null, "message": "..." } ] }

fromMap never throws on unexpected input:

Malformed input	Behavior
Unrecognized code	Degrades to UNKNOWN, original text kept on message
Missing code	Degrades to UNKNOWN, message preserved
Empty reason list	Yields one UNKNOWN reason so first stays safe
Unknown extra keys	Ignored
success: true alongside an error	Error wins; success is derived, never trusted

That matters because the enum is closed, so a wrapper built against an older SDK will eventually meet a code it can't name — a strict valueOf would throw at exactly that moment.

Self-review

Five defects found on a fresh adversarial pass, all fixed here:

• KDoc documented a wire shape that no longer exists. OneSignalResult's class doc still showed the nested error.error envelope from before the list was flattened. This file is what a wrapper author reads to build their parser, so a wrong doc is a defect in the deliverable.
• The non-empty invariant was documented but unenforced. error is documented "Never empty" and first as always safe, but only the factories guarded it — a direct constructor call did not.
• The invariant could still be defeated after construction. Found while writing up the previous item: the list was aliased rather than copied, so a caller holding a MutableList could clear it after require passed. Now a defensive toList().
• Exception text appended a bare null. A Detail with no message rendered as "STORAGE_LOCKED: null" in getOrThrow's stack trace.
• Interface declared in the wrong file. OneSignalResultData lived in OneSignalResult.kt while OneSignalResultData.kt held only the payloads.

One open question

Every constructor here is internal. That correctly stops customers fabricating SDK results in production, but it also stops them unit-testing their own error-handling branches — there's no supported way to build a failed OneSignalResult in a customer's test suite, and wrapper SDKs hit the same wall unless they live in this Gradle module. Worth deciding now, since a testing entry point is additive and cheap to add later but awkward to discover after adoption.

Not in this PR

Nothing populates BACKEND_ERROR or backendCode. That detail isn't reachable from the entry points yet; SDK-4989 is what carries it up. This PR only defines where it will go.

Test plan

• :OneSignal:core:testDebugUnitTest — full core suite green
• :OneSignal:core:apiCheck — .api diff shows ErrorCode/ErrorSource as java/lang/Enum
• :OneSignal:core:detekt and spotlessApply
• Round-trip coverage for success, failure, and empty payloads
• Degradation coverage for an unrecognized code, a missing code, and an empty reason list
• Constructor rejects an empty reason list

Made with Cursor

[github-actions]

📊 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

• OneSignalError.kt: 28/30 touched executable lines (93.3%) (160 touched lines in diff)
• OneSignalResult.kt: 17/19 touched executable lines (89.5%) (113 touched lines in diff)
• OneSignalResultData.kt: 9/17 touched executable lines (52.9%) (92 touched lines in diff)
  • 8 uncovered touched lines in this file

Overall (aggregate gate)

54/66 touched executable lines covered (81.8% — requires ≥ 80%)

Per-file detail (informational; gate is aggregate above):

• OneSignalResultData.kt: 52.9% (8 uncovered touched lines)

📥 View workflow run

[abdulraqeeb33]

i have added this an example, i will get rid of it when i integrate it with the login api

[fadi-george]

i thought were avoiding error codes

[fadi-george]

Ran this through a couple of adversarial passes. Design reads well, but a few things in the parsing half are worth fixing before the wire contract is locked in.

fromMap picks the error branch with as? List<...>, which doubles as the predicate. Anything present but not a kotlin.collections.List (a bare map, or a JSONArray, which doesn't implement java.util.List) silently falls through to the success branch and yields LoginData("", "") with isSuccess == true. The reverse direction is guarded but this one isn't, and org.json is the natural parse on the bridge side.

Same cast is erased at the element level, so error = listOf("oops") gets past it and throws ClassCastException inside Detail.fromMap. The suppression is what hides it. Both cases contradict the never-throws table.

getError() hands back a mutable ArrayList. The defensive copy stops the caller mutating the input list, but not the copy, so from Java getError().clear() then getFirst() throws and the require is bypassed. Collections.unmodifiableList closes it.

source goes out on the wire but fromMap never reads it back, and unknown codes degrade to UNKNOWN which is hardcoded CLIENT. So a backend failure comes back attributed to the client in exactly the forward compat case the degradation is for. For a schema that has to stay identical across SDKs, source is write only right now and another SDK can't tell whether it's authoritative.

On the open question about internal constructors: they don't actually stop fabrication. internal on a constructor is metadata only, it emits as JVM public, so new OneSignalResult<>(null, null) compiles from Java. BCV also filters internal out of the dump, so apiCheck can't see that surface at all. Worth deciding deliberately rather than leaving an accidental entry point only Java can reach.

Two smaller ones: OneSignalResultData is publicly implementable so any member added later is a break, sealed would close it at no cost. And OneSignalResult doesn't enforce its own either/or invariant, with isSuccess keyed off error and getOrThrow off data, so the two disagree whenever it's violated.