feat: [SDK-5022] add demo test crash flow by fadi-george · Pull Request #1715 · OneSignal/OneSignal-iOS-SDK

fadi-george · 2026-08-14T06:07:20Z

Description

One Line Summary

Adds a marked test-crash flow to the SwiftUI demo app.

Details

Motivation

The demo needs a deterministic way to exercise the SDK's uncaught Objective-C exception capture without treating unrelated host-app crashes as OneSignal failures.

Scope

Adds a secondary demo screen and crash action, plus an explicit userInfo marker recognized by the crash handler. Unmarked host-app exceptions remain ignored.

Other

Stacked on #1714.

Testing

Unit testing

Added crash-handler coverage for marked and unmarked host-app exceptions.

Manual testing

Triggered the demo crash and confirmed it is persisted and uploaded after relaunch.

Affected code checklist

Checklist

Overview

I have filled out all required sections above
PR does one thing
Any public API changes are explained above

Testing

I have included test coverage for these changes
Applicable automated tests pass
I have manually tested the demo flow

Final pass

Code is as readable as possible
I have reviewed this PR

Made with Cursor

cursor

Multi-model adversarial review (interrogate)

Intent

Add a deterministic demo test-crash flow that exercises the SDK's uncaught Objective-C exception capture without treating unrelated host-app crashes as OneSignal failures — via a SecondaryView crash action and a userInfo marker (OSCrashTestMarker) that lets OSLogCrashHandler persist marked host-stack exceptions. Unmarked host exceptions stay ignored. Stacked on #1714.

Reviewers

A: claude-fable-5-thinking-xhigh (6)
B: gpt-5.6-sol-xhigh (4)
C: cursor-grok-4.5-high-fast (7)
D: claude-opus-5-thinking-high (11)

Act On

Undeclared public API ships a permanent attribution bypass (A/B/C/D) — public enum OSCrashTestMarker is new supported surface on distributed OneSignalOSCore, while the PR checklist leaves Public API changes unchecked. Commit 7f205d3a already did the same gate with a private static let exception name and zero public API; a0f0e53f traded that away. Prefer: (a) OSCore-owned raise helper so isOneSignalAtFault accepts the frame with no bypass, (b) restore private name-based marker + string literal in the demo, or (c) @_spi + document if a symbol is truly required.
Marked crashes are unlabeled in telemetry (A/C/D) — marker is gate-only then discarded; CrashData gets RuntimeException + host stack + caller reason. Ops cannot filter demo pollution except brittle string match. Name-based predecessor self-labeled via exceptionType. Stamp a deterministic type/prefix when the marker branch admits the crash, or delete the bypass via the OSCore raise helper.
Wrong-layer OR on the shared fatal path (A/C/D) — isMarkedTestCrash || isOneSignalAtFault encodes a demo concern into every uncaught-exception path. Code judo: raise from OneSignalOSCore so attribution already works; keep handle attribution-only.

Consider

Demo is the only app file importing OneSignalOSCore (C/D) — teaches sub-framework coupling for a string constant.
Unit tests call handle(...) directly; no raise→handler→persist→relaunch coverage (B/D). Thin cases: no NSNumber bridging, no previous-handler forward assert, reason: nil unlike demo.
TEMP_* pbxproj UUID churn vs documented xcodegen generate (A/B/C/D).
Crash button can fire before remote-logging arms the handler (B) — not unique to this PR, but weakens “deterministic.”
#if DEBUG alone is insufficient — e2e archives Release (D).

Noted

Dismissed

Treating the bypass as a critical third-party “poison” exploit — intentional opt-in for integration apps; the real defect is shipping it as undeclared public supported API with unlabeled payloads.
Demanding a full Catalyst UI path in this PR — nice-to-have on the #1714 stack, not required for the stated iOS demo flow.

Agreement Map

4/4: public API + checklist gap. 3/4 (A/C/D): unlabeled telemetry after marker discard; prefer deleting the shared-path OR via OSCore-framed raise or private name marker. B uniquely stressed handler-not-armed timing. Strongest judo (C): OSCore raise helper deletes findings 1–3 together.

Verdict: Intent is sound and the marked path works in unit tests, but the userInfo refactor regresses the private name-based design — shipping public API and unlabeled host crashes into the SDK fault pipeline. Prefer the OSCore raise helper or restore private self-labeling before merge.

_{Sent by Cursor Automation: Automatic PR Review}

cursor · 2026-08-14T06:23:45Z

+/// this key to `true` in `NSException.userInfo` is the only supported way to bypass that.
+public enum OSCrashTestMarker {
+    public static let userInfoKey = "com.onesignal.crash.test"
+}


Act on — undeclared public API (A/B/C/D).

OneSignalOSCore ships to integrators (SPM + xcframework). This public enum is new supported surface whose only in-repo consumer is the demo, and the PR checklist leaves Public API changes unchecked.

Commit 7f205d3a already gated the same path with private static let testCrashExceptionName and no public symbol; a0f0e53f traded that for Android-parity RuntimeException + this key.

Prefer an OSCore-owned raise helper (throwing frame = OneSignalOSCore → existing isOneSignalAtFault), or keep the marker internal/@_spi and document if a public constant is truly required.

