fix(sentry): defer breadcrumb scrubbing to send time cp-13.36.0#43600
Merged
Conversation
Deep address sanitization in beforeBreadcrumb blocked the main thread during onboarding when MetaMetrics was enabled. Scrub breadcrumbs when errors and transactions are sent instead, and stop logging the Rive runtime object on the wallet-ready screen. Co-authored-by: Cursor <cursoragent@cursor.com>
gauthierpetetin
temporarily deployed
to
pr-comment
GitHub Actions
Inactive
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. |
✨ Files requiring CODEOWNER review ✨🔐 @MetaMask/web3auth (1 files, +1 -1)
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses a Sentry-related performance regression by deferring deep breadcrumb scrubbing until events are actually sent to Sentry, rather than doing heavy sanitization work at breadcrumb capture time (which can run for every fetch/console breadcrumb when analytics is enabled).
Changes:
- Stop deep-scrubbing in
beforeBreadcrumband instead sanitize breadcrumb payloads at send time for both errors (beforeSend) and transactions (beforeSendTransaction). - Add
rewriteTransactionReportand expand test coverage to assert breadcrumb scrubbing for both error and transaction events. - Prevent Sentry console breadcrumbs from capturing a large Rive runtime object by removing it from a
console.log.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| ui/pages/onboarding-flow/creation-successful/wallet-ready-animation.tsx | Avoids logging the full Rive runtime object so Sentry’s console breadcrumb capture is lighter. |
| app/scripts/lib/setupSentry.js | Moves breadcrumb deep-scrubbing to send-time hooks and adds transaction send-time scrubbing. |
| app/scripts/lib/setupSentry.test.js | Adds unit tests for breadcrumb address/url scrubbing on both error and transaction reports. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Remove per-breadcrumb cloneDeep now that beforeSend already clones the report, fix JSDoc wording, and add a test that live breadcrumb sources are not mutated during rewriteReport. Co-authored-by: Cursor <cursoragent@cursor.com>
gauthierpetetin
temporarily deployed
to
pr-comment
GitHub Actions
Inactive
gauthierpetetin
temporarily deployed
to
pr-comment
GitHub Actions
Inactive
Builds ready [68fefa2]
⚡ Performance Benchmarks (Total: 🟢 16 pass · 🟡 8 warn · 🔴 1 fail)
Bundle size diffs [🚀 Bundle size reduced!]
|
Contributor
|
Tested locally in FF and the regression is gone ✅ |
DDDDDanica
approved these changes
Jun 16, 2026
MajorLift
approved these changes
Jun 16, 2026
MajorLift
left a comment
MajorLift
left a comment
Contributor
There was a problem hiding this comment.
- Doesn't affect any existing instrumentation. Everything in
beforeSendis still happening. - No additions to
beforeSendTransactionnecessary to avoid breaking changes other thanremoveUrlsFromBreadCrumb(moved out ofbeforeBreadcrumb). - The Rive object causing a performance error should be documented. I'll note that in MetaMask/skills#41
- Firefox UI freezing non-reproable on local branch.
gauthierpetetin
added
regression-RC-13.36.0
lionellbriones
approved these changes
Jun 16, 2026
metamaskbot
added
the
release-13.37.0
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.
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
We've noticed an important performance regression (especially on Firefox) after #43337 was merged (see Slack thread). That PR expanded address sanitization beyond EVM account addresses (Bitcoin, Solana, Tron, etc.). The fact that it resulted in a performance regression showed that the breadcrumb scrubbing function was being executed not only before sending error events to Sentry, but also on every fetch/console event when analytics was on.
In reality, we're not interested in scrubbing breadcrumbs at capture time, only at send time.
This PR fixes it by:
beforeBreadcrumbonly filters, it no longer deep-scrubs)rewriteReport(beforeSend), and transactions viarewriteTransactionReport(beforeSendTransaction)Also, this PR avoids logging the full Rive runtime object on the wallet-ready screen, which Sentry's console integration would otherwise capture into breadcrumbs, thus slowing down the app.
Changelog
CHANGELOG entry: Fixes a performance issue that was slowing down the app
Related issues
Fixes: None
Manual testing steps
yarn start:mv2 --sentryScreenshots/Recordings
None
Pre-merge author checklist
Pre-merge reviewer checklist