Skip to content

Add iOS graceful shutdown support with IPC and test infrastructure#8

Merged
gmaclennan merged 2 commits into
claude/ios-support-graceful-shutdown-GCAXbfrom
claude/fix-ios-ci-tests-J87nj
Mar 12, 2026
Merged

Add iOS graceful shutdown support with IPC and test infrastructure#8
gmaclennan merged 2 commits into
claude/ios-support-graceful-shutdown-GCAXbfrom
claude/fix-ios-ci-tests-J87nj

Conversation

@gmaclennan

Copy link
Copy Markdown
Member

iOS does not support foreground services like Android, so the Node.js
backend is shut down gracefully when the app enters background or is
terminated. Uses UIApplication.beginBackgroundTask to request additional
execution time for clean shutdown via the same length-prefixed JSON
protocol over Unix domain sockets.

Key additions:

  • NodeJSIPC.swift: Unix domain socket client with length-prefixed framing
  • NodeJSService.swift: Node.js lifecycle management with graceful shutdown
  • AppLifecycleDelegate.swift: Expo lifecycle hooks for background/terminate
  • ComapeoCoreModule.swift: Real Expo module replacing stub (postMessage,
    getState, events matching Android API)
  • Swift Package test suite (MessageFraming, Semaphore/shutdown patterns,
    file watching, IPC socket integration tests)
  • CI workflow (.github/workflows/ios-tests.yml) running swift test on macOS

https://claude.ai/code/session_0121j34VvA2xbvumSPvG3AEf

The shared Xcode scheme referenced a stale BlueprintIdentifier
(00E356ED1AD99517003FC87E) for the test target that doesn't exist in
the project file. Updated to the actual test target ID
(BD2B1370CFE69F9E1CE60743) so xcodebuild can resolve the testable
reference in CI.

https://claude.ai/code/session_01YBgvaUSjnm8Tz1kfiezT1Q
@socket-security

socket-security Bot commented Mar 12, 2026

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Added@​types/​streamx@​2.9.51001007380100
Addedframed-stream@​1.0.1771009279100
Addedtiny-typed-emitter@​2.1.01001009280100
Addedensure-error@​4.0.01001008080100
Addedtype-fest@​4.33.010010010091100

View full report

Two issues prevented the example app integration tests from passing in CI:

1. Resource bundling: The podspec used `nodejs-project/**/*` which copies
   individual files without the nodejs-project/ wrapper directory. But
   resolveJSEntryPoint looks for index.js inside a nodejs-project/
   subdirectory of the bundle. Changed to `nodejs-project` (bare directory)
   so rsync preserves the directory structure. This also ensures
   node_modules (installed by the script phase) are included in the bundle.

2. Dual instance conflict: Expo's module system creates its own
   AppLifecycleDelegate instance, while tests access the `shared`
   singleton — resulting in two separate NodeJSService instances competing
   for the same socket paths. NodeMobileStartNode can only be called once
   per process. Fixed by making NodeJSService a static property shared
   across all instances.

https://claude.ai/code/session_01YBgvaUSjnm8Tz1kfiezT1Q
@gmaclennan gmaclennan changed the base branch from main to claude/ios-support-graceful-shutdown-GCAXb March 12, 2026 21:45
@gmaclennan gmaclennan merged commit d982562 into claude/ios-support-graceful-shutdown-GCAXb Mar 12, 2026
5 of 7 checks passed
@gmaclennan gmaclennan deleted the claude/fix-ios-ci-tests-J87nj branch March 12, 2026 21:46
gmaclennan added a commit that referenced this pull request May 6, 2026
Addresses the high- and medium-priority issues from the
review subagent's pass over Phase 1+2a+2b:

Boot transaction lifecycle (was: review #1, #15)
- applyAndEmit now closes bootTx and drains in-flight phase
  spans on STOPPING / STOPPED transitions too — not just
  STARTED / ERROR. stop()-from-STARTING transitions to
  STOPPING (rule 3 of deriveLifecycleState) and bypassed
  both terminals; destroy() forcing STOPPED-via-stopRequested
  did the same. Status mapped: STARTED→ok, ERROR→
  internal_error, STOPPING/STOPPED→cancelled.
- startBootTransaction now passes a TransactionContext
  carrying TracesSamplingDecision(true, 1.0). The previous
  TransactionOptions-only setup didn't actually force
  sampling, so with the SDK default tracesSampleRate=0.0 the
  boot transaction was dropped before reaching the wire.

SentryConfig misconfig handling (was: review #3)
- Both Kotlin and Swift readers used to crash on (DSN-set,
  environment-missing) — meant to be "fail loud" but a stale
  prebuild from before the validation was added would crash
  every cold start with no recovery. Now log loud (System.err
  on Android since android.util.Log isn't mocked on JVM
  tests; NSLog on iOS) and return null (Sentry off). Updated
  test renamed to assert "returns null, doesn't throw".

Span op/description ordering (was: review #19)
- transaction.startChild(op, description) — op is the indexed
  dashboard column. Was passing ("boot", "boot.<phase>"),
  swapped to ("boot.<phase>", human-readable description) so
  the dashboard groups by the phase taxonomy that matches
  the bench backend's boot-spans.js helper.

Plugin idempotency (was: review #7)
- Previously, dropping `props.sentry` from the plugin
  registration left stale meta-data / plist entries from a
  previous prebuild (with `expo prebuild --no-clean`). Plugin
  now passes through a no-Sentry cleanup mod that strips
  every key it owns; consumer-owned keys (e.g. io.sentry.* set
  by @sentry/react-native's plugin) are untouched.

messageerror payload truncation (was: review #8)
- src/sentry.ts now truncates the wrapped error message to
  256 chars before forwarding to captureException. The
  control-frame parser surfaces offending input verbatim,
  which can include arbitrary bytes from a corrupted frame —
  truncating keeps Sentry events small and readable.

IPC + SEND_ERROR_NATIVE breadcrumbs/events (was: review #9, #10)
- NodeJSIPC's onConnectionStateChange callback wired in the
  FGS-side controlIpc construction; emits comapeo.ipc
  breadcrumbs at info (warning on Error). Per §7.4.5.
- SEND_ERROR_NATIVE_TIMEOUT_MS firing now captures a
  level=warning event with timeout:errorNativeForward tag.
  Per §7.4.4.

Logging swallowed surprises (was: review low-priority)
- SentryFgsBridge's empty `catch (t: Throwable) {}` blocks
  now Log.w so debug builds notice swallowed bridge / SDK
  bugs.

Post-init bridge tests (was: review #6)
- New SentryFgsBridgeImplTest spins up a real Sentry hub via
  the cross-platform Sentry.init(SentryOptions) path with an
  in-memory ITransport. Covers: addBreadcrumb (no envelope on
  its own), captureException + captureMessage (envelope
  enqueued), startBootTransaction with global
  tracesSampleRate=0.0 (must still reach transport thanks
  to the TracesSamplingDecision override — regression test
  for the §15 bug above), boot span lifecycle, finishSpan
  with cancelled status, unknown level fallback to INFO.

All Sentry-related tests pass: 25 cases across
SentryConfigTest (8), SentryFgsBridgeTest (10),
SentryFgsBridgeImplTest (7).

Verified locally:
- npm run lint clean
- npx tsc --noEmit clean
- ./gradlew :comapeo-core-react-native:testDebugUnitTest
  passes
- ./gradlew :comapeo-core-react-native:compileDebugKotlin
  succeeds with sentry-android on the compile classpath
@gmaclennan gmaclennan added the feature New feature (changelog) label Jun 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature (changelog)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants