[DO NOT MERGE] Capture v10 Sentry coverage for #42867 telemetry validation#43965
[DO NOT MERGE] Capture v10 Sentry coverage for #42867 telemetry validation#43965MajorLift wants to merge 41 commits into
Conversation
Agent-Logs-Url: https://github.com/MetaMask/metamask-extension/sessions/66deb05c-d6c0-4fae-953c-8842d8e47c25 Co-authored-by: MajorLift <34228073+MajorLift@users.noreply.github.com>
Agent-Logs-Url: https://github.com/MetaMask/metamask-extension/sessions/66deb05c-d6c0-4fae-953c-8842d8e47c25 Co-authored-by: MajorLift <34228073+MajorLift@users.noreply.github.com>
Agent-Logs-Url: https://github.com/MetaMask/metamask-extension/sessions/66deb05c-d6c0-4fae-953c-8842d8e47c25 Co-authored-by: MajorLift <34228073+MajorLift@users.noreply.github.com>
Agent-Logs-Url: https://github.com/MetaMask/metamask-extension/sessions/66deb05c-d6c0-4fae-953c-8842d8e47c25 Co-authored-by: MajorLift <34228073+MajorLift@users.noreply.github.com>
Agent-Logs-Url: https://github.com/MetaMask/metamask-extension/sessions/66deb05c-d6c0-4fae-953c-8842d8e47c25 Co-authored-by: MajorLift <34228073+MajorLift@users.noreply.github.com>
Agent-Logs-Url: https://github.com/MetaMask/metamask-extension/sessions/b03175d9-7847-4981-aaca-bd62f1537b2e Co-authored-by: MajorLift <34228073+MajorLift@users.noreply.github.com>
The `@sentry/browser` v8→v10 upgrade crashed the extension on boot under LavaMoat scuttling, failing every e2e job at `.controller-loaded`. v10's `browserTracingIntegration` registers an INP listener via `globalThis.addEventListener` synchronously inside `Sentry.init`, v10 web-vitals (`whenIdleOrHidden`) feature-detects `requestIdleCallback`, and v10 also reads `Request`/`WebAssembly`. Scuttling hid or threw on these. Webpack scuttles the shared realm with throwing getters, so add `WebAssembly`, `Request`, and `requestIdleCallback` to the `scuttleGlobalThis` exceptions in `LavamoatPlugin`. The browserify `sentry-install` bundle reaches Sentry through a proxy whose allowlist (`scuttlingConfigBase`) omitted `addEventListener`; add it and `removeEventListener`, bound to `window` like `setTimeout`.
The hook spread the raw Sentry `Span`; `@sentry/core` v10 exposes `undefined`
internal fields (e.g. `_endTime`) that fail the `snap_startTrace` response's
`Json` validation and reject the RPC — surfacing as an `UnexpectedAlertOpenError`
in the preinstalled-example snap e2e test. Return `{ traceId, spanId }` from
`spanContext()` instead.
Under the v10 SDK's startup timing the Home page mount reliably sets `pendingShieldCohort` before the test error is captured, so the masked Sentry UI-state snapshot is `string` rather than `null`.
…hrough v10 tracing emits hundreds of performance envelopes per test (~800 in a failing multichain run). Passing them through to real sentry.io starved e2e startup and flaked the non-EVM account render, and consumed the metamask-performance quota from CI. Return a canned 200 locally instead.
…pshots The field is populated by the Home page mount, so it is captured as `null` or a string depending on timing; under v10's startup it flips and breaks the state snapshot. Drop it from the comparison like other timing-dependent fields rather than pinning a racy value.
Bisection for the v10 non-EVM (Solana) account-render timeout: v8 renders the account ~instantly, v10 not within 10s, with identical envelope volume (~570) — so it is v10's heavier per-operation browser-tracing instrumentation (long-animation-frame / INP / fetch observers) at e2e's 100% sample rate, not the telemetry traffic. e2e does not consume these traces; skipping the integration there removes the overhead (and the CI quota draw).
The bisection it tested is refuted: skipping browserTracingIntegration in e2e did not fix the non-EVM account-render timeout, and it broke metrics/traces (no traces) and regressed metrics/errors. Restore the de-flaked state.
Second timing-dependent field in the same snapshot: the profile-sync SRP session is only present once account-sync establishes it, so its capture varies across runs. Drop it from the comparison like `pendingShieldCohort`.
The refuted e2e browser-tracing gating slipped back in because the prior revert moved the branch ref without updating the worktree. Restore setupSentry.js to its un-gated state; keep the srpSessionData de-flake.
Root cause of the snap/multichain e2e failures: the shared `waitForNonEvmAccountsLoaded` (hit by every login-based test) waits for the Solana/Bitcoin snap account icons with the default 10s timeout. The icons render in <1s locally, but on constrained 2-core CI runners the v10 Sentry SDK's heavier startup pushes the render just past 10s — so it's a runner timing margin, not a functional regression (the specs pass locally on v10). Allow 30s for these snap-backed icons.
Reconcile `yarn.lock` against the merged `package.json`, keeping the v10 `@sentry/*` resolutions alongside main's dependency bumps.
After a wallet unlock the controllers re-initialize, and the v10 Sentry SDK's heavier startup pushes `.controller-loaded` past the default 10s selenium wait on 2-core CI runners (`multichain-accounts/add-account.spec.ts` "added account should persist after wallet lock"). Bump the startup wait in `navigate` and `waitForControllersLoaded` to 30s without inflating the global `this.timeout`.
Resolve `errors.spec.ts`: drop `MetaMetricsController.latestNonAnonymousEventTimestamp` from `removedBackgroundFields` per main's #43556 (the field was removed from the controller), keeping the timing-dependent `pendingShieldCohort` / `srpSessionData` masks. Reconcile and dedupe `yarn.lock` against the merged `package.json` (material-ui→mui migration, `transaction-controller` 67→68).
The `.controller-loaded` and snap-backed non-EVM icon waits are polled ceilings, not fixed delays — they return as soon as their readiness signal appears. The earlier hard-coded 30s only mattered on a genuine hang, but it also slowed local failures from 10s to 30s for no benefit (local runners aren't the 2-core CI constraint). Replace the four literals with a single named `STARTUP_LOAD_TIMEOUT` (30s on CI, 10s locally): CI keeps the headroom for the heavier Sentry v10 startup, local fails fast again.
Port newly-merged trace-propagation code off the v10-removed `@sentry/utils`: `addFetchInstrumentationHandler` now imports from `@sentry/core`, and the scope propagation context's `spanId` is read as v10's renamed `propagationSpanId` (`sentry-trace-propagation.ts` + its test). Reconcile and dedupe `yarn.lock` against the merged `package.json`. Drop the e2e startup-timeout band-aids (`STARTUP_LOAD_TIMEOUT` and the 30s `.controller-loaded` / non-EVM-icon bumps). The "heavier v10 startup" premise behind them was never measured, so restore the default 10s waits and let CI reveal whether the slowness is real rather than masking it.
On 2-core CI the Solana/Bitcoin snap account discovery hits an unmocked-network retry storm (~9s) before the icons render, landing at the default 10s wait. Give `waitForNonEvmAccountsLoaded` a 20s ceiling as a stopgap; it stays polled, so the happy path is unaffected. Not a Sentry/v10 issue — root cause and the real fix (globalize the snap discovery mocks) are tracked in #43817; revert this then.
…re globalized #43818 globalized the Bitcoin esplora and Solana `getSignaturesForAddress` discovery mocks in `mock-e2e.js`, eliminating the unmocked-network retry storm that delayed non-EVM icon render. `waitForNonEvmAccountsLoaded` returns to the default timeout, matching `main`. Resolves the stopgap tracked by #43817.
… v10 Reverting the stopgap removal: with the default 10s wait, three webpack-chrome e2e shards time out on `img[src="./images/bitcoin-logo.svg"]` (`test-snap-bip-44`, `multichain-account-list-menu`, `add-account`), each at ~10s. #43818's globalized Bitcoin/Solana discovery mocks are present in this branch but the non-EVM icons still render between 10s and 20s on the v10 SDK, so the widened wait is required. The residual latency (v10 snap-account discovery vs. a remaining unmocked call) needs a separate root-cause pass.
Root cause of the slow non-EVM account icons (and the e2e timeouts that the
`NON_EVM_ICON_TIMEOUT` stopgap was masking): the Bitcoin snap requests
`GET /esplora/block-height/{n}` during account discovery, but the global
`setupDefaultNonEvmDiscoveryMocks` only covered `/blocks`, `/blocks/tip/*`,
`/scripthash/*` and `/fee-estimates`. The unmocked `/block-height` request fell
through to the generic empty-200 catch-all, whose malformed body throws in the
snap and restarts the entire discovery cycle. The failing CI run shows the storm
directly: `/blocks`, `/scripthash/<h>/txs` and `/block-height/0` each requested
97 times in one spec, so the Bitcoin icon never renders within the default wait.
The snap is byte-identical on v8 and v10, so the gap predates this PR; the v10
SDK's far heavier startup envelope volume (~1100 Sentry POSTs per spec vs. a
fraction of that on v8) is what tips the same storm past the 10s threshold v8
fit under. Mocking `/block-height` lets discovery complete in a single pass on
both, eliminating the storm at its source and removing the need for the stopgap.
The previous mock returned the chain-tip hash for every `/block-height/{n}`, but
the Bitcoin snap only ever requests `/block-height/0` — the genesis block — and
validates the response as a network-identity check. A non-genesis hash there
makes the snap throw during account creation (`bitcoin-wallet-snap was stopped
... the Snap crashed`), which the previous fix surfaced once discovery stopped
storming. Return the canonical mainnet genesis hash for height 0 (tip for any
other height) so account creation completes.
Bitcoin discovery is now fully mocked in this PR, but the Solana snap's other 14 discovery RPC methods (incl. the `getGenesisHash` network-identity check) remain unmocked in `setupDefaultNonEvmDiscoveryMocks`, so they fall through to the empty-200 catch-all and drive a ~516-request retry storm that delays the Solana icon past the default wait — amplified by the v10 SDK's heavier startup envelope volume. Widen the wait as an explicit, root-caused interim until #43958 completes the Solana discovery mocks, at which point this is removed.
|
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. |
mm-token-exchange-service
Bot
added
the
team-extension-platform
✨ Files requiring CODEOWNER review ✨👨🔧 @MetaMask/extension-platform (3 files, +11 -0)
📜 @MetaMask/policy-reviewers (8 files, +216 -296)
Tip Follow the policy review process outlined in the LavaMoat Policy Review Process doc before expecting an approval from Policy Reviewers. 🧪 @MetaMask/qa (2 files, +63 -11)
👨🔧 @itsyoboieltr (3 files, +11 -0)
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Caution MetaMask internal reviewing guidelines:
|
|
Builds ready [d9b183f]
⚡ Performance Benchmarks (Total: 🟢 14 pass · 🟡 11 warn · 🔴 0 fail)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Throwaway capture branch off #42867 (v10). Runs the #43820 coverage spec in log-only mode to emit
[SENTRY-COVERAGE-SUMMARY]for the v10 build, paired with the v8 capture (#43964). Closed after the summary is read. Tracking: #43819 / #42867.