-
Notifications
You must be signed in to change notification settings - Fork 610
feat: Add snap_trackError
method for error tracking through Sentry
#3498
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
snap_trackError
method for error tracking through Sentrysnap_trackError
method for error tracking through Sentry
aa0e05f
to
1ddb30c
Compare
@metamaskbot update-pr |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3498 +/- ##
=======================================
Coverage 98.22% 98.23%
=======================================
Files 405 406 +1
Lines 11412 11453 +41
Branches 1779 1778 -1
=======================================
+ Hits 11210 11251 +41
Misses 202 202 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
) { | ||
return error.data; | ||
const value = getObjectProperty(error, 'data'); | ||
if (value !== null && isValidJson(value) && !Array.isArray(value)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we actually need to check for full JSON validity here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's what we did previously, I'm not sure if it's really needed, but kept it to be on the safe side.
@metamaskbot update-pr |
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** This adds support for the `snap_trackError` RPC method. See MetaMask/snaps#3498 for more details. [](https://codespaces.new/MetaMask/metamask-extension/pull/33866?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** This adds support for the `snap_trackError` RPC method. See MetaMask/snaps#3498 for more details. [](https://codespaces.new/MetaMask/metamask-extension/pull/33866?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** This adds support for the `snap_trackError` RPC method. See MetaMask/snaps#3498 for more details. [](https://codespaces.new/MetaMask/metamask-extension/pull/33866?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
In order to track failed RPC requests, we've added an `errorTrackingTransport` that wraps the existing transport pipeline to track failed RPC requests. It integrates with the current architecture and uses the new `snap_trackError` method to capture error details. This transport wrapper acts as a lightweight solution, for us to have better logs and debugging capabilities. Reference: MetaMask/snaps#3498 What it does: - Catches HTTP errors (4xx, 5xx) - Detects JSON-RPC errors that come back in 2xx responses - Preserves the original error flow so nothing breaks
This adds a new RPC method
snap_trackError
, which can be used to track errors in Sentry. It accepts aname
,message
, andstack
, which should be turned into a properError
class again on the client end before sending it to Sentry withSentry.captureException
.To make the API a bit easier to use, I've added a
getJsonError
helper function which takes an error (string,Error
class, or other value) and turns it into the properties expected bysnap_trackError
: