feat: Sentry production readiness — source maps, crash reporter, IPC wrapper #1869
There are no files selected for viewing
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -111,6 +111,7 @@ | |
| "@electron-toolkit/utils": "^4.0.0", | ||
| "@electron/rebuild": "^4.0.2", | ||
| "@playwright/test": "^1.52.0", | ||
| "@sentry/vite-plugin": "^4.9.1", | ||
| "@tailwindcss/postcss": "^4.1.17", | ||
| "@testing-library/dom": "^10.0.0", | ||
| "@testing-library/jest-dom": "^6.9.1", | ||
|
|
||
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
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,81 @@ | ||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||
| * Tests for Sentry utility functions | ||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| import { describe, it, expect, vi, beforeEach } from 'vitest'; | ||||||||||||||||||||||||
| import type { IpcMainInvokeEvent } from 'electron'; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| const mockCaptureException = vi.fn(); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| vi.mock('@sentry/electron/main', () => ({ | ||||||||||||||||||||||||
| init: vi.fn(), | ||||||||||||||||||||||||
| addBreadcrumb: vi.fn(), | ||||||||||||||||||||||||
| captureException: mockCaptureException, | ||||||||||||||||||||||||
| })); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| vi.mock('electron', () => ({ | ||||||||||||||||||||||||
| app: { isPackaged: false, getVersion: () => '0.0.0-test' }, | ||||||||||||||||||||||||
| ipcMain: { on: vi.fn(), handle: vi.fn() }, | ||||||||||||||||||||||||
| crashReporter: { start: vi.fn() }, | ||||||||||||||||||||||||
| })); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| vi.mock('../settings-utils', () => ({ | ||||||||||||||||||||||||
| readSettingsFile: () => ({}), | ||||||||||||||||||||||||
| })); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| vi.mock('../../shared/constants', () => ({ | ||||||||||||||||||||||||
| DEFAULT_APP_SETTINGS: { sentryEnabled: true }, | ||||||||||||||||||||||||
| })); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| vi.mock('../../shared/constants/ipc', () => ({ | ||||||||||||||||||||||||
| IPC_CHANNELS: { | ||||||||||||||||||||||||
| SENTRY_STATE_CHANGED: 'sentry-state-changed', | ||||||||||||||||||||||||
| GET_SENTRY_DSN: 'get-sentry-dsn', | ||||||||||||||||||||||||
| GET_SENTRY_CONFIG: 'get-sentry-config', | ||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||
| })); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| vi.mock('../../shared/utils/sentry-privacy', () => ({ | ||||||||||||||||||||||||
| processEvent: vi.fn((e) => e), | ||||||||||||||||||||||||
| PRODUCTION_TRACE_SAMPLE_RATE: 0.1, | ||||||||||||||||||||||||
| })); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| describe('withSentryIpc', () => { | ||||||||||||||||||||||||
| let withSentryIpc: typeof import('../sentry').withSentryIpc; | ||||||||||||||||||||||||
| const fakeEvent = {} as IpcMainInvokeEvent; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| beforeEach(async () => { | ||||||||||||||||||||||||
| vi.clearAllMocks(); | ||||||||||||||||||||||||
| const mod = await import('../sentry'); | ||||||||||||||||||||||||
| withSentryIpc = mod.withSentryIpc; | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
|
Comment on lines
+47
to
+51
Contributor
There was a problem hiding this comment. 🧹 Nitpick | 🔵 Trivial Module caching between
♻️ Use `vi.resetModules()` for a clean slate per test beforeEach(async () => {
vi.clearAllMocks();
+ vi.resetModules();
const mod = await import('../sentry');
withSentryIpc = mod.withSentryIpc;
});📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| it('passes through successful handler result', async () => { | ||||||||||||||||||||||||
| const handler = vi.fn().mockResolvedValue('ok'); | ||||||||||||||||||||||||
| const wrapped = withSentryIpc('test-channel', handler); | ||||||||||||||||||||||||
| const result = await wrapped(fakeEvent, 'arg1'); | ||||||||||||||||||||||||
| expect(result).toBe('ok'); | ||||||||||||||||||||||||
| expect(handler).toHaveBeenCalledWith(fakeEvent, 'arg1'); | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| it('re-throws and captures exception on handler error', async () => { | ||||||||||||||||||||||||
| const err = new Error('boom'); | ||||||||||||||||||||||||
| const handler = vi.fn().mockRejectedValue(err); | ||||||||||||||||||||||||
| const wrapped = withSentryIpc('test-channel', handler); | ||||||||||||||||||||||||
| await expect(wrapped(fakeEvent)).rejects.toThrow('boom'); | ||||||||||||||||||||||||
| expect(mockCaptureException).toHaveBeenCalledWith( | ||||||||||||||||||||||||
| err, | ||||||||||||||||||||||||
| { tags: { ipc_channel: 'test-channel' } } | ||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| it('tags error with the correct ipc_channel', async () => { | ||||||||||||||||||||||||
| const handler = vi.fn().mockRejectedValue(new Error('fail')); | ||||||||||||||||||||||||
| const wrapped = withSentryIpc('my-custom-channel', handler); | ||||||||||||||||||||||||
| await expect(wrapped(fakeEvent)).rejects.toThrow('fail'); | ||||||||||||||||||||||||
| expect(mockCaptureException).toHaveBeenCalledWith( | ||||||||||||||||||||||||
| expect.any(Error), | ||||||||||||||||||||||||
| { tags: { ipc_channel: 'my-custom-channel' } } | ||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
|
Comment on lines
+1
to
+81
Contributor
There was a problem hiding this comment. 🧹 Nitpick | 🔵 Trivial No test coverage for the new The crash reporter initialization added in
Would you like me to draft the 🤖 Prompt for AI Agents |
||||||||||||||||||||||||
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Sentry credentials in the packaging steps are unused and redundant.
SENTRY_AUTH_TOKEN,SENTRY_ORG, andSENTRY_PROJECTare consumed only during the Vite build step (npm run build) by the@sentry/vite-plugin. The subsequent packaging steps (npm run package:*) invokeelectron-builder, which does not call Vite and therefore never reads these variables. Including them in the package steps is harmless but adds noise.♻️ Remove redundant Sentry env vars from package steps
- name: Package macOS (Intel) run: cd apps/frontend && npm run package:mac -- --x64 env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} CSC_LINK: ${{ secrets.MAC_CERTIFICATE }} CSC_KEY_PASSWORD: ${{ secrets.MAC_CERTIFICATE_PASSWORD }} SENTRY_DSN: ${{ secrets.SENTRY_DSN }} SENTRY_TRACES_SAMPLE_RATE: ${{ secrets.SENTRY_TRACES_SAMPLE_RATE }} SENTRY_PROFILES_SAMPLE_RATE: ${{ secrets.SENTRY_PROFILES_SAMPLE_RATE }} - SENTRY_AUTH_TOKEN: ${{ secrets.SENTRY_AUTH_TOKEN }} - SENTRY_ORG: ${{ secrets.SENTRY_ORG }} - SENTRY_PROJECT: ${{ secrets.SENTRY_PROJECT }}Apply the same removal to the Package macOS (ARM64), Package Windows, and Package Linux steps.
Also applies to: 152-154, 228-230, 406-408
🤖 Prompt for AI Agents