From d826bfce5020019c5cc8dea20173c99404b922e9 Mon Sep 17 00:00:00 2001 From: Jeevan Pillay <169354619+jeevanpillay@users.noreply.github.com> Date: Wed, 6 May 2026 16:32:48 +1000 Subject: [PATCH] =?UTF-8?q?fix(desktop):=20observability=20hardening=20?= =?UTF-8?q?=E2=80=94=20IPC=20routing=20+=20debug-id=20symbolication?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rc.2 ad-hoc dry-run smoke test surfaced three more latent issues that together prevented Sentry from receiving and symbolicating any renderer events. Fixes: Renderer SDK swap. apps/desktop/src/renderer/src/main.ts was importing @sentry/browser and calling Sentry.init directly — that SDK fetches the Sentry ingest URL, which the renderer CSP blocks. Switched to @sentry/electron/renderer, which auto-detects the IPC bridge installed by @sentry/electron/main + the new @sentry/electron/preload import in preload.ts. Events now route through main via the `sentry-ipc:` scheme (registered with bypassCsp by @sentry/electron) instead of fetch. Debug-id injection in the asar. The previous flow (sentry-cli releases files upload-sourcemaps with --url-prefix) relied on Sentry matching runtime stack frame URLs to uploaded asset URLs — fragile across the Vite/Forge/asar path conventions. Switched to the modern artifact-bundle flow with debug-ids: a Forge `packageAfterCopy` hook calls `sentry-cli sourcemaps inject` against the staging dir AFTER plugin-vite's build but BEFORE asar packing, so each .js gets `//# debugId=` and the matching .map file gets the same id baked in. The hook then mirrors the modified files back to the source `.vite/` dir so the post-build upload-sourcemaps.mjs uploads files with debug-ids that match the asar. Sentry resolves stack frames by debug-id without caring about URL matching at all. (User-defined `prePackage` hooks run BEFORE plugin-vite in Forge's hook order, so prePackage was the wrong lifecycle event.) Main bundle crash on launch. factory.ts:15 used `dirname(fileURLToPath(import.meta.url))`. Rollup in our Vite CJS output strips `import.meta` to `{}`, so `fileURLToPath({}.url) = fileURLToPath(undefined)` threw and the app crashed before app.ready. Switched to __dirname (CJS-native, polyfilled correctly by Vite) with a biome-ignore for the noGlobalDirnameFilename rule — the rule's suggested fix (`import.meta.dirname`) gets stripped the same way and is also broken in CJS output. Co-Authored-By: Claude Opus 4.7 (1M context) --- apps/desktop/forge.config.ts | 38 ++++++++++++++++++++++ apps/desktop/scripts/upload-sourcemaps.mjs | 36 +++++--------------- apps/desktop/src/main/windows/factory.ts | 11 +++++-- apps/desktop/src/preload/preload.ts | 5 +++ apps/desktop/src/renderer/src/main.ts | 7 +++- 5 files changed, 65 insertions(+), 32 deletions(-) diff --git a/apps/desktop/forge.config.ts b/apps/desktop/forge.config.ts index 5f2b1ed66..d443266db 100644 --- a/apps/desktop/forge.config.ts +++ b/apps/desktop/forge.config.ts @@ -1,3 +1,5 @@ +import { execFileSync } from "node:child_process"; +import { cpSync, existsSync } from "node:fs"; import { resolve } from "node:path"; import { FuseV1Options, FuseVersion } from "@electron/fuses"; import { MakerDMG } from "@electron-forge/maker-dmg"; @@ -69,7 +71,43 @@ const githubPublisher = process.env.GITHUB_TOKEN }) : null; +// Inject sentry debug-ids into the staged Vite output AFTER electron-packager +// has copied it to its temp build path. We can't use prePackage because +// Forge runs user hooks before plugin-vite's build, so .vite/ doesn't exist +// yet. We can't use any earlier post-Vite hook because plugin-vite doesn't +// expose one. packageAfterCopy fires once per platform/arch with `buildPath` +// pointing at the staging dir whose contents will be sealed into the asar; +// inject there, then mirror the modified files back to the source `.vite/` +// so `scripts/upload-sourcemaps.mjs` uploads sourcemaps with debug-ids that +// match what got packed. +function injectSentryDebugIds(buildPath: string, sourceRoot: string): void { + if (!process.env.SENTRY_AUTH_TOKEN) { + return; + } + const targets = [".vite/build", ".vite/renderer/main_window"]; + for (const t of targets) { + const stagingDir = resolve(buildPath, t); + const sourceDir = resolve(sourceRoot, t); + if (!existsSync(stagingDir)) { + continue; + } + execFileSync( + "pnpm", + ["exec", "sentry-cli", "sourcemaps", "inject", stagingDir], + { cwd: sourceRoot, stdio: "inherit" } + ); + if (existsSync(sourceDir)) { + cpSync(stagingDir, sourceDir, { recursive: true, force: true }); + } + } +} + const config: ForgeConfig = { + hooks: { + packageAfterCopy: async (_forgeConfig, buildPath) => { + injectSentryDebugIds(buildPath, import.meta.dirname); + }, + }, packagerConfig: { name: "Lightfast", executableName: "lightfast", diff --git a/apps/desktop/scripts/upload-sourcemaps.mjs b/apps/desktop/scripts/upload-sourcemaps.mjs index 814f54f56..67afb0ce5 100755 --- a/apps/desktop/scripts/upload-sourcemaps.mjs +++ b/apps/desktop/scripts/upload-sourcemaps.mjs @@ -26,7 +26,6 @@ for (const name of required) { // `apps/desktop/src/main/sentry.ts`; keep both in sync. const releaseName = pkg.name.replace(/^@/, "").replace("/", "-"); const release = `${releaseName}@${pkg.version}+${pkg.buildNumber}`; -const urlPrefix = "app:///"; const buildDir = resolve(desktopRoot, ".vite/build"); const rendererDir = resolve(desktopRoot, ".vite/renderer/main_window"); @@ -38,33 +37,14 @@ function sentry(args) { }); } -sentry(["releases", "new", release]); -sentry([ - "releases", - "files", - release, - "upload-sourcemaps", - "--url-prefix", - urlPrefix, - "--ext", - "js", - "--ext", - "map", - buildDir, -]); -sentry([ - "releases", - "files", - release, - "upload-sourcemaps", - "--url-prefix", - urlPrefix, - "--ext", - "js", - "--ext", - "map", - rendererDir, -]); +// Modern artifact-bundle flow with debug-id matching. `sourcemaps inject` +// runs in `forge.config.ts`'s prePackage hook so the injected //# debugId= +// comments land in the asar; here we only `upload`. Stack frames in Sentry +// resolve via debug-id, which avoids URL-prefix mismatches between the +// uploaded path (`assets/index-*.js`) and the runtime frame +// (`app:///.vite/renderer/main_window/assets/index-*.js`). +sentry(["sourcemaps", "upload", "--release", release, buildDir]); +sentry(["sourcemaps", "upload", "--release", release, rendererDir]); sentry(["releases", "finalize", release]); console.log(`Uploaded sourcemaps for release ${release}`); diff --git a/apps/desktop/src/main/windows/factory.ts b/apps/desktop/src/main/windows/factory.ts index 76685390b..af88fda95 100644 --- a/apps/desktop/src/main/windows/factory.ts +++ b/apps/desktop/src/main/windows/factory.ts @@ -1,5 +1,4 @@ -import { dirname, join } from "node:path"; -import { fileURLToPath } from "node:url"; +import { join } from "node:path"; import { BrowserWindow, type BrowserWindowConstructorOptions, @@ -12,7 +11,13 @@ import { loadWindowState, trackWindowState } from "../window-state"; declare const MAIN_WINDOW_VITE_DEV_SERVER_URL: string | undefined; declare const MAIN_WINDOW_VITE_NAME: string; -const factoryDir = dirname(fileURLToPath(import.meta.url)); +// Vite emits the main bundle as CJS and Rollup does not polyfill +// `import.meta` — both `.url` and `.dirname` get stripped to `{}.` = +// undefined, which crashes downstream `fileURLToPath(undefined)`. `__dirname` +// is CJS-native and the only reliable way to get the bundle directory inside +// the asar at runtime. +// biome-ignore lint/correctness/noGlobalDirnameFilename: Vite CJS output strips import.meta.*; __dirname is the only working option here. +const factoryDir = __dirname; const PRELOAD_PATH = join(factoryDir, "preload.js"); const RENDERER_DIST = join(factoryDir, `../renderer/${MAIN_WINDOW_VITE_NAME}`); diff --git a/apps/desktop/src/preload/preload.ts b/apps/desktop/src/preload/preload.ts index 140ef1593..e6df4776f 100644 --- a/apps/desktop/src/preload/preload.ts +++ b/apps/desktop/src/preload/preload.ts @@ -1,3 +1,8 @@ +// Installs the @sentry/electron IPC bridge on the contextBridge so the +// renderer-side SDK routes events through main via `sentry-ipc:` instead of +// fetching the ingest URL directly (which the renderer CSP blocks). Pair: +// src/main/sentry.ts (main init) and src/renderer/src/main.ts (renderer init). +import "@sentry/electron/preload"; import { contextBridge, type IpcRendererEvent, ipcRenderer } from "electron"; import type { AcceleratorName } from "../shared/accelerators"; import { diff --git a/apps/desktop/src/renderer/src/main.ts b/apps/desktop/src/renderer/src/main.ts index 430a15c56..fc59e1412 100644 --- a/apps/desktop/src/renderer/src/main.ts +++ b/apps/desktop/src/renderer/src/main.ts @@ -1,4 +1,9 @@ -import * as Sentry from "@sentry/browser"; +// @sentry/electron/renderer routes events through `sentry-ipc:` (CSP-bypass +// scheme registered by @sentry/electron/main + bridged by the +// `@sentry/electron/preload` import in src/preload/preload.ts). The plain +// @sentry/browser SDK fetches the ingest URL directly, which the renderer +// CSP blocks — events silently drop. +import * as Sentry from "@sentry/electron/renderer"; import "./react/entry"; import { ACCELERATORS,