From b78fda8f620c2442e53cdae590205b01b17146d9 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 15 Oct 2026 10:50:38 +0100 Subject: [PATCH 1/4] fix: Stop re-running event processors on every main process scope change Every scope change in the main process scheduled its own setImmediate that rebuilt the event defaults by running every event processor. ElectronContext calls the synchronous app.getAppMetrics(), which costs about 0.04 ms per process, so a burst of breadcrumbs blocked the main thread for seconds in apps with many processes. Scope changes now coalesce into one save on the next event loop turn and the event defaults are rebuilt at most every 5 seconds. A save that reused cached defaults schedules one catch-up save at expiry so context that only exists after the app is ready, such as screen and GPU details, still reaches the stored scope and the crash reporter parameters. Both the Sentry and the Electron minidump uploader paths use the new helper. In the Electron uploader path the event build is now synchronous, so the epoch guard is gone. Fixes #1467 --- src/main/context.ts | 6 + src/main/integrations/electron-minidump.ts | 58 ++++---- .../integrations/sentry-minidump/index.ts | 22 ++-- src/main/scope-updates.ts | 69 ++++++++++ test/unit/scope-updates.test.ts | 124 ++++++++++++++++++ 5 files changed, 235 insertions(+), 44 deletions(-) create mode 100644 src/main/scope-updates.ts create mode 100644 test/unit/scope-updates.test.ts diff --git a/src/main/context.ts b/src/main/context.ts index 4282ec1a..d111a99a 100644 --- a/src/main/context.ts +++ b/src/main/context.ts @@ -32,6 +32,12 @@ export function getDefaultEnvironment(): string { return app.isPackaged ? 'production' : 'development'; } +/** + * How long scope saves may reuse the event defaults. Building them runs + * every event processor and some are slow, such as `app.getAppMetrics()`. + */ +export const EVENT_DEFAULTS_TTL_MS = 5_000; + /** * Gets the default event data by calling all event processors. */ diff --git a/src/main/integrations/electron-minidump.ts b/src/main/integrations/electron-minidump.ts index 239d55d2..3514ea0b 100644 --- a/src/main/integrations/electron-minidump.ts +++ b/src/main/integrations/electron-minidump.ts @@ -3,10 +3,11 @@ import { applyScopeDataToEvent, debug, defineIntegration, makeDsn, uuid4 } from import type { NodeClient, NodeOptions } from '@sentry/node'; import { app, crashReporter } from 'electron'; import { addScopeListener, getScopeData } from '../../common/scope.js'; -import { getEventDefaults, getSdkInfo } from '../context.js'; +import { EVENT_DEFAULTS_TTL_MS, getEventDefaults, getSdkInfo } from '../context.js'; import { CRASH_REASONS } from '../electron-normalize.js'; import { mergeEvents } from '../merge.js'; import { normalizePaths } from '../normalize.js'; +import { createScopeSaver } from '../scope-updates.js'; import { checkPreviousSession, sessionCrashed, unreportedDuringLastSession } from '../sessions.js'; /** Is object defined and has keys */ @@ -93,14 +94,12 @@ export function minidumpUrlFromDsn(dsn: string): string | undefined { * Sends minidumps via the Electron built-in uploader. */ export const electronMinidumpIntegration = defineIntegration(() => { - /** Counter used to ensure no race condition when updating extra params */ - let updateEpoch = 0; let lastParamCount = 0; - async function getNativeUploaderEvent(client: NodeClient, scope: ScopeData): Promise { + function getNativeUploaderEvent(client: NodeClient, scope: ScopeData, defaults: Event): Event { const { userInfo: inferIpAddress } = client.getDataCollectionOptions(); - const event = mergeEvents(await getEventDefaults(client), { + const event = mergeEvents(defaults, { sdk: getSdkInfo(inferIpAddress), event_id: uuid4(), level: 'fatal', @@ -116,35 +115,26 @@ export const electronMinidumpIntegration = defineIntegration(() => { return normalizePaths(event, app.getAppPath()); } - function updateExtraParams(client: NodeClient, scope: ScopeData): void { - updateEpoch += 1; - const currentEpoch = updateEpoch; + function updateExtraParams(client: NodeClient, scope: ScopeData, defaults: Event): void { + const event = getNativeUploaderEvent(client, scope, defaults); - getNativeUploaderEvent(client, scope) - .then((event) => { - if (currentEpoch !== updateEpoch) { - return; - } - - // Update the extra parameters in the main process - const mainParams = getNativeUploaderExtraParams(event); - const count = Object.keys(mainParams).length; + // Update the extra parameters in the main process + const mainParams = getNativeUploaderExtraParams(event); + const count = Object.keys(mainParams).length; - // Remove any extra parameters that extend beyond the current count - if (lastParamCount > count) { - for (let i = count + 1; i <= lastParamCount; i++) { - crashReporter.removeExtraParameter(`sentry__${i}`); - } - } + // Remove any extra parameters that extend beyond the current count + if (lastParamCount > count) { + for (let i = count + 1; i <= lastParamCount; i++) { + crashReporter.removeExtraParameter(`sentry__${i}`); + } + } - for (const [key, value] of Object.entries(mainParams)) { - crashReporter.addExtraParameter(key, value); - } + for (const [key, value] of Object.entries(mainParams)) { + crashReporter.addExtraParameter(key, value); + } - // Track how many parameters we added so we can remove extras next time - lastParamCount = count; - }) - .catch((error) => debug.error(error)); + // Track how many parameters we added so we can remove extras next time + lastParamCount = count; } function startCrashReporter(options: NodeOptions): void { @@ -172,8 +162,14 @@ export const electronMinidumpIntegration = defineIntegration(() => { } function setupScopeListener(client: NodeClient): void { + const update = createScopeSaver( + () => getEventDefaults(client), + (scope, defaults) => updateExtraParams(client, scope, defaults), + EVENT_DEFAULTS_TTL_MS, + ); + addScopeListener((scope) => { - updateExtraParams(client, scope); + update(scope); }); } diff --git a/src/main/integrations/sentry-minidump/index.ts b/src/main/integrations/sentry-minidump/index.ts index 61c3d2ce..4837652a 100644 --- a/src/main/integrations/sentry-minidump/index.ts +++ b/src/main/integrations/sentry-minidump/index.ts @@ -4,9 +4,10 @@ import type { NodeClient } from '@sentry/node'; import { chromeStackLineParser } from '@sentry/browser'; import { app, crashReporter } from 'electron'; import { addScopeListener, getScopeData } from '../../../common/scope.js'; -import { getEventDefaults } from '../../context.js'; +import { EVENT_DEFAULTS_TTL_MS, getEventDefaults } from '../../context.js'; import { EXIT_REASONS, getSentryCachePath } from '../../electron-normalize.js'; import { getRendererProperties, trackRendererProperties } from '../../renderers.js'; +import { createScopeSaver } from '../../scope-updates.js'; import type { ElectronMainOptions } from '../../sdk.js'; import { previousSessionWasAbnormal, restorePreviousSession, setPreviousSessionAsCurrent } from '../../sessions.js'; import { BufferedWriteStore } from '../../store.js'; @@ -58,22 +59,17 @@ export const sentryMinidumpIntegration = defineIntegration((options: Options = { } function setupScopeListener(client: NodeClient): void { - function scopeChanged(scope: ScopeData): void { - // Since the initial scope read is async, we need to ensure that any writes do not beat that - // https://github.com/getsentry/sentry-electron/issues/585 - setImmediate(async () => - scopeStore?.set({ - scope, - event: await getEventDefaults(client), - }), - ); - } + const save = createScopeSaver( + () => getEventDefaults(client), + (scope, event) => scopeStore?.set({ scope, event }), + EVENT_DEFAULTS_TTL_MS, + ); addScopeListener((scope) => { - scopeChanged(scope); + save(scope); }); - scopeChanged(getScopeData()); + save(getScopeData()); } async function sendNativeCrashes( diff --git a/src/main/scope-updates.ts b/src/main/scope-updates.ts new file mode 100644 index 00000000..7e320bdc --- /dev/null +++ b/src/main/scope-updates.ts @@ -0,0 +1,69 @@ +import { debug } from '@sentry/core'; + +/** + * Creates a function that saves the latest scope together with event + * defaults that are at most `ttlMs` old. + * + * Scope changes in one event loop turn are merged into one save on the + * next turn. The defaults are rebuilt only when a save finds them + * expired. A save that used cached defaults arms one timer so the saved + * data catches up once they expire. This matters because some context, + * such as screen and GPU details, only exists after the app is ready. + */ +export function createScopeSaver( + getDefaults: () => Promise, + save: (scope: TScope, defaults: TDefaults) => void | Promise, + ttlMs: number, +): (scope: TScope) => void { + let latest: TScope; + let scheduled = false; + let defaults: Promise | undefined; + let expires = 0; + let refresh: NodeJS.Timeout | undefined; + + function schedule(): void { + if (scheduled) { + return; + } + + scheduled = true; + + // Deferring to the next turn ensures that the first save does not + // beat the initial async read of the store + // https://github.com/getsentry/sentry-electron/issues/585 + setImmediate(() => { + run().catch((error) => debug.error(error)); + }); + } + + async function run(): Promise { + scheduled = false; + const scope = latest; + const now = Date.now(); + + if (!defaults || now >= expires) { + const current = getDefaults(); + defaults = current; + expires = now + ttlMs; + + current.catch(() => { + if (defaults === current) { + defaults = undefined; + } + }); + } else if (!refresh) { + refresh = setTimeout(() => { + refresh = undefined; + schedule(); + }, expires - now); + refresh.unref(); + } + + await save(scope, await defaults); + } + + return (scope: TScope) => { + latest = scope; + schedule(); + }; +} diff --git a/test/unit/scope-updates.test.ts b/test/unit/scope-updates.test.ts new file mode 100644 index 00000000..6f3f45c4 --- /dev/null +++ b/test/unit/scope-updates.test.ts @@ -0,0 +1,124 @@ +import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; +import { createScopeSaver } from '../../src/main/scope-updates'; + +const TTL = 1000; + +describe('createScopeSaver', () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + test('merges synchronous scope changes into one save with the last scope', async () => { + const getDefaults = vi.fn().mockResolvedValue('defaults'); + const save = vi.fn(); + const update = createScopeSaver(getDefaults, save, TTL); + + for (let i = 0; i < 1000; i++) { + update(i); + } + + expect(save).not.toHaveBeenCalled(); + await vi.advanceTimersByTimeAsync(0); + expect(getDefaults).toHaveBeenCalledTimes(1); + expect(save).toHaveBeenCalledTimes(1); + expect(save).toHaveBeenCalledWith(999, 'defaults'); + }); + + test('saves on each turn but builds the defaults once within the ttl', async () => { + const getDefaults = vi.fn().mockResolvedValue('defaults'); + const save = vi.fn(); + const update = createScopeSaver(getDefaults, save, TTL); + + for (let i = 0; i < 10; i++) { + update(i); + await vi.advanceTimersByTimeAsync(10); + } + + expect(save).toHaveBeenCalledTimes(10); + expect(save).toHaveBeenLastCalledWith(9, 'defaults'); + expect(getDefaults).toHaveBeenCalledTimes(1); + }); + + test('rebuilds the defaults when a save finds them expired', async () => { + const getDefaults = vi.fn().mockResolvedValueOnce('first').mockResolvedValueOnce('second'); + const save = vi.fn(); + const update = createScopeSaver(getDefaults, save, TTL); + + update(1); + await vi.advanceTimersByTimeAsync(TTL); + update(2); + await vi.advanceTimersByTimeAsync(0); + + expect(save).toHaveBeenNthCalledWith(1, 1, 'first'); + expect(save).toHaveBeenNthCalledWith(2, 2, 'second'); + }); + + test('saves once more with fresh defaults after the ttl when cached defaults were used', async () => { + const getDefaults = vi.fn().mockResolvedValueOnce('first').mockResolvedValueOnce('second'); + const save = vi.fn(); + const update = createScopeSaver(getDefaults, save, TTL); + + update(1); + await vi.advanceTimersByTimeAsync(100); + update(2); + update(3); + await vi.advanceTimersByTimeAsync(100); + expect(save).toHaveBeenCalledTimes(2); + expect(save).toHaveBeenLastCalledWith(3, 'first'); + + await vi.advanceTimersByTimeAsync(TTL); + expect(save).toHaveBeenCalledTimes(3); + expect(save).toHaveBeenLastCalledWith(3, 'second'); + expect(getDefaults).toHaveBeenCalledTimes(2); + + // Nothing else is pending once the data has caught up + await vi.advanceTimersByTimeAsync(TTL * 2); + expect(save).toHaveBeenCalledTimes(3); + }); + + test('does not arm a catch-up save when the defaults were just built', async () => { + const getDefaults = vi.fn().mockResolvedValue('defaults'); + const save = vi.fn(); + const update = createScopeSaver(getDefaults, save, TTL); + + update(1); + await vi.advanceTimersByTimeAsync(TTL * 2); + + expect(save).toHaveBeenCalledTimes(1); + expect(getDefaults).toHaveBeenCalledTimes(1); + }); + + test('does not cache rejected defaults', async () => { + const getDefaults = vi.fn().mockRejectedValueOnce(new Error('nope')).mockResolvedValue('defaults'); + const save = vi.fn(); + const update = createScopeSaver(getDefaults, save, TTL); + + update(1); + await vi.advanceTimersByTimeAsync(0); + expect(save).not.toHaveBeenCalled(); + + update(2); + await vi.advanceTimersByTimeAsync(0); + expect(save).toHaveBeenCalledTimes(1); + expect(save).toHaveBeenCalledWith(2, 'defaults'); + expect(getDefaults).toHaveBeenCalledTimes(2); + }); + + test('a rejecting save does not break later saves', async () => { + const getDefaults = vi.fn().mockResolvedValue('defaults'); + const save = vi.fn().mockRejectedValueOnce(new Error('nope')).mockResolvedValue(undefined); + const update = createScopeSaver(getDefaults, save, TTL); + + update(1); + await vi.advanceTimersByTimeAsync(0); + update(2); + await vi.advanceTimersByTimeAsync(0); + + expect(save).toHaveBeenCalledTimes(2); + expect(save).toHaveBeenLastCalledWith(2, 'defaults'); + }); +}); From 959ea57b7b765003c9f4b1418ccac442b33d0164 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 15 Oct 2026 11:38:09 +0100 Subject: [PATCH 2/4] fix: Skip a stale scope save when a newer run has started A defaults fetch that outlives the cache window lets a later save start a second fetch. If the second one resolves first, the older save would write an older scope over the newer one. Each run now records its sequence number and skips its save when a newer run has started. --- src/main/scope-updates.ts | 12 +++++++++++- test/unit/scope-updates.test.ts | 23 +++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/main/scope-updates.ts b/src/main/scope-updates.ts index 7e320bdc..a4f4ef0c 100644 --- a/src/main/scope-updates.ts +++ b/src/main/scope-updates.ts @@ -17,6 +17,7 @@ export function createScopeSaver( ): (scope: TScope) => void { let latest: TScope; let scheduled = false; + let runs = 0; let defaults: Promise | undefined; let expires = 0; let refresh: NodeJS.Timeout | undefined; @@ -38,6 +39,7 @@ export function createScopeSaver( async function run(): Promise { scheduled = false; + const thisRun = ++runs; const scope = latest; const now = Date.now(); @@ -59,7 +61,15 @@ export function createScopeSaver( refresh.unref(); } - await save(scope, await defaults); + const resolved = await defaults; + + // A newer run holds a newer scope, so a slow older one must not + // write over it + if (thisRun !== runs) { + return; + } + + await save(scope, resolved); } return (scope: TScope) => { diff --git a/test/unit/scope-updates.test.ts b/test/unit/scope-updates.test.ts index 6f3f45c4..444bcd85 100644 --- a/test/unit/scope-updates.test.ts +++ b/test/unit/scope-updates.test.ts @@ -92,6 +92,29 @@ describe('createScopeSaver', () => { expect(getDefaults).toHaveBeenCalledTimes(1); }); + test('a slow older run does not write over a newer save', async () => { + let resolveFirst: (value: string) => void = () => undefined; + const getDefaults = vi + .fn<() => Promise>() + .mockImplementationOnce(() => new Promise((resolve) => (resolveFirst = resolve))) + .mockResolvedValue('second'); + const save = vi.fn(); + const update = createScopeSaver(getDefaults, save, TTL); + + update(1); + await vi.advanceTimersByTimeAsync(TTL); + expect(save).not.toHaveBeenCalled(); + + update(2); + await vi.advanceTimersByTimeAsync(0); + expect(save).toHaveBeenCalledTimes(1); + expect(save).toHaveBeenCalledWith(2, 'second'); + + resolveFirst('first'); + await vi.advanceTimersByTimeAsync(0); + expect(save).toHaveBeenCalledTimes(1); + }); + test('does not cache rejected defaults', async () => { const getDefaults = vi.fn().mockRejectedValueOnce(new Error('nope')).mockResolvedValue('defaults'); const save = vi.fn(); From 5faf0ba5a54b4c83a8adc3d573f8248bd5d4b2a4 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 15 Oct 2026 11:53:42 +0100 Subject: [PATCH 3/4] fix: Read the merged scope once per save instead of once per change addScopeListener builds the merged scope inside each of the three scope listeners before it calls back, which copies both breadcrumb arrays on every change. The saver only keeps the last scope of a turn, so that work was wasted. The saver now takes a scope getter and reads it once when the save runs, and the minidump integrations subscribe through a new onScopeChange that passes nothing. --- src/common/scope.ts | 11 +++ src/main/integrations/electron-minidump.ts | 7 +- .../integrations/sentry-minidump/index.ts | 10 ++- src/main/scope-updates.ts | 26 ++++--- test/unit/scope-updates.test.ts | 67 ++++++++++++------- 5 files changed, 74 insertions(+), 47 deletions(-) diff --git a/src/common/scope.ts b/src/common/scope.ts index 5651a83b..2680e155 100644 --- a/src/common/scope.ts +++ b/src/common/scope.ts @@ -12,6 +12,17 @@ export function getScopeData(): ScopeData { return globalScope; } +/** + * Calls back on any change to the global, isolation or current scope + * without building the merged scope. Use this when the callback reads + * the scope lazily, so bursts of changes cost nothing until it does. + */ +export function onScopeChange(callback: () => void): void { + getGlobalScope().addScopeListener(callback); + getIsolationScope().addScopeListener(callback); + getCurrentScope().addScopeListener(callback); +} + /** Hooks both current and isolation scope changes and passes merged scope on changes */ export function addScopeListener(callback: (merged: ScopeData, changed: Scope) => void): void { getIsolationScope().addScopeListener((isolation) => { diff --git a/src/main/integrations/electron-minidump.ts b/src/main/integrations/electron-minidump.ts index 3514ea0b..422bc519 100644 --- a/src/main/integrations/electron-minidump.ts +++ b/src/main/integrations/electron-minidump.ts @@ -2,7 +2,7 @@ import type { Event, ScopeData } from '@sentry/core'; import { applyScopeDataToEvent, debug, defineIntegration, makeDsn, uuid4 } from '@sentry/core'; import type { NodeClient, NodeOptions } from '@sentry/node'; import { app, crashReporter } from 'electron'; -import { addScopeListener, getScopeData } from '../../common/scope.js'; +import { getScopeData, onScopeChange } from '../../common/scope.js'; import { EVENT_DEFAULTS_TTL_MS, getEventDefaults, getSdkInfo } from '../context.js'; import { CRASH_REASONS } from '../electron-normalize.js'; import { mergeEvents } from '../merge.js'; @@ -163,14 +163,13 @@ export const electronMinidumpIntegration = defineIntegration(() => { function setupScopeListener(client: NodeClient): void { const update = createScopeSaver( + getScopeData, () => getEventDefaults(client), (scope, defaults) => updateExtraParams(client, scope, defaults), EVENT_DEFAULTS_TTL_MS, ); - addScopeListener((scope) => { - update(scope); - }); + onScopeChange(update); } return { diff --git a/src/main/integrations/sentry-minidump/index.ts b/src/main/integrations/sentry-minidump/index.ts index 4837652a..17abee1c 100644 --- a/src/main/integrations/sentry-minidump/index.ts +++ b/src/main/integrations/sentry-minidump/index.ts @@ -3,7 +3,7 @@ import { applyScopeDataToEvent, captureEvent, createStackParser, debug, defineIn import type { NodeClient } from '@sentry/node'; import { chromeStackLineParser } from '@sentry/browser'; import { app, crashReporter } from 'electron'; -import { addScopeListener, getScopeData } from '../../../common/scope.js'; +import { getScopeData, onScopeChange } from '../../../common/scope.js'; import { EVENT_DEFAULTS_TTL_MS, getEventDefaults } from '../../context.js'; import { EXIT_REASONS, getSentryCachePath } from '../../electron-normalize.js'; import { getRendererProperties, trackRendererProperties } from '../../renderers.js'; @@ -60,16 +60,14 @@ export const sentryMinidumpIntegration = defineIntegration((options: Options = { function setupScopeListener(client: NodeClient): void { const save = createScopeSaver( + getScopeData, () => getEventDefaults(client), (scope, event) => scopeStore?.set({ scope, event }), EVENT_DEFAULTS_TTL_MS, ); - addScopeListener((scope) => { - save(scope); - }); - - save(getScopeData()); + onScopeChange(save); + save(); } async function sendNativeCrashes( diff --git a/src/main/scope-updates.ts b/src/main/scope-updates.ts index a4f4ef0c..5f8882f6 100644 --- a/src/main/scope-updates.ts +++ b/src/main/scope-updates.ts @@ -1,21 +1,22 @@ import { debug } from '@sentry/core'; /** - * Creates a function that saves the latest scope together with event - * defaults that are at most `ttlMs` old. + * Creates a function that schedules a save of the current scope together + * with event defaults that are at most `ttlMs` old. * - * Scope changes in one event loop turn are merged into one save on the - * next turn. The defaults are rebuilt only when a save finds them - * expired. A save that used cached defaults arms one timer so the saved - * data catches up once they expire. This matters because some context, - * such as screen and GPU details, only exists after the app is ready. + * Calls in one event loop turn are merged into one save on the next + * turn, and the scope is read once when that save runs. The defaults + * are rebuilt only when a save finds them expired. A save that used + * cached defaults arms one timer so the saved data catches up once they + * expire. This matters because some context, such as screen and GPU + * details, only exists after the app is ready. */ export function createScopeSaver( + getScope: () => TScope, getDefaults: () => Promise, save: (scope: TScope, defaults: TDefaults) => void | Promise, ttlMs: number, -): (scope: TScope) => void { - let latest: TScope; +): () => void { let scheduled = false; let runs = 0; let defaults: Promise | undefined; @@ -40,7 +41,7 @@ export function createScopeSaver( async function run(): Promise { scheduled = false; const thisRun = ++runs; - const scope = latest; + const scope = getScope(); const now = Date.now(); if (!defaults || now >= expires) { @@ -72,8 +73,5 @@ export function createScopeSaver( await save(scope, resolved); } - return (scope: TScope) => { - latest = scope; - schedule(); - }; + return schedule; } diff --git a/test/unit/scope-updates.test.ts b/test/unit/scope-updates.test.ts index 444bcd85..112db0b3 100644 --- a/test/unit/scope-updates.test.ts +++ b/test/unit/scope-updates.test.ts @@ -4,25 +4,33 @@ import { createScopeSaver } from '../../src/main/scope-updates'; const TTL = 1000; describe('createScopeSaver', () => { + let scope = 0; + const getScope = vi.fn(() => scope); + beforeEach(() => { vi.useFakeTimers(); + scope = 0; + getScope.mockClear(); }); afterEach(() => { vi.useRealTimers(); }); - test('merges synchronous scope changes into one save with the last scope', async () => { + test('merges synchronous changes into one save and reads the scope once', async () => { const getDefaults = vi.fn().mockResolvedValue('defaults'); const save = vi.fn(); - const update = createScopeSaver(getDefaults, save, TTL); + const update = createScopeSaver(getScope, getDefaults, save, TTL); for (let i = 0; i < 1000; i++) { - update(i); + scope = i; + update(); } + expect(getScope).not.toHaveBeenCalled(); expect(save).not.toHaveBeenCalled(); await vi.advanceTimersByTimeAsync(0); + expect(getScope).toHaveBeenCalledTimes(1); expect(getDefaults).toHaveBeenCalledTimes(1); expect(save).toHaveBeenCalledTimes(1); expect(save).toHaveBeenCalledWith(999, 'defaults'); @@ -31,10 +39,11 @@ describe('createScopeSaver', () => { test('saves on each turn but builds the defaults once within the ttl', async () => { const getDefaults = vi.fn().mockResolvedValue('defaults'); const save = vi.fn(); - const update = createScopeSaver(getDefaults, save, TTL); + const update = createScopeSaver(getScope, getDefaults, save, TTL); for (let i = 0; i < 10; i++) { - update(i); + scope = i; + update(); await vi.advanceTimersByTimeAsync(10); } @@ -46,11 +55,13 @@ describe('createScopeSaver', () => { test('rebuilds the defaults when a save finds them expired', async () => { const getDefaults = vi.fn().mockResolvedValueOnce('first').mockResolvedValueOnce('second'); const save = vi.fn(); - const update = createScopeSaver(getDefaults, save, TTL); + const update = createScopeSaver(getScope, getDefaults, save, TTL); - update(1); + scope = 1; + update(); await vi.advanceTimersByTimeAsync(TTL); - update(2); + scope = 2; + update(); await vi.advanceTimersByTimeAsync(0); expect(save).toHaveBeenNthCalledWith(1, 1, 'first'); @@ -60,12 +71,15 @@ describe('createScopeSaver', () => { test('saves once more with fresh defaults after the ttl when cached defaults were used', async () => { const getDefaults = vi.fn().mockResolvedValueOnce('first').mockResolvedValueOnce('second'); const save = vi.fn(); - const update = createScopeSaver(getDefaults, save, TTL); + const update = createScopeSaver(getScope, getDefaults, save, TTL); - update(1); + scope = 1; + update(); await vi.advanceTimersByTimeAsync(100); - update(2); - update(3); + scope = 2; + update(); + scope = 3; + update(); await vi.advanceTimersByTimeAsync(100); expect(save).toHaveBeenCalledTimes(2); expect(save).toHaveBeenLastCalledWith(3, 'first'); @@ -83,9 +97,10 @@ describe('createScopeSaver', () => { test('does not arm a catch-up save when the defaults were just built', async () => { const getDefaults = vi.fn().mockResolvedValue('defaults'); const save = vi.fn(); - const update = createScopeSaver(getDefaults, save, TTL); + const update = createScopeSaver(getScope, getDefaults, save, TTL); - update(1); + scope = 1; + update(); await vi.advanceTimersByTimeAsync(TTL * 2); expect(save).toHaveBeenCalledTimes(1); @@ -99,13 +114,15 @@ describe('createScopeSaver', () => { .mockImplementationOnce(() => new Promise((resolve) => (resolveFirst = resolve))) .mockResolvedValue('second'); const save = vi.fn(); - const update = createScopeSaver(getDefaults, save, TTL); + const update = createScopeSaver(getScope, getDefaults, save, TTL); - update(1); + scope = 1; + update(); await vi.advanceTimersByTimeAsync(TTL); expect(save).not.toHaveBeenCalled(); - update(2); + scope = 2; + update(); await vi.advanceTimersByTimeAsync(0); expect(save).toHaveBeenCalledTimes(1); expect(save).toHaveBeenCalledWith(2, 'second'); @@ -118,13 +135,15 @@ describe('createScopeSaver', () => { test('does not cache rejected defaults', async () => { const getDefaults = vi.fn().mockRejectedValueOnce(new Error('nope')).mockResolvedValue('defaults'); const save = vi.fn(); - const update = createScopeSaver(getDefaults, save, TTL); + const update = createScopeSaver(getScope, getDefaults, save, TTL); - update(1); + scope = 1; + update(); await vi.advanceTimersByTimeAsync(0); expect(save).not.toHaveBeenCalled(); - update(2); + scope = 2; + update(); await vi.advanceTimersByTimeAsync(0); expect(save).toHaveBeenCalledTimes(1); expect(save).toHaveBeenCalledWith(2, 'defaults'); @@ -134,11 +153,13 @@ describe('createScopeSaver', () => { test('a rejecting save does not break later saves', async () => { const getDefaults = vi.fn().mockResolvedValue('defaults'); const save = vi.fn().mockRejectedValueOnce(new Error('nope')).mockResolvedValue(undefined); - const update = createScopeSaver(getDefaults, save, TTL); + const update = createScopeSaver(getScope, getDefaults, save, TTL); - update(1); + scope = 1; + update(); await vi.advanceTimersByTimeAsync(0); - update(2); + scope = 2; + update(); await vi.advanceTimersByTimeAsync(0); expect(save).toHaveBeenCalledTimes(2); From 207119bb35831874004be9a0debce16b40f6827c Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 15 Oct 2026 12:41:22 +0100 Subject: [PATCH 4/4] fix: Rebuild the cached event defaults once the app is ready Screen and GPU context only exist after app ready. When the first scope change ran before that, the defaults were cached without them and a native crash in the next five seconds carried incomplete context. The saver now exposes refresh, which drops the cached defaults and saves again, and both minidump integrations call it from app.whenReady(). The catch-up timer at expiry is gone because this covers the case it was added for. --- src/main/integrations/electron-minidump.ts | 6 +- .../integrations/sentry-minidump/index.ts | 8 +- src/main/scope-updates.ts | 33 ++++---- test/unit/scope-updates.test.ts | 77 +++++++++---------- 4 files changed, 63 insertions(+), 61 deletions(-) diff --git a/src/main/integrations/electron-minidump.ts b/src/main/integrations/electron-minidump.ts index 422bc519..2de2f91c 100644 --- a/src/main/integrations/electron-minidump.ts +++ b/src/main/integrations/electron-minidump.ts @@ -162,14 +162,16 @@ export const electronMinidumpIntegration = defineIntegration(() => { } function setupScopeListener(client: NodeClient): void { - const update = createScopeSaver( + const saver = createScopeSaver( getScopeData, () => getEventDefaults(client), (scope, defaults) => updateExtraParams(client, scope, defaults), EVENT_DEFAULTS_TTL_MS, ); - onScopeChange(update); + onScopeChange(saver.save); + // Screen and GPU context only exist once the app is ready + void app.whenReady().then(saver.refresh); } return { diff --git a/src/main/integrations/sentry-minidump/index.ts b/src/main/integrations/sentry-minidump/index.ts index 17abee1c..dfa5f163 100644 --- a/src/main/integrations/sentry-minidump/index.ts +++ b/src/main/integrations/sentry-minidump/index.ts @@ -59,15 +59,17 @@ export const sentryMinidumpIntegration = defineIntegration((options: Options = { } function setupScopeListener(client: NodeClient): void { - const save = createScopeSaver( + const saver = createScopeSaver( getScopeData, () => getEventDefaults(client), (scope, event) => scopeStore?.set({ scope, event }), EVENT_DEFAULTS_TTL_MS, ); - onScopeChange(save); - save(); + onScopeChange(saver.save); + // Screen and GPU context only exist once the app is ready + void app.whenReady().then(saver.refresh); + saver.save(); } async function sendNativeCrashes( diff --git a/src/main/scope-updates.ts b/src/main/scope-updates.ts index 5f8882f6..54fe4933 100644 --- a/src/main/scope-updates.ts +++ b/src/main/scope-updates.ts @@ -1,27 +1,30 @@ import { debug } from '@sentry/core'; +export interface ScopeSaver { + /** Saves the current scope on the next event loop turn */ + save: () => void; + /** Drops the cached defaults and saves the current scope */ + refresh: () => void; +} + /** - * Creates a function that schedules a save of the current scope together - * with event defaults that are at most `ttlMs` old. + * Creates a saver that writes the current scope together with event + * defaults that are at most `ttlMs` old. * * Calls in one event loop turn are merged into one save on the next * turn, and the scope is read once when that save runs. The defaults - * are rebuilt only when a save finds them expired. A save that used - * cached defaults arms one timer so the saved data catches up once they - * expire. This matters because some context, such as screen and GPU - * details, only exists after the app is ready. + * are rebuilt only when a save finds them expired, or after `refresh`. */ export function createScopeSaver( getScope: () => TScope, getDefaults: () => Promise, save: (scope: TScope, defaults: TDefaults) => void | Promise, ttlMs: number, -): () => void { +): ScopeSaver { let scheduled = false; let runs = 0; let defaults: Promise | undefined; let expires = 0; - let refresh: NodeJS.Timeout | undefined; function schedule(): void { if (scheduled) { @@ -54,12 +57,6 @@ export function createScopeSaver( defaults = undefined; } }); - } else if (!refresh) { - refresh = setTimeout(() => { - refresh = undefined; - schedule(); - }, expires - now); - refresh.unref(); } const resolved = await defaults; @@ -73,5 +70,11 @@ export function createScopeSaver( await save(scope, resolved); } - return schedule; + return { + save: schedule, + refresh: () => { + defaults = undefined; + schedule(); + }, + }; } diff --git a/test/unit/scope-updates.test.ts b/test/unit/scope-updates.test.ts index 112db0b3..36760df8 100644 --- a/test/unit/scope-updates.test.ts +++ b/test/unit/scope-updates.test.ts @@ -20,11 +20,11 @@ describe('createScopeSaver', () => { test('merges synchronous changes into one save and reads the scope once', async () => { const getDefaults = vi.fn().mockResolvedValue('defaults'); const save = vi.fn(); - const update = createScopeSaver(getScope, getDefaults, save, TTL); + const saver = createScopeSaver(getScope, getDefaults, save, TTL); for (let i = 0; i < 1000; i++) { scope = i; - update(); + saver.save(); } expect(getScope).not.toHaveBeenCalled(); @@ -39,11 +39,11 @@ describe('createScopeSaver', () => { test('saves on each turn but builds the defaults once within the ttl', async () => { const getDefaults = vi.fn().mockResolvedValue('defaults'); const save = vi.fn(); - const update = createScopeSaver(getScope, getDefaults, save, TTL); + const saver = createScopeSaver(getScope, getDefaults, save, TTL); for (let i = 0; i < 10; i++) { scope = i; - update(); + saver.save(); await vi.advanceTimersByTimeAsync(10); } @@ -55,56 +55,51 @@ describe('createScopeSaver', () => { test('rebuilds the defaults when a save finds them expired', async () => { const getDefaults = vi.fn().mockResolvedValueOnce('first').mockResolvedValueOnce('second'); const save = vi.fn(); - const update = createScopeSaver(getScope, getDefaults, save, TTL); + const saver = createScopeSaver(getScope, getDefaults, save, TTL); scope = 1; - update(); + saver.save(); await vi.advanceTimersByTimeAsync(TTL); scope = 2; - update(); + saver.save(); await vi.advanceTimersByTimeAsync(0); expect(save).toHaveBeenNthCalledWith(1, 1, 'first'); expect(save).toHaveBeenNthCalledWith(2, 2, 'second'); }); - test('saves once more with fresh defaults after the ttl when cached defaults were used', async () => { + test('refresh rebuilds the defaults and saves without a scope change', async () => { const getDefaults = vi.fn().mockResolvedValueOnce('first').mockResolvedValueOnce('second'); const save = vi.fn(); - const update = createScopeSaver(getScope, getDefaults, save, TTL); + const saver = createScopeSaver(getScope, getDefaults, save, TTL); scope = 1; - update(); + saver.save(); await vi.advanceTimersByTimeAsync(100); - scope = 2; - update(); - scope = 3; - update(); - await vi.advanceTimersByTimeAsync(100); - expect(save).toHaveBeenCalledTimes(2); - expect(save).toHaveBeenLastCalledWith(3, 'first'); + expect(save).toHaveBeenLastCalledWith(1, 'first'); - await vi.advanceTimersByTimeAsync(TTL); - expect(save).toHaveBeenCalledTimes(3); - expect(save).toHaveBeenLastCalledWith(3, 'second'); + saver.refresh(); + await vi.advanceTimersByTimeAsync(0); + expect(save).toHaveBeenCalledTimes(2); + expect(save).toHaveBeenLastCalledWith(1, 'second'); expect(getDefaults).toHaveBeenCalledTimes(2); - - // Nothing else is pending once the data has caught up - await vi.advanceTimersByTimeAsync(TTL * 2); - expect(save).toHaveBeenCalledTimes(3); }); - test('does not arm a catch-up save when the defaults were just built', async () => { - const getDefaults = vi.fn().mockResolvedValue('defaults'); + test('a save in the same turn as refresh is merged into it', async () => { + const getDefaults = vi.fn().mockResolvedValueOnce('first').mockResolvedValueOnce('second'); const save = vi.fn(); - const update = createScopeSaver(getScope, getDefaults, save, TTL); + const saver = createScopeSaver(getScope, getDefaults, save, TTL); - scope = 1; - update(); - await vi.advanceTimersByTimeAsync(TTL * 2); + saver.save(); + await vi.advanceTimersByTimeAsync(0); - expect(save).toHaveBeenCalledTimes(1); - expect(getDefaults).toHaveBeenCalledTimes(1); + scope = 2; + saver.save(); + saver.refresh(); + await vi.advanceTimersByTimeAsync(0); + + expect(save).toHaveBeenCalledTimes(2); + expect(save).toHaveBeenLastCalledWith(2, 'second'); }); test('a slow older run does not write over a newer save', async () => { @@ -114,15 +109,15 @@ describe('createScopeSaver', () => { .mockImplementationOnce(() => new Promise((resolve) => (resolveFirst = resolve))) .mockResolvedValue('second'); const save = vi.fn(); - const update = createScopeSaver(getScope, getDefaults, save, TTL); + const saver = createScopeSaver(getScope, getDefaults, save, TTL); scope = 1; - update(); + saver.save(); await vi.advanceTimersByTimeAsync(TTL); expect(save).not.toHaveBeenCalled(); scope = 2; - update(); + saver.save(); await vi.advanceTimersByTimeAsync(0); expect(save).toHaveBeenCalledTimes(1); expect(save).toHaveBeenCalledWith(2, 'second'); @@ -135,15 +130,15 @@ describe('createScopeSaver', () => { test('does not cache rejected defaults', async () => { const getDefaults = vi.fn().mockRejectedValueOnce(new Error('nope')).mockResolvedValue('defaults'); const save = vi.fn(); - const update = createScopeSaver(getScope, getDefaults, save, TTL); + const saver = createScopeSaver(getScope, getDefaults, save, TTL); scope = 1; - update(); + saver.save(); await vi.advanceTimersByTimeAsync(0); expect(save).not.toHaveBeenCalled(); scope = 2; - update(); + saver.save(); await vi.advanceTimersByTimeAsync(0); expect(save).toHaveBeenCalledTimes(1); expect(save).toHaveBeenCalledWith(2, 'defaults'); @@ -153,13 +148,13 @@ describe('createScopeSaver', () => { test('a rejecting save does not break later saves', async () => { const getDefaults = vi.fn().mockResolvedValue('defaults'); const save = vi.fn().mockRejectedValueOnce(new Error('nope')).mockResolvedValue(undefined); - const update = createScopeSaver(getScope, getDefaults, save, TTL); + const saver = createScopeSaver(getScope, getDefaults, save, TTL); scope = 1; - update(); + saver.save(); await vi.advanceTimersByTimeAsync(0); scope = 2; - update(); + saver.save(); await vi.advanceTimersByTimeAsync(0); expect(save).toHaveBeenCalledTimes(2);