Skip to content

Commit f9376f6

Browse files
authored
Merge branch 'main' into alwx/ci/sample-app-production-only-builds
2 parents 8beab64 + 4c8732b commit f9376f6

288 files changed

Lines changed: 18584 additions & 3710 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.agents/skills/.gitkeep

Whitespace-only changes.
Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
---
2+
name: code-guidelines
3+
description: Enforce Sentry React Native SDK code guidelines for implementation, refactoring, and review. Use when implementing features, adding functionality, refactoring, reviewing code, designing APIs, changing the public API surface (the `packages/core/src/js/index.ts` barrel), handling breaking changes, deprecating options, writing integrations, touching the JS↔native bridge, or making architecture decisions in this yarn-workspaces monorepo.
4+
---
5+
6+
Apply these guidelines to all new and modified code in `packages/core`. Existing code may not follow these conventions — do not refactor it unless asked.
7+
8+
This SDK spans **four surfaces**: TypeScript/JS (`packages/core/src/js/`), Android (Java/Kotlin, `packages/core/android/`), iOS (ObjC/Swift, `packages/core/ios/`), and the **JS↔native bridge** (`RNSentry` TurboModule / legacy NativeModule) that connects them. Read the `AGENTS.md` for the surface you're touching — root, `packages/core/`, `packages/core/android/`, `packages/core/ios/` — before writing. The bridge is where most SDK-specific bugs live; the deep guidance for it is in [`packages/core/AGENTS.md`](../../../packages/core/AGENTS.md).
9+
10+
For non-trivial work — a new feature or integration, a public barrel-file change, or a change that crosses the native bridge — load the **design-first** skill to shape the modules and seams before writing code. When writing or modifying tests, also load the **test-guidelines** skill.
11+
12+
## SDK Development Rules
13+
14+
### Integrations
15+
16+
SDK features are packaged as **integration factory functions** that return an `Integration` (from `@sentry/core`). See `src/js/integrations/` for the canonical shape (e.g. `nativelinkederrors.ts`):
17+
18+
```typescript
19+
const INTEGRATION_NAME = 'MyFeature';
20+
21+
export const myFeatureIntegration = (options: Partial<MyOptions> = {}): Integration => {
22+
return {
23+
name: INTEGRATION_NAME,
24+
setupOnce: () => { /* one-time global side effects, if any */ },
25+
setup: (client: Client) => { /* per-client wiring */ },
26+
processEvent: (event, hint, client) => { /* enrich/drop */ return event; },
27+
};
28+
};
29+
```
30+
31+
- **Check prerequisites and feature flags early** — if the feature is disabled or the native module is missing, log via `debug` and return without wiring anything up.
32+
- Name the integration with a module-level `INTEGRATION_NAME` constant; don't inline the string.
33+
- Integrations should be **order-independent**. If yours must run before/after another, reconsider the design.
34+
- Register default integrations in `src/js/integrations/default.ts`; keep an integration's options and defaults with the integration, not scattered across `options.ts`.
35+
- Clean up anything global (listeners, timers) — an integration that arms a timer or subscribes to an emitter must have a path that tears it down.
36+
37+
### Native Bridge (JS side)
38+
39+
Every call into native goes through the `NATIVE` wrapper (`src/js/wrapper.ts`) / `RNSentry` TurboModule. Rules:
40+
41+
- **Guard on availability.** If `enableNative` is false or the module isn't linked, degrade gracefully — return a safe fallback, never throw into user code.
42+
- **Never let a native rejection escape.** Wrap bridge calls; log via `debug` and return a fallback. A thrown promise rejection from the bridge becomes an unhandled rejection in the host app.
43+
- **Everything crossing the bridge must be serializable** — plain JSON (no functions, class instances, `undefined` holes, circular refs, or `BigInt`). Both the New Architecture (JSI/TurboModule codegen) and the legacy bridge serialize payloads; a non-serializable value is silently dropped or throws on the native side. Verify `toJSON`/normalization round-trips.
44+
- **Do not wrap `RNSentry`'s own scope-sync methods** (`setTag`, `setContext`, `addBreadcrumb`, `setUser`, `captureEnvelope`, …). Wrapping them recurses — see the `RNSENTRY_SKIP` / `ignoreTurboModules` notes in `packages/core/AGENTS.md`.
45+
- In this hot path, `logger` from `@sentry/core` is the **Logs API** (emits log events), not the debug logger. Use **`debug`** for diagnostics — see the TurboModule section of `packages/core/AGENTS.md`.
46+
47+
### Native Bridge (native side)
48+
49+
- **Android** (`packages/core/android/`): resolve/reject the `Promise` on every path; never leak it. Release JNI local refs; don't let a native exception cross back into the bridge uncaught. Old vs new arch code lives in `src/oldarch/` and `src/newarch/`; shared code in `src/main/`.
50+
- **iOS** (`packages/core/ios/`): prefix classes `RNSentry`; use nullability annotations; watch for retain cycles in blocks (capture `weakSelf`). Route hybrid-SDK access through `RNSentryInternal` (see `packages/core/ios/AGENTS.md`), not deprecated `PrivateSentrySDKOnly`.
51+
- A native exception must **never crash the host app** — catch at the bridge boundary and reject/log instead.
52+
53+
### Codegen (New Architecture)
54+
55+
The bridge spec is codegen'd. `src/js/NativeRNSentry.ts` is the `TurboModule` spec; `RNSentryReplayMask*NativeComponent.ts` are Fabric component specs.
56+
57+
- Codegen only supports a **restricted type vocabulary** — primitives, `Object` (untyped map), arrays, and nullable via `?`. No unions of literals, no generics, no `Record<K,V>` with non-string keys. If a method's shape can't be expressed, pass an `Object` and validate/parse on the native side.
58+
- Any change to a `Native*.ts` spec is a **bridge ABI change**: it must land in lockstep across the JS spec, `ios/RNSentry.mm`, and the Android `RNSentryModuleImpl`, and stay backward-compatible with older native binaries a user may have cached. Treat it like a public API change.
59+
60+
### Usage tracking & SDK metadata
61+
62+
The event's [SDK interface](https://develop.sentry.dev/sdk/data-model/event-payloads/sdk/) carries `sdk.integrations`, `sdk.packages`, and an optional `sdk.features` list; the spec says a feature should be reported through **either** an integration **or** the `features` list, not both. RN takes the *integrations* route: unlike sentry-dart (which populates `sdk.features` via an explicit `addFeature` / `SentryFeatures` API), RN's `@sentry/core` reports usage only through `event.sdk.integrations` and does **not** populate `sdk.features`. So usage is reported implicitly:
63+
64+
- An **installed integration is auto-reported by its `name`**`@sentry/core` collects the names of installed integrations into `event.sdk.integrations`. So the practical rule is: give the integration a stable, correct `INTEGRATION_NAME`, and register it (a default in `integrations/default.ts`, or `client.addIntegration(...)`) — that registration *is* the usage signal. A feature that runs without a registered, named integration is invisible to usage tracking.
65+
- **SDK identity/packages** come from `src/js/version.ts` (`SDK_NAME`, `SDK_PACKAGE_NAME`, `SDK_VERSION`) surfaced by `integrations/sdkinfo.ts`, which also appends the native SDK package — not the place to register feature usage.
66+
- Do **not** confuse this with `addFeatureFlag` on the native module (`wrapper.ts` / `NativeRNSentry.ts`) — that is the user-facing **feature-flags** product, unrelated to SDK usage tracking.
67+
68+
When adding something you want measured, make sure it lands as a named, registered integration rather than a bare side effect.
69+
70+
### Logging
71+
72+
- **`debug`** (from `@sentry/core`) is the internal diagnostic logger — use it for all SDK diagnostics. It is tree-shaken / gated so it stays quiet in production.
73+
- **`logger`** (from `@sentry/core`) is the **Logs API** — it emits user-visible log *events*. Never use it for internal diagnostics, and never in a hot path (see bridge rules above).
74+
- Log at the right level: `debug` for lifecycle/config, `warn` for recoverable/degraded paths (native module missing, option ignored), `error` for failures that affect SDK behavior.
75+
76+
### Privacy (PII)
77+
78+
- **Never collect PII without gating on `options.sendDefaultPii`.** This covers IP inference, device identifiers, deep-link URLs, navigation params, request/response bodies, and user-supplied context.
79+
- Flag any change that could place user data into breadcrumbs, event payloads, span attributes, or logs for review.
80+
- Deliberate exceptions exist and must be documented where they live (e.g. TurboModule module/method names are app-defined identifiers sent regardless of `sendDefaultPii` — see `packages/core/AGENTS.md` "Privacy"). Don't add new always-on data collection without that justification.
81+
82+
### Breaking changes
83+
84+
- The public API is the `src/js/index.ts` barrel plus every exported option in `options.ts`. Its shape is captured in the API report (`packages/core/etc/sentry-react-native.api.md`); regenerate with `yarn api-report` and check with `yarn api-report:check`.
85+
- Removing or changing the signature/behavior of an exported symbol or option is a **semver-major breaking change**. Prefer **deprecation with a migration path** (`@deprecated` JSDoc + a working shim) over immediate removal.
86+
- Bridge/ABI changes (see Codegen) are breaking even when the JS surface looks unchanged — an app can ship new JS against an older cached native binary.
87+
88+
### Adding dependencies
89+
90+
When adding or changing any third-party reference — an npm dependency, a `.vscode/extensions.json` recommendation, a GitHub Action (`uses:`), or a native dependency (Podfile / Gemfile / Gradle) — verify it is published by its legitimate owner **before** referencing it. The tool being real is not enough: the *namespace* must be one the project trusts. Prefer Sentry's own scope/org (`@sentry/*`, `getsentry/*`), then the artifact's documented official publisher, and pin to an exact version / full commit SHA rather than a floating tag. Treat an unscoped or unfamiliar-publisher name as a supply-chain risk until proven otherwise — a claimable namespace lets an attacker ship code to every contributor. Provenance is necessary but not sufficient: a legitimate owner's account or CI can be compromised and ship a malicious *version*, so pin to an immutable ref (lockfile integrity hash / full SHA) and check advisories for that specific version, not just the publisher. See [references/supply-chain.md](references/supply-chain.md) for the surface-by-surface checklist and verification commands.
91+
92+
## File Organization
93+
94+
**Group by feature, not by type.** A processor or helper a feature owns lives with that feature (the TurboModule files sit together as `turbomodule/` + `integrations/turboModuleContext*.ts`). The `integrations/` directory is a **type-bucket** — it collects things that share the `Integration` type. Don't treat "it's an integration" as the whole home for a cohesive subsystem; keep the subsystem's own logic together and let the integration file be thin wiring.
95+
96+
Native code lives under `packages/core/android/` and `packages/core/ios/` and is reached only through the `NATIVE` wrapper seam — features *call* the bridge, they don't embed native code.
97+
98+
*Where* a given piece goes is a locality judgment — see **design-first**.
99+
100+
## TypeScript & API style
101+
102+
Shape the public surface deliberately (see **design-first** for module shape). In an SDK these matter more than in app code:
103+
104+
- **Private by default.** Don't export a symbol from the barrel unless it's genuinely part of the SDK's API — every export is a breaking-change liability and shows up in the API report.
105+
- **Explicit types on public functions** — annotate parameters and return types; don't rely on inference across the API boundary.
106+
- **`unknown` over `any`.** Narrow with type guards; `any` disables the checker exactly where SDK robustness matters.
107+
- **Prefer `interface` for object shapes**; use `type` for unions/mapped types.
108+
- **Optional chaining / nullish coalescing** (`?.`, `??`) over hand-rolled truthiness — but remember `??` and `?.` treat `0`/`''`/`false` correctly where `||` doesn't.
109+
- **Re-throw, don't wrap-and-rethrow.** Preserve the original error and its stack; when adding context, attach a `cause` rather than replacing the error — stack-trace fidelity is the product.
110+
- **No cross-boundary deep imports.** Import from `@sentry/core` / `@sentry/browser` public entry points, not their internal paths; keep RN-internal imports within `src/js`.
111+
112+
Style enforced by ESLint/oxlint/Prettier (single quotes, trailing commas, 120 cols, arrow-paren avoidance, import ordering) is **not** restated here — don't flag what `yarn lint` / `yarn fix` already handles. See `packages/core/AGENTS.md` for the list.
113+
114+
## Documentation comments
115+
116+
Prefer self-documenting code; comment for the two cases that earn it:
117+
118+
- **Public APIs** — JSDoc for every exported symbol and option, written for users who can't see the implementation.
119+
- **Non-obvious *why*** — workarounds, ordering constraints, native-bridge quirks, RN-version differences. The reasoning, not the play-by-play.
120+
121+
Don't narrate obvious behavior or restate the code. Use `@deprecated`, `@internal`, and `@hidden` deliberately — `@internal`/`@hidden` keep a symbol out of the generated API surface even when it must be exported for cross-file use.
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
# Native bridge & codegen gotchas
2+
3+
Reference for the JS↔native seam. Loaded by **code-guidelines** and cited by **review**'s Correctness axis. The living, file-level detail is in [`packages/core/AGENTS.md`](../../../../packages/core/AGENTS.md) — this doc is the durable checklist.
4+
5+
## The two architectures
6+
7+
React Native ships two module systems and this SDK supports both:
8+
9+
- **New Architecture** — JSI / TurboModules (JS spec in `src/js/NativeRNSentry.ts`) + Fabric components. Codegen generates the C++/Java/ObjC glue from the TS spec. Native side: `ios/RNSentry.mm`, `android/src/newarch/`.
10+
- **Old Architecture** — the async bridge, `NativeModules.RNSentry`. Native side: `android/src/oldarch/`. Legacy auto-instrumentation of third-party modules is opt-in (`enableLegacyNativeModules`) and best-effort.
11+
12+
A change is only "done" when it works on **both** archs. `Platform`/arch branches (`isTurboModuleEnabled()`, `isFabricEnabled()`) are the usual seams — test both branches.
13+
14+
## Codegen type vocabulary (New Arch)
15+
16+
Codegen only understands a narrow set of types in a `TurboModule` / component spec:
17+
18+
- Primitives: `boolean`, `number`, `string`
19+
- `Object` — an **untyped** map (opaque `NSDictionary`/`ReadableMap`); the shape is *not* checked
20+
- Arrays of the above
21+
- Nullability via `?` on the property/param
22+
- `Promise<T>` return
23+
24+
Not supported: literal unions, enums, generics, `Record<K, V>`, tuples, discriminated unions, function-valued props (except event callbacks on components), `undefined` as a value. If a method needs a richer shape, declare the param as `Object` and validate/normalize on the native side — the type system will *not* catch a mismatch for you.
25+
26+
## Serialization across the bridge
27+
28+
Everything crossing the bridge is serialized to a JSON-ish value:
29+
30+
- **No** functions, class instances, `Symbol`, `BigInt`, `undefined` holes, circular references, or `Map`/`Set`. These are dropped, throw, or arrive mangled.
31+
- `undefined` object properties do not survive; use `null` or omit the key.
32+
- Large payloads (envelopes, screenshots, replay frames) cross as base64/byte arrays — mind the copy cost and size limits.
33+
- Always confirm the value **round-trips**: what you send is what native receives and what comes back parses. Add a serialization test for any new payload shape.
34+
35+
## ABI / versioning
36+
37+
- Any edit to `src/js/NativeRNSentry.ts` (or a `*NativeComponent.ts` spec) is a **bridge ABI change**. It must land in lockstep in the JS spec, `ios/RNSentry.mm`, and `android/.../RNSentryModuleImpl`, and must be **backward compatible**: a user can ship new JS against an older cached native binary. New methods are safe; changing an existing method's signature/semantics is breaking.
38+
- Treat spec changes like public-API changes — they belong in the changelog and may be semver-major.
39+
40+
## Memory & crash safety
41+
42+
- **Android:** release JNI local refs; resolve *or* reject every `Promise` on every path (a leaked Promise hangs the JS caller). Never let a native exception propagate uncaught across the bridge.
43+
- **iOS:** avoid retain cycles in blocks (`__weak` self); honor nullability annotations; route hybrid-SDK access through `RNSentryInternal`.
44+
- A native crash caused by the SDK crashes the **host app** — the highest-severity failure this SDK can cause. Catch at the bridge boundary; degrade, don't crash.
45+
46+
## Privacy at the bridge
47+
48+
Data flowing from native (device context, breadcrumbs, module/method names, deep-link URLs) must respect `sendDefaultPii`. Any always-on collection needs an explicit, documented justification (see `packages/core/AGENTS.md` "Privacy" for the TurboModule exception and why it exists).
49+
50+
## Quick checklist for a bridge change
51+
52+
- [ ] Works on New **and** Old Architecture (both branches exercised)
53+
- [ ] Spec change (if any) landed in JS + iOS + Android in lockstep, backward-compatible
54+
- [ ] Payload is codegen-expressible or validated as `Object` on the native side
55+
- [ ] Payload round-trips (serialization test added)
56+
- [ ] Native failure is caught at the boundary — no host-app crash, no leaked Promise, no leaked native memory
57+
- [ ] PII gated on `sendDefaultPii` (or documented exception)
58+
- [ ] Graceful fallback when `enableNative` is false / module not linked

0 commit comments

Comments
 (0)