Skip to content

refactor(sentry): deliver Node telemetry to the native SDK through the addon, not the control socket #244

Description

@gmaclennan

control.sock carries every @sentry/node error event and envelope from the embedded backend to the native SDK. On both platforms the consumer of those frames is in the same process as Node: on iOS it is SentryNativeBridge, and on Android it is SentryFgsBridge in the FGS, which owns sentry-android.

So the highest-volume traffic on the control socket is telemetry being serialized, framed, written to a socket, read, parsed and re-deserialized in order to reach a function that was always directly callable. This issue replaces that hop with two addon calls.

Depends on the addon introduced in #243.

Why not wrap the Sentry API

Worth stating explicitly, because it is the obvious first instinct and it is wrong here.

The payload already is the argument. sentry-event is routed to SentryEvent.Deserializer + Sentry.captureEvent precisely so native scope (device/OS/app/user) merges at capture time; sentry-envelope goes to InternalSentrySdk.captureEnvelope(bytes, false) for offline-capable transport, deliberately without native scope, because the parent transaction is opened natively and Node's spans inherit via Sentry.continueTrace. That split is already the right one.

Wrapping a native Sentry method surface instead would mean marshalling a JS Error into a native exception, and JS stack frames, source context, breadcrumbs, sampling decisions and the trace linkage have no native representation. Encoding them into a structured payload to get them across reproduces the envelope — minus @sentry/node's per-RPC spans and integration hooks. This is also why Sentry's own hybrid SDKs (React Native, Flutter, Capacitor) hand envelopes to the native transport rather than re-expressing events.

So: keep both payload formats and both entry points exactly as they are. Only the transport under them changes.

What this removes

The two Kotlin/Swift functions already have the right signatures, so the JS side keeps calling captureEventJson(String) and captureEnvelopeBase64(String) with byte-identical arguments. before-send.js, the scrubbers, the tripwire, sentry-frame.js's event-vs-envelope classification, and every tag and span in ARCHITECTURE §7.5 are untouched.

Deleted:

  • #recentSentryFrames in backend/lib/simple-rpc.js — the 100-frame replay ring, plus MAX_RECENT_SENTRY_FRAMES and the replay loop in #onConnection. It exists solely because on Android both the FGS and the main app process connect to control.sock and connect order is not guaranteed; the frames are replayed to a consumer (ComapeoCoreModule.kt) that explicitly discards them to avoid double-sending. That whole mechanism is in service of nothing.
  • ControlFrame.SentryEvent and ControlFrame.SentryEnvelope on both platforms, and their branches in handleControlMessage / the FGS when — including the empty is ControlFrame.SentryEvent -> {} discards in ComapeoCoreModule.kt.
  • The base64 round trip on envelopes. InternalSentrySdk.captureEnvelope wants bytes; today they are base64-encoded to survive JSON framing and decoded again on the other side. With a direct call the addon can pass a Buffer straight through.

preListenQueue in backend/lib/sentry.js stays, but shrinks in practice: the sink can be registered as soon as the addon is loaded rather than after controlIpcServer.listen(), so the window it covers is loader.mjs only.

Performance, and how much of it is real

Be honest about this, because the obvious framing oversells it.

tracesSampleRate is 0 unless the user's debug preference is on (src/sentry.ts:352), and no profiling is configured. So in production the envelope traffic is error events, sessions, and one forced-sampled boot transaction per launch — small and infrequent. The production throughput case for this change is weak; complexity removal is the real justification.

Where it does matter is debug mode, which flips tracesSampleRate to 1.0 and produces a transaction envelope per RPC method. That is precisely when someone is diagnosing a problem and the telemetry path should not be competing for the loop.

Two things are worth fixing regardless of volume.

The event path does four full JSON passes. JSON.stringify in JS, JSONObject(raw) on receipt, payload.toString() to re-serialize (ControlFrame.kt does this explicitly, because SentryEvent.Deserializer wants to re-parse "against the bytes it expects"), then the Deserializer parse. Passing the payload JSON straight through the addon makes it two. That is a fixed ~2x reduction on every captured error at any volume, and step three exists only because the frame parse and the SDK parse cannot share a representation across a wire.

The envelope path allocates ~10x the payload to move it. Counting materializations of an envelope of size E: serialized bytes (E), base64 string (1.33E), JSON.stringify result (1.33E), Buffer.from UTF-8 (1.33E), kernel buffer (1.33E), native read buffer (1.33E), Kotlin String (1.33E), optString (1.33E), Base64.decode byte array (E). The addon path is 2E. That transient pressure lands in the FGS process, where memory isolation is one of the three reasons the process exists at all (ARCHITECTURE §2.2).

Copy discipline

One copy is the floor. Do not chase zero.

N-API cannot steal a V8 backing store — napi_detach_arraybuffer detaches from JS but does not transfer ownership of the allocation, and there is no napi_take_arraybuffer. Borrowing is possible (napi_get_buffer_info yields a pointer that is stable, since external backing stores are not relocated by GC), but holding it past the call's return needs a napi_reference, and releasing that reference must happen on the JS thread — a TSFN round trip and a cross-thread lifetime hazard, bought for one memcpy.

