Skip to content

fix: Stop re-running event processors on every main process scope change - #1468

Open
timfish wants to merge 4 commits into
masterfrom
fix/scope-change-event-defaults
Open

timfish wants to merge 4 commits into
masterfrom
fix/scope-change-event-defaults

Conversation

@timfish

@timfish timfish commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #1467

Every scope change in the main process rebuilt the event defaults by running every event processor, including the synchronous app.getAppMetrics() (about 0.04 ms per process), and built the merged scope three times over. A burst of console.log calls or forwarded renderer breadcrumbs blocked the main thread for seconds in apps with many processes.

A new createScopeSaver merges the scope changes of one event loop turn into a single save on the next turn, reads the merged scope once when that save runs, and rebuilds the defaults only when they are older than 5 seconds. Both minidump integrations call its refresh from app.whenReady(), because screen and GPU context only exist once the app is ready. A sequence number stops a slow older save from writing over a newer one. onScopeChange in common/scope.ts registers a bare callback on the three scopes; addScopeListener is unchanged for the renderer. The Electron uploader path now builds its event synchronously, so its epoch guard is gone.

Unit tests cover coalescing, the single scope read, caching, refresh, the stale-save guard and rejection handling. The reproduction from the issue now makes zero app.getAppMetrics() calls for 5,000 console.log calls and the main thread is busy for about 18 ms instead of seconds (about 12 ms with no SDK). The full native crash e2e suite passes on Electron 39.8.10.

app.getAppMetrics() still runs once per real event through ElectronContext; whether the app-wide memory figure is worth keeping is a separate question.

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

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit b78fda8. Configure here.

Comment thread src/main/scope-updates.ts
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.
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.
@timfish
timfish requested a review from JPeer264 October 15, 2026 11:03
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.
@timfish
timfish requested a review from nicohrubec October 15, 2026 15:37
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.

Main process: every scope change re-runs all event processors, including a synchronous app.getAppMetrics()

1 participant