fix: Fix MV2 build sourcemap upload#26467
Conversation
development/sentry-publish.js
Outdated
There was a problem hiding this comment.
From local testing, it seems that this was not working. It was returning empty even for releases with artifacts. I think this is a legacy command that no longer works due to a change in how Sentry tracks artifacts that was made >1 year ago (they're no longer considered as "release bundles", they're "artifact bundles" now, which aren't returned here.)
Since it wasn't working anyway, I removed it rather than trying to update it to use --dist correctly (it didn't seem to accept a --dist flag, as far as I could tell, so there was no easy way to get this to work).
development/sentry-publish.js
Outdated
There was a problem hiding this comment.
This reflects the directory names we use in the CircleCI config
0001ba3 to
c3b092e
Compare
|
LGTM ! |
Builds ready [c3b092e]
Page Load Metrics (335 ± 298 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #26467 +/- ##
===========================================
- Coverage 70.02% 70.02% -0.00%
===========================================
Files 1406 1406
Lines 49076 49078 +2
Branches 13725 13727 +2
===========================================
Hits 34364 34364
- Misses 14712 14714 +2 ☔ View full report in Codecov by Sentry. |
|
I manually tested this and found one bug, two parameters were in the wrong order. Fixed in be3203f It appears to work correctly now. |
Builds ready [be3203f]
Page Load Metrics (76 ± 15 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Builds ready [351625e]
Page Load Metrics (257 ± 263 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
|
Found a problem with the MV3 builds. They're still showing as having a |
|
The issue referenced here was resolved in this commit. This is ready for review again, I have re-tested it. Updating the testing instructions now. |
Builds ready [011ff47]
Page Load Metrics (349 ± 312 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
|
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. |
The sourcemaps for MV2 builds (used for Firefox) were not being uploaded to Sentry at all. This resulted in invalid stack traces for Firefox error reports. The Sentry initiatization has been updated to use a `dist` option, letting us differentiate between different types of build for the same version. This is now used to signify which builds are mv2 and which are mv3. Both distributions are uploaded separately as part of the release process (for Flask and main builds; we don't have MV2 builds for MMI or beta). Fixes #26466
This reverts commit 4bd50d4.
I was seeing a bug where MV3 builds were still set as `dist: 'mv2'`. This was because `isManifestV3` was resolving to `false` in the `sentry-install.js` bundle for some reason. It turns out that the runtime manifest is unavailable because the Chrome extension API doesn't exist in the global scope early on in a service worker process. It magically appears some time later. So in this case, the fallback `process.env.ENABLE_MV3` was used. This fallback case was broken because it expected `ENABLE_MV3` to be set to the string `"true"`, but it was set to the boolean `true`. The fallback check has been updated to account for both cases.
85b1aa2 to
4c27315
Compare
|
desi
left a comment
There was a problem hiding this comment.
I don't see anything here that is alarming from a circle ci point of view and trust that the other reviewers would have caught issues in the other areas I am less familiar with. LGTM.
Builds ready [4c27315]
Page Load Metrics (76 ± 8 ms)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|




Description
The sourcemaps for MV2 builds (used for Firefox) were not being uploaded to Sentry at all. This resulted in invalid stack traces for Firefox error reports.
The Sentry initiatization has been updated to use a
distoption, letting us differentiate between different types of build for the same version. This is now used to signify which builds are mv2 and which are mv3. Both distributions are uploaded separately as part of the release process (for Flask and main builds; we don't have MV2 builds for MMI or beta).Related issues
Fixes #26466
Manual testing steps
Unfortunately I don't know of an easy way to test the CircleCI changes. Those will just have to be read carefully.
We can test the application changes and the Sentry script changes though. The two outcomes we want to test are:
Here are the steps I used to test this:
metamaskprojectorg:ciscope, which is the only scope available for Sentry auth tokens.package.json(just in case you need to re-test, this is an easy way to separate old errors/builds from new ones)SENTRY_DSN_DEVis set to the DSN of your personal Sentry account'smetamaskprojectyarn dist:mv2to create an MV2 builddist-mv2directory (mv dist dist-mv2)yarn distto create an MV3 buildSENTRY_AUTH_TOKENis set to the Auth Token generated from the custom integration earlier.yarn sentry:publish --org [your organization]to upload the MV3 buildyarn sentry:publish --org [your organization] --dist mv2to upload the MV2 buildAt this point, you should be able to see the releases on the Sentry dashboard along with the artifacts. Look in "Settings -> Projects -> Source Maps" for these. They are labeled by release number and dist.
Now, load each build in your browser (one at a time, never both enabled at once) and follow these steps:
transactionController.updateSecurityAlertResponse(is mapped to source code correctly (it should resolve toapp/scripts/lib/ppom/ppom-util.ts). Also check that thedistis correct in the "Tags" section of the Sentry issue page.Here are screenshots of what that looked like for me:
Screenshots/Recordings
N/A
Pre-merge author checklist
Pre-merge reviewer checklist