Skip to content

fix(desktop): bridge renderer errors to main Sentry SDK#643

Merged
jeevanpillay merged 2 commits intomainfrom
feat/desktop-renderer-error-bridge
May 6, 2026
Merged

fix(desktop): bridge renderer errors to main Sentry SDK#643
jeevanpillay merged 2 commits intomainfrom
feat/desktop-renderer-error-bridge

Conversation

@jeevanpillay
Copy link
Copy Markdown
Member

@jeevanpillay jeevanpillay commented May 6, 2026

Summary

  • Renderer-side @sentry/electron/renderer silently fails to register a client in the v10 carrier — Sentry.init() is a no-op, so events captured in the renderer never reached the IPC transport. Org-level Sentry stats show zero ingestion across rc.1, rc.2, rc.3 despite all the observability hardening from fix(desktop): observability hardening — IPC routing + debug-id symbolication #641/fix(desktop): create Sentry release before finalize #642 landing cleanly.
  • Forward IpcChannels.rendererError payloads to Sentry.captureException on the main process, where @sentry/electron/main is fully working. Preserve the renderer stack so debug-id sourcemaps still symbolicate, tag events with bundle: "renderer" to keep them distinguishable from main exceptions.
  • Drop the dead renderer/preload Sentry plumbing (no-op renderer init, @sentry/electron/preload IPC bridge, getSentryInitOptionsSync channel, sentryInit bridge field) and the v10 deps it pulled in: @sentry/browser, @sentry-internal/{browser-utils,feedback,replay,replay-canvas}, @sentry/node, @sentry/core. Renderer bundle drops from ~525K → 421K; preload shrinks to 2.5K.

Why this is the fix

The previous PR (#641) wired the IPC bridge correctly — __SENTRY_IPC__ was exposed, no more CSP fetch errors — but __SENTRY__["10.47.0"] showed clientPresent: false, defaultClient: null, no acs key. Multiple investigations confirmed the renderer SDK loaded, but Sentry.init never registered a client. Bridging through main bypasses the broken renderer SDK entirely while keeping the user-facing pipeline (installErrorBoundary + lightfastBridge.reportError) unchanged.

Test plan

  • Local dev launches without errors (no missing sentryInit reads anywhere)
  • Tag @lightfast/desktop@0.1.0-rc.4 after merge, confirm workflow is green and 6 assets uploaded
  • Smoke-test rc.4 .dmg: open via open -a Lightfast after quarantine clear, then trigger uncaught throw via CDP --remote-debugging-port=9222 → verify Sentry receives an issue with bundle:renderer tag and a symbolicated stack pointing into source files (not minified bundle).
  • Repeat in main process by triggering an uncaught error and confirm bundle:electron host:app tags arrive too.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Refactor
    • Reorganized error tracking initialization in the desktop application, centralizing configuration and error routing in the main process.
    • Streamlined and consolidated package dependencies.

The renderer-side `@sentry/electron/renderer` path silently failed to
register a client in the v10 carrier — `Sentry.init()` was a no-op, so
captured events never reached the IPC transport regardless of the CSP
or preload bridge wiring. Org-level stats showed zero ingestion across
rc.1, rc.2, and rc.3 despite the rest of the observability hardening
landing cleanly.

Forward `IpcChannels.rendererError` payloads to `Sentry.captureException`
on the main process, where `@sentry/electron/main` is fully working.
Preserve the renderer stack so debug-id-paired sourcemaps still
symbolicate, and tag events with `bundle: "renderer"` to keep them
distinguishable from main-process exceptions.

Drop the dead renderer/preload Sentry plumbing and the v10 deps it
pulled in (`@sentry/browser`, `@sentry-internal/*`, `@sentry/node`,
`@sentry/core`). Renderer bundle drops from ~525K to 421K; preload
shrinks to 2.5K.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel Bot commented May 6, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
lightfast-app Ready Ready Preview, Comment May 6, 2026 7:47am
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
lightfast-platform Skipped Skipped May 6, 2026 7:47am
lightfast-www Skipped Skipped May 6, 2026 7:47am

Request Review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 6, 2026

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

Sentry initialization is centralized in the Electron main process using @sentry/electron. Renderer errors are now forwarded to Sentry via IPC instead of logged to console. Initialization code is removed from preload and renderer scripts, and IPC types for passing Sentry options are eliminated.

Changes

Sentry Main-Process Centralization

Layer / File(s) Summary
Dependency Update
apps/desktop/package.json
Sentry internal packages consolidated to single @sentry/electron ^7.11.0 dependency.
IPC Contract
apps/desktop/src/shared/ipc.ts
Removed SentryInitSnapshot type, getSentryInitOptionsSync channel, and sentryInit property from LightfastBridge; Sentry options no longer sync'd across processes.
Main Process Initialization & Error Routing
apps/desktop/src/main/index.ts
Added initSentry() call at startup. Replaced console error logging with forwardRendererErrorToSentry() that routes renderer crashes to Sentry with preserved stack traces and "renderer" tag. Removed sync IPC handler for getSentryInitOptionsSync.
Preload Cleanup
apps/desktop/src/preload/preload.ts
Removed Sentry preload bootstrap import and sentryInit from bridge payload.
Renderer Cleanup
apps/desktop/src/renderer/src/main.ts
Removed Sentry.init() call and sentryInit destructuring; renderer no longer initializes Sentry directly.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed Title follows conventional commits format with 'fix:' prefix, is 55 characters (under 70 limit), and accurately describes the core change: bridging renderer errors to the main Sentry SDK instead of renderer-side initialization.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/desktop-renderer-error-bridge
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch feat/desktop-renderer-error-bridge

Comment @coderabbitai help to get the list of available commands and usage tips.

Import order + line-length fixes flagged by CI Biome check.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@vercel vercel Bot temporarily deployed to Preview – lightfast-platform May 6, 2026 07:44 Inactive
@vercel vercel Bot temporarily deployed to Preview – lightfast-www May 6, 2026 07:44 Inactive
@vercel vercel Bot temporarily deployed to Preview – lightfast-app May 6, 2026 07:44 Inactive
@jeevanpillay jeevanpillay merged commit ac986f9 into main May 6, 2026
14 of 15 checks passed
@jeevanpillay jeevanpillay deleted the feat/desktop-renderer-error-bridge branch May 6, 2026 07:49
jeevanpillay added a commit that referenced this pull request May 6, 2026
CodeRabbit flagged the renderer comment recommending `@sentry/electron/renderer`
as a direct third-party SDK reference, which violates repo policy. Same policy
violation in apps/desktop/src/main/index.ts where PR #643 added a direct
`import * as Sentry from "@sentry/electron/main"` instead of routing through
the existing `@vendor/observability/sentry-electron-main` re-export
(which already exposes captureException).

- Switch main/index.ts to import { captureException } from the vendor wrap.
- Update the renderer comment to point at a hypothetical
  `@vendor/observability/sentry-electron-renderer` re-export rather than
  the underlying `@sentry/electron/renderer` SDK.
- Mirror the same guidance in the dry-run report's §Correction follow-ups.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant