fix: Add maxWait to sendUpdate debounce to fix UI sync starvation#40331
Merged
fix: Add maxWait to sendUpdate debounce to fix UI sync starvation#40331
maxWait to sendUpdate debounce to fix UI sync starvation#40331Conversation
Contributor
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
Workaround for the Accounts List "syncing" issue. Forces a periodic flush even under continuous updates to avoid indefinite background-to-UI state sync starvation.
The `maxWait` option on `sendUpdate`'s debounce can trigger flushes during intermediate states where `preferences` hasn't been populated yet. This causes `_buildUserTraitsObject` to crash on undefined `preferences` access. - Early-return null from `_buildUserTraitsObject` when `metamaskState.preferences` is undefined (fixes unit test TypeError) - Make `segment-user-traits` e2e test resilient to intermediate identify events by aggregating traits across all events (fixes flaky Firefox assertion)
Contributor
Builds ready [4dc441a]
⚡ Performance Benchmarks (1336 ± 93 ms)
🌐 Dapp Page Load BenchmarksCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
4dc441a to
0cfebf5
Compare
Contributor
Builds ready [0cfebf5]
⚡ Performance Benchmarks (1387 ± 108 ms)
🌐 Dapp Page Load BenchmarksCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
maxWait to sendUpdate debounce to fix UI starvationmaxWait to sendUpdate debounce to fix UI sync starvation
Contributor
Builds ready [50506af]
⚡ Performance Benchmarks (1435 ± 104 ms)
🌐 Dapp Page Load BenchmarksCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs
|
…buildUserTraitsObject` Replaces the blanket null-guard that silently dropped all trait updates (including non-`preferences`-derived traits like `is_metrics_opted_in`) when `preferences` was missing during intermediate state flushes. Replaces hard `driver.delay(2000)` in segment-user-traits e2e test with `driver.wait` polling for the expected trait values, eliminating timing-dependent flakiness.
50506af to
16efaaf
Compare
Contributor
Builds ready [16efaaf]
⚡ Performance Benchmarks (1376 ± 109 ms)
🌐 Dapp Page Load BenchmarksCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Contributor
Builds ready [e1a1c8f]
⚡ Performance Benchmarks (1434 ± 105 ms)
🌐 Dapp Page Load BenchmarksCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚀 Bundle size reduced!]
|
MajorLift
commented
Mar 4, 2026
MajorLift
commented
Mar 4, 2026
|
Contributor
Builds ready [c512fb6]
⚡ Performance Benchmarks
🌐 Dapp Page Load BenchmarksCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
HowardBraham
approved these changes
Mar 10, 2026
MajorLift
commented
Mar 11, 2026
Contributor
Author
MajorLift
left a comment
There was a problem hiding this comment.
Notes for reviewers:
| this.sendUpdate = debounce( | ||
| this.privateSendUpdate.bind(this), | ||
| MILLISECOND * 200, | ||
| { maxWait: SECOND }, // Force flush to avoid indefinite sync starvation |
| metamaskState.useTokenDetection, | ||
| [MetaMetricsUserTrait.ShowNativeTokenAsMainBalance]: | ||
| metamaskState.preferences.showNativeTokenAsMainBalance, | ||
| metamaskState.preferences?.showNativeTokenAsMainBalance ?? false, |
Contributor
Author
There was a problem hiding this comment.
The nullish fallback values in this file are based on getDefaultPreferencesControllerState
Comment on lines
-168
to
+172
| events = await getEventPayloads(driver, mockedEndpoints); | ||
| assert.equal(events.length, 1); | ||
| assert.deepStrictEqual(events[0].traits.is_metrics_opted_in, true); | ||
| assert.deepStrictEqual(events[0].traits.has_marketing_consent, true); | ||
| // maxWait on sendUpdate debounce may split the two toggles into | ||
| // separate identify events. Poll until the expected traits arrive. | ||
| await driver.wait(async () => { | ||
| try { | ||
| events = await getEventPayloads(driver, mockedEndpoints, false); |
Contributor
Author
There was a problem hiding this comment.
- Added polling logic for metametrics segment traits test.
- Handles edge case where toggle state updates are not batched together due to
maxWait.
hjetpoluru
approved these changes
Mar 11, 2026
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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
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.
Description
Adds
{ maxWait: MILLISECOND * 1000 }to thedebounceoptions forsendUpdateinmetamask-controller.js, forcing a periodic flush even under continuous state updates. This prevents the background-to-UI state sync from being starved indefinitely during heavy operations (e.g. syncing >100 accounts).Changelog
CHANGELOG entry: Fixed background-to-UI state sync starvation that caused UI to become stuck indefinitely during large account syncs.
Related issues
Fixes: https://github.com/MetaMask/MetaMask-planning/issues/6777
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Medium Risk
Adjusts background-to-UI state update scheduling; behavioral/timing changes could affect UI freshness and event ordering under heavy state churn. Includes an e2e test change to reduce flakiness, but edge-case timing regressions are still possible.
Overview
Prevents background-to-UI state sync starvation by adding
maxWait: SECONDto thesendUpdatedebounce inmetamask-controller.js, ensuring state updates flush periodically even during continuous updates.Hardens MetaMetrics user-trait generation in
metametrics-controller.tsby making severalpreferences-derived traits null-safe with defaults, and updates the metrics e2e test to tolerate identify events being split across multiple updates by polling/merging traits before asserting.Written by Cursor Bugbot for commit c512fb6. This will update automatically on new commits. Configure here.