Phase 1: Sentry integration plan and JS-side adapter#54
Open
gmaclennan wants to merge 6 commits intomainfrom
Open
Phase 1: Sentry integration plan and JS-side adapter#54gmaclennan wants to merge 6 commits intomainfrom
gmaclennan wants to merge 6 commits intomainfrom
Conversation
Plans an opt-in, host-app-driven Sentry integration covering: - error capture across backend (Node), JS/RN, and native layers - RPC tracing via @comapeo/ipc onRequestHook (mirrors comapeo-mobile) - forwarding @comapeo/core OpenTelemetry spans (PR digidem/comapeo-core#1051) - app-specific gating so non-CoMapeo consumers ship no Sentry traffic https://claude.ai/code/session_01EcVXzczA1TVkhEkgUg9DKX
Closes the FGS-cold-start gap where the prior draft required RN to be alive before backend Sentry could initialize: - §4 reworked: Expo config plugin writes DSN/environment/release into Android manifest meta-data and iOS Info.plist at prebuild time. Native reads those at process start, no JS round-trip, before booting @sentry/node and @sentry/android. - §7.4 added: native telemetry data design mapped onto Sentry primitives (breadcrumbs for state transitions, transaction + spans for boot/shutdown phases, captureMessage for timeouts, tags/contexts for cross-process attribution). Categorizes captures as essential vs opt-in and documents a hard never-capture list for PII. - §9 added: persisted "capture application data" toggle with restart-to-activate semantics. Snapshot read at boot, embedded in the init frame; gates per-RPC spans, sync-session transactions, memory checkpoints, and storage-size sampling. Never unlocks the never-capture list. - §10 phasing and §13 file-change list updated. New open questions added for release tagging, plugin no-op behavior, toggle UI, and boot sample rate. https://claude.ai/code/session_01EcVXzczA1TVkhEkgUg9DKX
Three corrections after deeper review of the comapeo-mobile backend implementation: - §4.1: drop applicationId-suffix derivation. That convention is CoMapeo-specific and shouldn't be hard-coded in this module. Consumers feed `environment` via app.config.js + EAS build-profile env vars (SENTRY_ENVIRONMENT). The plugin requires `dsn` and `environment`; `release` defaults to versionName at runtime. - §4.5 + §5: replace control-socket init-frame transport with Node argv at spawn time. Sentry config in argv satisfies the auto-instrumentation order requirement (Sentry.init must run before any module loads so import-in-the-middle can patch them). The init frame stays focused on the rootkey, which we still deliberately keep out of argv. - §5.1-5.3: multi-entry rollup with separate `loader.mjs` (parses argv, conditionally inits Sentry, dynamically imports index.mjs), `importHook.js` (the import-in-the-middle hook, must be a separate file because it's loaded with module.register), and `lib/register.js` (a sub-dep that resolves at a hard-coded relative path). Includes the path-rewrite plugin from comapeo-mobile's rollup config. @sentry/node chunk loads only when --sentryDsn is passed; consumers without Sentry pay disk cost only, no runtime cost. - §9.5: toggle plumbing now goes via argv flag (--captureApplicationData), not the init frame. - §11 test plan: covers loader argv parsing, lazy chunk gating, rollup output shape, and the rewritten import-hook reference. - §12 open questions: add iOS lazy-chunk verification (--jitless), offline transport decision, sourcemap upload setup, and the capture-application-data default in non-production environments. https://claude.ai/code/session_01EcVXzczA1TVkhEkgUg9DKX
Replaces speculation with concrete decisions across 9 of 13 open questions from §12, leaving 3 verify-during-build items: - §6.2: replace Option A/B speculation with the concrete onRequestHook pattern from comapeo-mobile/src/frontend/lib/ createMapeoApi.ts. Confirmed @comapeo/ipc@^8 supports onRequestHook directly; uses Sentry.getActiveSpan() short-circuit for the no-op path. - §4.1 + §4.2: release defaults to versionName + "+" + versionCode on Android (longVersionCode) and CFBundleShortVersionString + "+" + CFBundleVersion on iOS. Successive EAS builds of the same marketing version produce distinct releases. - §5.1: pin @sentry/node@^8, @sentry/react-native@^6, @sentry/core@^8 — OpenTelemetry-first majors required for PR #1051 forwarding to work without glue. - §5.1: drop @sentry/rollup-plugin. Module ships sourcemaps in the npm package; consumer excludes from APK/IPA and runs sentry-cli sourcemaps upload in their own CI with their own credentials. Adds a README task for documenting the consumer workflow. - §9.7: capture-application-data default is per-environment via a new captureApplicationDataDefault plugin field. EAS pattern: default to true when environment !== "production". User toggle always overrides once set. - §12: collapsed into §12.1 Decided (9 entries) + §12.2 Still open / verify-during-build (3 entries: cross-process FGS tag scope, iOS jitless lazy chunk, offline transport deferred). https://claude.ai/code/session_01EcVXzczA1TVkhEkgUg9DKX
Implements the JS-side adapter handoff (Phase 1) and the Expo config
plugin + native config readers (Phase 2a) from
docs/sentry-integration-plan.md. The native-side direct-Sentry-SDK
init in the FGS process and the breadcrumb/span/event calls remain
deferred to Phase 2b — they need a Gradle dep on
io.sentry:sentry-android-core, scoped out of this PR.
Phase 1 (JS-side adapter):
- src/sentry-internal.ts holds the active SentryAdapter slot. Phase
3 RPC tracing reads from it.
- src/sentry.ts exposes configureSentry({ sentry }) as the public
API. SentryAdapter is hand-rolled (not Pick<typeof @sentry/...>)
so consumers without the optional peer dep don't get a typecheck
error. State listeners emit a comapeo.state breadcrumb on every
transition and a captureException tagged ComapeoError:<phase> on
ERROR. messageerror events capture as warning-level.
- package.json gains an exports field (./, ./sentry, ./app.plugin)
and lists @sentry/react-native@^6 as an optional peer dep.
Phase 2a (config plugin + native readers):
- app.plugin.js (root) is an ESM Expo plugin (the package is
type:module). withAndroidManifest upserts <meta-data> on the main
<application>; withInfoPlist upserts plist keys. Validates
dsn+environment at prebuild. No-op when registered without a
sentry argument.
- SentryConfig.kt + SentryConfigTest.kt land the typed manifest
reader. Pure load(metaString, defaultRelease) overload makes the
parser unit-testable on the JVM classpath without mocking
android.os.Bundle. Default release = versionName + "+" +
longVersionCode (API 28+) so successive EAS builds disambiguate.
- SentryConfig.swift + SentryConfigTests.swift mirror the Android
side. Default release = CFBundleShortVersionString + "+" +
CFBundleVersion. Accepts both string-coerced (the plugin's normal
output) and native plist types defensively.
Plan + README updated:
- docs/sentry-integration-plan.md §10.2 split into Phase 2a
(this PR) + Phase 2b (follow-up). §13 file list reflects what
actually shipped.
- README gains an Optional: Sentry integration section with the
app.config.js + configureSentry pattern.
https://claude.ai/code/session_01EcVXzczA1TVkhEkgUg9DKX
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
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
2 participants
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.
Summary
This PR implements Phase 1 of the Sentry integration plan for
@comapeo/core-react-native, establishing the foundation for opt-in error reporting and RPC tracing. The integration is designed to be app-driven and zero-cost for consumers who don't use it.Key Changes
Documentation & Architecture
docs/sentry-integration-plan.md(2073 lines): Comprehensive design document covering:JS-Side Implementation
src/sentry.ts: Public sub-export (@comapeo/core-react-native/sentry) providing:SentryAdapterinterface: hand-rolled type contract matching@sentry/react-native@^6methods without hard dependencyconfigureSentry()function: one-shot handoff for the host app's already-initialized Sentry clientsrc/sentry-internal.ts: Internal state management for the active Sentry adapterExpo Config Plugin
app.plugin.js(217 lines): Build-time plugin that:com.comapeo.core.sentry.*keys)ComapeoCore*prefixed to avoid collisions)Native Configuration Types
android/src/main/java/com/comapeo/core/SentryConfig.kt: Typed data class and loader for Android manifest meta-dataPackageManager.getApplicationInfo().metaData:ComapeoCoreFGS processversionName+versionCodewhen not explicitly setios/SentryConfig.swift: Equivalent iOS implementation reading fromBundle.main.infoDictionaryCFBundleShortVersionString+CFBundleVersionTest Coverage
android/src/test/java/com/comapeo/core/SentryConfigTest.kt: JVM unit tests for Android config deserializationios/Tests/SentryConfigTests.swift: Swift unit tests for iOS config deserializationPackage Configuration
package.json: Added conditional exports for the new./sentrysub-exportios/Package.swift: AddedSentryConfig.swiftto the library targetREADME.md: Added optional Sentry integration section with setup instructionsImplementation Details
Design Principles
@sentry/react-nativeor@sentry/node; it accepts a Sentry-shaped adapter objectConfiguration Flow
app.config.jswith DSN and environment from build-time sources (EAS env vars)expo prebuildhttps://claude.ai/code/session_01EcVXzczA1TVkhEkgUg9DKX