cursor · 2026-08-14T06:23:45Z


    func handle(exception: NSException, stackSymbols: [String]) {
-        guard Self.isOneSignalAtFault(stackSymbols) else {
+        guard Self.isMarkedTestCrash(exception) || Self.isOneSignalAtFault(stackSymbols) else {


Act on — demo concern on the shared fatal path (A/C/D).

This || punches a hole in the host-crash firewall from #1713 for any process that can set the documented key. Complexity belongs with the demo raise site, not every uncaught exception.

Code judo: OSCrashTest.raise(...) (or equivalent) inside OneSignalOSCore so attribution already accepts the frame — then delete isMarkedTestCrash entirely.

cursor · 2026-08-14T06:23:45Z

+        guard Self.isMarkedTestCrash(exception) || Self.isOneSignalAtFault(stackSymbols) else {
            previousExceptionHandler?(exception)
            return
        }


Act on — marker is gate-only; uploaded records are unlabeled (A/C/D).

After the guard admits a marked crash, the marker is discarded. CrashData gets exception.name (RuntimeException), caller reason, and a pure host stack — indistinguishable from a real SDK-attributed fault except brittle string matching.

The name-based predecessor self-labeled via exceptionType == "com.onesignal.crash.test". If a bypass must remain, stamp a deterministic type/prefix here when the marker branch fires, and assert it in tests.

cursor · 2026-08-14T06:23:45Z

+ */
+
+import Foundation
+import OneSignalOSCore


Consider — only demo file importing OneSignalOSCore (C/D).

Other app sources use OneSignalFramework / feature modules. This import exists solely for OSCrashTestMarker.userInfoKey and teaches sub-framework coupling.

If the raise helper / private marker path lands, this import should disappear.

cursor · 2026-08-14T06:23:45Z

+            name: NSExceptionName("RuntimeException"),
+            reason: "Test crash from OneSignal Demo App - \(timestamp)",
+            userInfo: [OSCrashTestMarker.userInfoKey: true]
+        ).raise()


Act on / judo — host-raised crash forces the bypass (C).

Raising from app code guarantees no OneSignal frame, which is why the marker exists. An OSCore helper that raises would put OneSignalOSCore on the stack (oneSignalModules already includes it) and keep handle attribution-only — no public key, no unlabeled host records.

Also: DateFormatter without en_US_POSIX makes the reason string locale-dependent; with unlabeled payloads that string is the only filter handle.

cursor · 2026-08-14T06:23:45Z

+                .filter { $0.hasSuffix(".otlp") }
+                .count,
+            1
+        )


Consider — thin verification vs claimed contract (B/D).

Covers Swift true → .otlp count only. Missing: NSNumber/@YES bridging, previous-handler still forwarded after marked capture, payload exceptionType/message assertions, and any raise→uncaught-handler path (tests call handle directly). reason: nil also diverges from the demo’s always-set reason.

cursor · 2026-08-14T15:25:43Z

Thanks for this — the demo crash flow looks solid. SecondaryView, the marked vs unmarked host-exception tests, and the manual persist-and-upload-after-relaunch check all make sense for exercising app-crash capture.

Two small requests, if you have a chance:

Public API. I don’t think we need OSCrashTestMarker as public surface on OneSignalOSCore. Could we keep it internal / @_spi, or raise the demo crash from an OSCore helper so the existing attribution path covers it without a new public symbol?
Uncaught-exception handler. The current tests call handle(...) directly, which is useful, but would it be possible to also simulate a real uncaught NSException going through the installed handler (raise → handler → persist)? Even one case like that would add a lot of confidence that the demo path matches production.

Everything else looks good to me — thank you!

abdulraqeeb33 · 2026-08-14T15:27:21Z

Also @fadi-george were you able to test a crash is captured in both KMP and iOS core code?

fadi-george added 4 commits August 13, 2026 18:38

feat(demo): add SecondaryView with crash/ANR actions

17ba0ae

refactor(demo): remove ANR action from SecondaryView

46458bf

feat(logging): persist marked test crashes

7f205d3

refactor(logging): replace name-based crash marker with userInfo key

a0f0e53

fadi-george mentioned this pull request Aug 14, 2026

feat: [SDK-5032] enable Mac Catalyst demo builds #1716

Open

14 tasks

cursor Bot reviewed Aug 14, 2026

View reviewed changes

Navigation Menu

Uh oh!

feat: [SDK-5022] add demo test crash flow - #1715

feat: [SDK-5022] add demo test crash flow#1715
fadi-george wants to merge 4 commits into
fadi/sdk-4998from
fadi/sdk-5022

fadi-george commented Aug 14, 2026

Uh oh!

cursor Bot left a comment

Uh oh!

cursor Bot Aug 14, 2026

Uh oh!

cursor Bot Aug 14, 2026

Uh oh!

cursor Bot Aug 14, 2026

Uh oh!

cursor Bot Aug 14, 2026

Uh oh!

cursor Bot Aug 14, 2026

Uh oh!

cursor Bot Aug 14, 2026

Uh oh!

cursor Bot commented Aug 14, 2026

Uh oh!

abdulraqeeb33 commented Aug 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

fadi-george commented Aug 14, 2026

Description

One Line Summary

Details

Motivation

Scope

Other

Testing

Unit testing

Manual testing

Affected code checklist

Checklist

Overview

Testing

Final pass

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Multi-model adversarial review (interrogate)

Intent

Reviewers

Act On

Consider

Noted

Dismissed

Agreement Map

Uh oh!

cursor Bot Aug 14, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot Aug 14, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot Aug 14, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot Aug 14, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot Aug 14, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot Aug 14, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot commented Aug 14, 2026

Uh oh!

abdulraqeeb33 commented Aug 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants