Skip to content

fix: Reinitialize SnapController after clearing state#3870

Merged
FrederikBolding merged 4 commits intomainfrom
fb/clear-state-reinit
Feb 24, 2026
Merged

fix: Reinitialize SnapController after clearing state#3870
FrederikBolding merged 4 commits intomainfrom
fb/clear-state-reinit

Conversation

@FrederikBolding
Copy link
Member

@FrederikBolding FrederikBolding commented Feb 24, 2026

Call init as part of clearState to reinitialize the controller after resetting. This ensures that isReady is populated lazily. Additionally adds an argument to init to disable waiting for the platform to be ready and re-creating the controllerSetup promise.


Note

Medium Risk
Changes the controller lifecycle around clearState/init, which can affect readiness timing and Snap startup behavior; regressions could surface as race conditions if callers assume synchronous platform readiness.

Overview
After clearState, SnapController now recreates the internal #controllerSetup deferred and calls init(false) to reinitialize the controller (including reinstalling any preinstalled Snaps) rather than leaving it in an uninitialized state.

init gains a waitForPlatform flag to optionally avoid awaiting platform readiness (fires #ensureCanUsePlatform in the background and logs errors), and the affected test expectation is updated to reflect isReady becoming true after clearState.

Written by Cursor Bugbot for commit 324eb36. This will update automatically on new commits. Configure here.

@codecov
Copy link

codecov bot commented Feb 24, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.57%. Comparing base (6836c76) to head (324eb36).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3870   +/-   ##
=======================================
  Coverage   98.57%   98.57%           
=======================================
  Files         425      425           
  Lines       12311    12314    +3     
  Branches     1919     1921    +2     
=======================================
+ Hits        12136    12139    +3     
  Misses        175      175           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@FrederikBolding FrederikBolding marked this pull request as ready for review February 24, 2026 12:57
@FrederikBolding FrederikBolding requested a review from a team as a code owner February 24, 2026 12:57
@FrederikBolding FrederikBolding added this pull request to the merge queue Feb 24, 2026
Merged via the queue into main with commit 6508212 Feb 24, 2026
131 checks passed
@FrederikBolding FrederikBolding deleted the fb/clear-state-reinit branch February 24, 2026 13:04
Copy link

@cursor cursor bot left a comment

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.

this.#controllerSetup = createDeferredPromise();

// Re-initialize the controller after clearing the state, re-installing preinstalled Snaps etc.
await this.init(false);
Copy link

Choose a reason for hiding this comment

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

clearState silently swallows errors, breaking controller permanently

Medium Severity

clearState delegates to init(false), but init's try/catch catches all errors (including from #handlePreinstalledSnaps) and only calls logWarning without rethrowing. Previously, clearState called #handlePreinstalledSnaps directly, so errors propagated to the caller. Now they're silently swallowed. Worse, on failure the catch block calls this.#controllerSetup.reject(error), leaving #controllerSetup.promise permanently rejected. Every subsequent call to #ensureCanUsePlatform (used by handleRequest, startSnap, installSnaps, etc.) will throw, making the controller permanently unusable with no signal to the caller that clearState failed.

Additional Locations (1)

Fix in Cursor Fix in Web

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.

2 participants