Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions apps/desktop/src/main/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as Sentry from "@sentry/electron/main";
import { captureException } from "@vendor/observability/sentry-electron-main";
import {
app,
BrowserWindow,
Expand Down Expand Up @@ -82,17 +82,15 @@ function forwardRendererErrorToSentry(payload: unknown): void {
if (!isRendererErrorPayload(payload)) {
return;
}
// The renderer-side @sentry/electron/renderer SDK silently fails to register
// a client (v10 carrier shape). Bridge renderer errors through the working
// main-side SDK instead, preserving the renderer stack so debug-id-paired
// sourcemaps can still symbolicate it.
// Bridge renderer errors through the main-side Sentry SDK, preserving the
// renderer stack so debug-id-paired sourcemaps still symbolicate.
const error = new Error(payload.message);
error.name =
payload.kind === "unhandledrejection" ? "UnhandledRejection" : "Error";
if (payload.stack) {
error.stack = payload.stack;
}
Sentry.captureException(error, {
captureException(error, {
tags: { bundle: "renderer", rendererKind: payload.kind },
extra: { source: payload.source, url: payload.url },
});
Expand Down
10 changes: 6 additions & 4 deletions apps/desktop/src/renderer/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@ declare global {
}

// Renderer errors are forwarded over IPC to main, which calls
// Sentry.captureException via the working `@sentry/electron/main` SDK. The
// renderer-side `@sentry/electron/renderer` path was broken — `Sentry.init`
// silently failed to register a client in the v10 carrier — so events never
// reached the IPC transport regardless of CSP setup.
// captureException via `@vendor/observability/sentry-electron-main`. Single
// SDK init in main keeps the bundle small and avoids per-process Sentry
// config; the renderer doesn't need to know about Sentry. Trade-off: no
// automatic renderer-side breadcrumbs / page-navigation tracking / future
// replay — to add those, expose a `sentry-electron-renderer` re-export from
// `@vendor/observability` and call `init` here.
installErrorBoundary(window.lightfastBridge.reportError);

const { buildInfo, platform } = window.lightfastBridge;
Expand Down
69 changes: 68 additions & 1 deletion thoughts/shared/2026-05-06-desktop-rc1-ad-hoc-dry-run-report.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ prs:

Cutting four release candidates uncovered seven distinct release-pipeline bugs that would have shipped silently on the first developer-id build. All seven are fixed and merged. The ad-hoc workflow now runs green end-to-end, produces a launchable signed-equivalent `.app`, and registers a Sentry release with paired-debug-id sourcemaps. The renderer-error → Sentry bridge is the only piece that's not directly observable today (Sentry org is over its free-tier quota), but every link in the chain up to and including `Sentry.captureException` execution is verified.

> **Correction 2026-05-06.** Bug F's root-cause diagnosis below — "renderer-side `@sentry/electron/renderer` `Sentry.init` is a silent no-op in the v10 carrier" — is **wrong**. A follow-up experiment on `@sentry/electron@7.13.0` with the renderer SDK restored shows `Sentry.getClient()` returns a fully constructed client and `__SENTRY__["10.50.0"].defaultCurrentScope._client` carries the configured DSN/release/transport. The renderer SDK works fine — the v10 carrier just exposes the active client at `defaultCurrentScope._client` instead of the `slot.client` / `slot.defaultClient` paths I was inspecting (those are v8/v9 carrier shapes). The "zero events ingested" outcome on rc.1/rc.2/rc.3 was almost certainly the Sentry org quota exhaustion, not a broken SDK. **PR #643's main-side bridge still stands as the chosen architecture** — simpler, smaller bundle, single SDK init — but it's an architectural choice, not a workaround for an SDK bug. See §"Correction" near the end of this report for the full investigation.

## What this dry-run was for

Per the plan: cut `@lightfast/desktop@0.1.0-rc.1` in ad-hoc mode to exercise ~90% of the release pipeline before Apple Developer enrollment unblocks. The bet was that latent bugs in the pipeline would surface on real tags, not on local `pnpm package` smoke-runs. That bet paid out — the local run was green for every fix that later shipped, but seven bugs only surfaced after pushing real tags.
Expand Down Expand Up @@ -134,8 +136,73 @@ Once Apple Developer enrollment lands:

- **Sentry quota.** Org is on free tier and over quota. New events get accepted into stats but not into searchable issues until quota restores. Doesn't affect the release pipeline; affects observability of any error post-release.
- **Bug D (`LIGHTFAST_REMOTE_DEBUG_PORT` in packaged builds).** Intentional `if (!app.isPackaged)` gate. CDP via CLI flag works as a manual workaround. Can revisit if a documented dev-friendly debug path becomes important.
- **Bug F root cause.** Pinned the symptom (no client registered in v10 carrier) but not the underlying reason `Sentry.init` no-ops. Unblocked by the main-side bridge but worth filing upstream if it recurs in a future SDK upgrade.
- **Bug F root cause.** ~~Pinned the symptom (no client registered in v10 carrier) but not the underlying reason `Sentry.init` no-ops. Unblocked by the main-side bridge but worth filing upstream if it recurs in a future SDK upgrade.~~ **See §Correction below — the renderer SDK was not broken; my carrier-shape inspection was looking at v8/v9 field paths.**

## Plan vs. reality

Plan called for three phases (status update, codesign pre-fixes, rc.1 cut). Reality required four `rc.N` cuts to surface and fix everything. The plan's premise — that real tags surface bugs that local `pnpm package` doesn't — held: every one of the seven bugs above was invisible until a real tag pushed. The ad-hoc dry-run was the right call.

## Correction (Bug F was misdiagnosed)

After the dry-run wrapped, I ran a follow-up experiment on a throwaway branch to validate the upstream issue I was about to file. Bumped `@sentry/electron` to the latest 7.13.0, restored `@sentry/electron/renderer` + `@sentry/electron/preload` + `Sentry.init({...})` in the renderer, built locally, launched with CDP, and inspected the carrier. The result contradicted the dry-run diagnosis.

### What the experiment showed

```
__SENTRY__["10.50.0"].defaultCurrentScope._client = {
constructor.name: 'Zb', // minified Sentry browser client
_options: {
dsn: 'https://abc123def456@o4509.ingest.us.sentry.io/450...',
release: 'experiment-7.13',
environment: 'test',
transport: <fn>,
integrations: [...],
ipcNamespace: <set>,
enabled: true,
sendClientReports: true,
enableLogs: true
}
}
```

`Sentry.getClient()` in the renderer returns the same fully-constructed client. `Sentry.init` did register a client. The renderer SDK was always working — both on 7.13.0 and (almost certainly) on 7.11.0 during the dry-run.

### Where the misdiagnosis came from

During rc.1/rc.2/rc.3 investigations I was inspecting `__SENTRY__[ver].client` and `__SENTRY__[ver].defaultClient` and concluding "no client registered" when both were `undefined`. Those are v8/v9 carrier field names. **In v10 the active client lives on `defaultCurrentScope._client`**, not those top-level slots. Looking at the wrong paths produced a wrong answer that was internally consistent across three RCs because the answer was wrong in the same way every time.

### What actually caused "zero events" on rc.1/rc.2/rc.3

The Sentry org's free-tier quota — the same one flagged late in the dry-run (§Open items above). Events were transporting from the renderer SDK (and, after PR #643, from the main-side bridge) but the receiving end was rejecting at ingest. I conflated "no issue visible in Sentry UI" with "no client registered in renderer."

### Why PR #643's bridge still stands

The architectural choice is defensible on its own merits:

- **Smaller bundle** — renderer 421K vs ~525K with the renderer SDK; preload 2.5K vs ~30K with the IPC bridge.
- **Single SDK configuration site** — release, environment, tags configured once in `apps/desktop/src/main/sentry.ts`; renderer doesn't need to know about Sentry at all.
- **One source of truth for what gets transported** — `installErrorBoundary` already IPCs all uncaught errors and unhandled rejections to main; `forwardRendererErrorToSentry` captures them through the same code path that handles main-process errors.
- **Insulated from v10/v11 carrier shape changes** — the bridge has no dependency on SDK internals.

What we lose by not running the renderer SDK:

- Automatic breadcrumbs from renderer `console.*` calls
- Page-navigation tracking (irrelevant — the renderer is largely a shell over `app.lightfast.localhost`, which has its own Sentry)
- `BrowserTracingIntegration` and session replay (impossible without renderer SDK)
- Explicit `Sentry.captureMessage(...)` / `Sentry.setTag(...)` from renderer code (we don't currently use these; if needed later, route through IPC)

These features matter once we want session replay or a rich breadcrumb timeline. Until then, the bridge is enough.

### What changed in this correction

- This report's TL;DR carries a Correction callout pointing here.
- The Bug F entry's blanket claim "renderer SDK silently fails to register a client" is replaced by "we chose to bridge through main; the renderer SDK works."
- The "Open items" entry "Bug F root cause" is struck through.
- The corresponding G-12 row in [`thoughts/shared/research/2026-04-23-codex-vs-lightfast-desktop-production-gap.md`](research/2026-04-23-codex-vs-lightfast-desktop-production-gap.md) is amended with the same correction.
- No upstream issue filed.

### Follow-ups (non-blocking)

- Bump `@sentry/electron` from 7.11.0 → 7.13.0 in a separate PR (two minor versions behind; small win, no behavior change for our usage).
- Revisit the bridge architecture if/when we want session replay or rich renderer breadcrumbs in v0.2.0+. To restore the renderer SDK cleanly, add a `sentry-electron-renderer` re-export to `@vendor/observability` (mirroring the existing `sentry-electron-main` and `sentry-browser` wraps) so renderer code never imports `@sentry/electron/renderer` directly — that direct import would violate the repo's vendor-abstraction policy.
- Update the `installErrorBoundary` comment in `apps/desktop/src/renderer/src/main.ts` to remove the inaccurate "renderer SDK was broken" rationale.
Original file line number Diff line number Diff line change
Expand Up @@ -588,7 +588,7 @@ These were invisible to local `pnpm package`; only real tag pushes surfaced them
| **G-9** | sentry-cli rejects `/` in release id; `@lightfast/desktop@…` parsed as path | PR #639 — strip `@`, replace `/` with `-` in both `apps/desktop/scripts/upload-sourcemaps.mjs` and `apps/desktop/src/main/sentry.ts` |
| **G-10** | Vite library mode emits no `.map` files by default | PR #640 — `build.sourcemap: true` in all three `vite.{main,preload,renderer}.config.ts` |
| **G-11** | `sentry-cli sourcemaps inject` ran post-asar-pack; user-defined Forge `prePackage` hook fires *before* plugin-vite (opposite of expected) | PR #641 — switched to `packageAfterCopy` hook (runs after vite + after copy to staging, before asar pack); inject into staging dir, mirror back to source `.vite/` |
| **G-12** | `@sentry/electron/renderer` `Sentry.init` is a silent no-op in v10 carrier — `__SENTRY__["10.47.0"]` never registers a client. Root cause not pinned | PR #643 — bridge renderer errors through main's `@sentry/electron/main` SDK via `IpcChannels.rendererError` → `Sentry.captureException`. Renderer-side init dropped; preload `@sentry/electron/preload` import dropped; v10 deps removed |
| **G-12** | ~~`@sentry/electron/renderer` `Sentry.init` is a silent no-op in v10 carrier~~**misdiagnosed**; follow-up experiment on 7.13.0 showed the renderer SDK works fine. The v10 carrier exposes the active client at `defaultCurrentScope._client`, not the v8/v9 `.client` / `.defaultClient` paths I was inspecting. The "zero events" outcome was Sentry quota exhaustion, not a broken SDK. Architectural choice to bridge through main still stands on its own merits (smaller bundle, single SDK init); see dry-run report §"Correction". | PR #643 — bridge renderer errors through main's `@sentry/electron/main` SDK via `IpcChannels.rendererError` → `Sentry.captureException` |
| **G-13** | `sentry-cli sourcemaps upload --release X` does not auto-create release `X`; subsequent `releases finalize X` fails | PR #642 — restore explicit `sentry-cli releases new <release>` as first step before upload |
| **G-14** | Vite CJS Rollup strips `import.meta` to literal `{}` (no polyfill); `import.meta.url` and `import.meta.dirname` both crash on access | PR #641 — use `__dirname` in `apps/desktop/src/main/windows/factory.ts` (CJS-native), with biome-ignore comment |

Expand Down
Loading