fix(desktop): observability hardening — IPC routing + debug-id symbolication#641
Merged
jeevanpillay merged 1 commit intomainfrom May 6, 2026
Merged
Conversation
…ication
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=<uuid>` 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) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughSentry integration improvements for the Electron desktop app: added post-package debug-ID injection via Forge hook, modernized sourcemap uploads to artifact-bundle flow, switched Sentry imports to Electron-specific modules, and refactored path resolution to use ChangesSentry Integration & Sourcemaps
Sequence DiagramsequenceDiagram
participant Forge as Build System<br/>(Forge)
participant CLI as Sentry CLI
participant FS as Filesystem
participant Upload as Upload Script
participant Sentry as Sentry Backend
Forge->>Forge: Package app with Vite output
Forge->>CLI: inject sourcemaps --release
CLI->>FS: Read staged build artifacts
CLI->>FS: Inject debug IDs into .js/.map files
CLI->>FS: Write updated files back to .vite
Forge->>FS: Build complete
Upload->>Upload: Get build & renderer dirs
Upload->>CLI: sourcemaps upload --release buildDir
CLI->>Sentry: Upload with embedded debug IDs
Sentry-->>CLI: ✓ Registered
Upload->>CLI: sourcemaps upload --release rendererDir
CLI->>Sentry: Upload with embedded debug IDs
Sentry-->>CLI: ✓ Registered
Upload->>Upload: finalize release
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
This was referenced May 6, 2026
jeevanpillay
added a commit
that referenced
this pull request
May 6, 2026
Pull in 5 desktop release PRs that landed since fork: - #638 codesign pre-fixes - #639 release-pipeline fixes - #640 Vite sourcemaps - #641 observability hardening (IPC routing + debug-id symbolication) - #642 Sentry release/finalize fix Conflicts resolved: - apps/desktop/src/main/windows/factory.ts — comment-only divergence on the Vite/CJS __dirname workaround. Took main's more detailed wording. - apps/desktop/src/renderer/src/main.ts — main removed renderer-side Sentry init (the v10 SDK silently no-op'd; events forward via IPC to main now). Our HEAD's initSentryBrowser block referenced sentryInit which main also dropped. Took main's version. - pnpm-lock.yaml — regenerated against the merged package.json files. Verified: 38/38 desktop tests pass; @lightfast/desktop and @lightfast/app typechecks clean. Live PKCE sign-in re-verification deferred to post-commit step.
2 tasks
4 tasks
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
1 participant
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
The rc.2 ad-hoc dry-run smoke test surfaced three latent issues that together prevented Sentry from receiving and symbolicating any renderer events. All fixed here so rc.3 can produce real symbolicated stacks end-to-end.
F1 — Renderer SDK was bypassing the IPC bridge
`apps/desktop/src/renderer/src/main.ts` imported `@sentry/browser` and called `Sentry.init` directly. The browser SDK fetches `https://o…ingest.us.sentry.io/api/…/envelope\`, which the renderer CSP blocks → events silently drop. Console showed `Fetch API cannot load https://…ingest.us.sentry.io/…` for every uncaught error.
Fix: swap to `@sentry/electron/renderer` and add `import "@sentry/electron/preload"` to `apps/desktop/src/preload/preload.ts`. The preload import installs the IPC bridge on `contextBridge`; the renderer SDK detects the bridge and routes events through main via the `sentry-ipc:` scheme (registered by `@sentry/electron/main` with `bypassCsp: true`). Main's Node HTTP transport has no CSP. Events deliver.
E — No debug-ids in the shipped bundle
`scripts/upload-sourcemaps.mjs` used the legacy `releases files upload-sourcemaps --url-prefix app:///` flow, which relies on Sentry matching runtime frame URLs to uploaded asset URLs. The runtime URL was `app:///.vite/renderer/main_window/assets/index-.js` but the upload was `app:///assets/index-.js` — mismatch, no symbolication. Worse: `grep debugId` against the rc.2 asar returned 0 matches in every JS file — the bundles had no `//# debugId=` comments at all, so even the modern artifact-bundle path had nothing to match on.
Fix: switch to the modern `sourcemaps inject` + `sourcemaps upload --release` flow. A new Forge `packageAfterCopy` hook calls `sentry-cli sourcemaps inject` on the staging dir (after plugin-vite's build, before asar pack), then mirrors the modified files back to source `.vite/` so the workflow's later upload step uploads debug-id-bearing maps. Sentry resolves stack frames by debug-id, sidestepping URL conventions entirely.
`prePackage` was the wrong hook — Forge runs user hooks before plugin-vite's build, so `.vite/` doesn't exist yet at that point. `packageAfterCopy` fires after staging, with the temp build path passed in.
Crash regression — main bundle threw on launch
Verifying the local build I hit `TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string … Received undefined` at `fileURLToPath` before `app.ready`. Source was `apps/desktop/src/main/windows/factory.ts:15` — `dirname(fileURLToPath(import.meta.url))`. In our Vite CJS output, Rollup strips `import.meta` to `{}`, so `fileURLToPath({}.url) === fileURLToPath(undefined)` throws. (rc.2 worked because Rollup polyfilled `import.meta.url` to `typeof document>"u"?require("./bootstrap.js").url:document.currentScript.url`; something between rc.2 and now stopped that — likely fallout from `sourcemap: true` in PR #640.)
Fix: `__dirname` (CJS-native, reliable) with a `biome-ignore` for `noGlobalDirnameFilename`. Biome's suggested fix `import.meta.dirname` gets stripped to `{}.dirname` the same way — also broken.
Test plan
Bug-trail
Tracking the dry-run finds:
Summary by CodeRabbit
New Features
Chores