On Android it would buy nothing anyway: InternalSentrySdk.captureEnvelope(bytes: ByteArray, false) takes a Java byte[], and native memory to Java heap is SetByteArrayRegion, which copies. NewDirectByteBuffer avoids it but Sentry does not accept a ByteBuffer. iOS could genuinely borrow via Data(bytesNoCopy:deallocator:), but PrivateSentrySDKOnly.envelope(with:) parses the bytes into an object immediately, so the borrow window is only the parse.

That surfaces the actual trade: a synchronous call is zero-copy but parses on Node's loop; a queued call costs one copy and keeps the loop free. Take the copy. Android needs it regardless, captureEnvelope may touch the envelope disk cache, and platform symmetry is worth more than one memcpy.

So the reductions to make are the ones that need no cleverness:

  • Drop base64 (removes the 1.33x inflation and two transform passes).
  • Drop the JSON wrapper — the frame no longer crosses a wire, so it need not be JSON-serializable. Pass type and payload as separate arguments.
  • Pass a Buffer, not a JS string, for the event JSON: napi_get_buffer_info is a pointer fetch, whereas napi_get_value_string_utf8 needs a length-query pass and then a copy pass.
  • Copy once, as early as possible — straight into the JNI byte[] or Data inside the addon, then return. Do not hold N-API references across the async boundary.

Cost

Sentry frames must not block Node's loop thread. Today sink() is a buffered postMessage that returns immediately. As a direct call it becomes JNI or Swift, and envelopes can be large — profiles especially. The addon must hand off to a Kotlin coroutine or a GCD utility queue and return, fire-and-forget. This is a real behaviour change and needs to be deliberate: a synchronous implementation would put keystore-scale latency on every captured event.

Backpressure disappears with the socket. Today an overwhelmed consumer eventually shows up as a full socket buffer. A fire-and-forget queue can grow unbounded. Bound it and count drops on the existing telemetryForwardingFailure metric.

Plan

1. Addon surface

Two functions on the bridge from #243, both fire-and-forget, both returning immediately:

captureSentryEvent(json)        // string — a single-item error-event envelope's payload
captureSentryEnvelope(bytes)    // Buffer — no base64

Unlike the key-store calls these are not napi_create_async_work — there is no result to await. They copy the payload, enqueue it natively, and return. Copy rather than retain the Buffer: Node may reuse the backing store as soon as the call returns.

2. Native entry points

Android: a @JvmStatic pair that hands to serviceScope.launch(Dispatchers.IO) and then calls the existing SentryFgsBridge.captureEventJson / .captureEnvelopeBase64. Add a byte-array overload of the latter so the base64 round trip can be dropped — InternalSentrySdk.captureEnvelope already takes bytes.

iOS: @_cdecl functions that DispatchQueue.global(qos: .utility).async into SentryNativeBridge.captureEventJson / .captureEnvelopeBase64, with the same byte-array addition.

Both queues want a bounded depth. On overflow, drop the oldest and count telemetryForwardingFailure, matching what preListenQueue already does.

3. Backend wiring

In backend/index.js, replace

sentry.setSink((frame) => controlIpcServer.broadcast(frame));

with a sink that dispatches on frame type to the two addon calls, and move the setSink call earlier — it no longer has to wait for controlIpcServer.listen().

backend/lib/sentry-frame.js keeps its classification logic but stops base64-encoding the envelope: sentry-envelope becomes { type: "sentry-envelope", data: Uint8Array } internally. Since the frame no longer crosses a wire it does not need to be JSON-serializable at all, which is worth simplifying rather than preserving.

Remove the sentry-event / sentry-envelope branch from SimpleRpcServer.broadcast.

4. Native cleanup

Delete the two ControlFrame cases and their parse branches on both platforms, and the discard branches in ComapeoCoreModule.kt.

Testing

  • ControlFrameTests.swift and ControlFrameTest.kt lose their Sentry-frame cases; the remaining lifecycle cases stay.
  • simple-rpc.test.mjs loses the replay-ring tests.
  • New: a backend unit test that a captured error and a captured transaction reach the faked bridge with the expected payload shape, and that the sink is registered before listen.
  • New: a device test per platform asserting an event captured in Node reaches the native SDK — assert on the native SDK's beforeSend seeing it rather than on network egress.
  • Confirm the tripwire and scrubber suites still pass unchanged. They should, since payloads are byte-identical; if they do not, something moved that should not have.

Risks

Envelope size on the addon boundary. Profiles can be megabytes. Measure a profile-carrying envelope end to end and confirm the copy plus enqueue does not stall the loop; if it does, the copy should happen on the native side of an async work item after all.

Ordering. The socket gave FIFO for free. A single-consumer queue per platform preserves it; anything fan-out does not. Keep it single-consumer.

Loss on crash. Frames sitting in the native queue when the process dies are lost, where socket-buffered frames had at least reached the consumer's kernel buffer. In practice both are lost on a hard crash, and §7.4 already says native crashes inside the runtime are out of scope — but the queue should be shallow enough not to widen the window meaningfully.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions