feat: integrate error tracking service with RPC handlers #242
Conversation
mj-kiwi
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| // By default, track all errors that are Error instances or have error-like properties | ||
| return ( | ||
| error instanceof Error || | ||
| (typeof error === 'object' && (error?.message ?? error?.error)) |
There was a problem hiding this comment.
Nullish coalescing causes errors to go untracked
Medium Severity
The shouldTrackError function uses ?? (nullish coalescing) instead of || (logical OR) in the expression error?.message ?? error?.error. This means error objects with an empty string message property but a truthy error property won't be tracked, even though they have error-like properties. The ?? operator only checks for null/undefined, so { message: '', error: 'something' } evaluates to '' (falsy) and won't be tracked.
| const errorInfo: ErrorTrackingInfo = { | ||
| snapName: this.#snapName, | ||
| method, | ||
| errorMessage: `Response error: ${JSON.stringify(response)}`, |
There was a problem hiding this comment.
Unprotected JSON.stringify can throw during error capture
Low Severity
JSON.stringify() calls in captureResponseError (line 180) and #extractErrorInfo (line 82) occur outside the try-catch block in #trackErrorViaSnap. If the input contains circular references, BigInt values, or other non-serializable data, these calls throw unhandled exceptions. This defeats the intent to "safely handle error tracking without throwing additional errors" as stated in the code comments.
Additional Locations (1)
MoMannn
left a comment
MoMannn
left a comment
There was a problem hiding this comment.
Also some cursor bug bot things to address
There was a problem hiding this comment.
Why do we need this? We are not using it anywhere? Also this code is duplicated in permissions-kernel-snap and not used there either?
packages/shared/package.json
Outdated
| "./constants": "./src/constants/index.ts", | ||
| "./testing": "./src/testing/index.ts" | ||
| "./testing": "./src/testing/index.ts", | ||
| "./error-tracking": "./src/utils/errorTracking.ts" |
There was a problem hiding this comment.
Why is this exported here? its already exported in index.ts?
… and improve console warning implementation
Description
This PR improves error observability for the gator snap by introducing Sentry-based error tracking via MetaMask.
A new
ErrorTrackerabstraction is added, which reports errors using thesnap_trackErrormethod so that runtime failures inside snaps can be surfaced in Sentry. Sincesnap_trackErrorcurrently only supports a basicerrorpayload and does not expose Sentry-specific features such astags, relevant snap metadata (e.g. snap name) is embedded into the error message itself.On the Sentry side, alerts are configured to filter errors originating from
gator-permissions-snapandpermissions-kernel-snap, and forward those errors to the team via email notifications. This allows us to proactively monitor snap errors and respond more quickly to production issues.Related issues
Fixes: N/A
Manual testing steps
ErrorTrackercallssnap_trackErrorwith the expected error payload.Screenshots/Recordings
Before
After
Email as below
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Adds centralized error tracking and integrates it into RPC flows for better observability.
SnapErrorTrackerwithcreateErrorTracker/getErrorTracker,captureError, andcaptureResponseErrorin@metamask/7715-permissions-shared/utils/errorTracking; re-exported via./utilsand added export./error-tracking. Added@metamask/snaps-sdkdependency.snapName: 'gator-permissions-snap') and wrapsonRpcRequestin try/catch to report viaerrorTracker.captureError. Added optional RPC wrappers inrpc/rpcErrorHandler.tsfor per-method tracking.snapName: 'permissions-kernel-snap') and reports errors inonRpcRequest. Addedrpc/rpcErrorHandler.tswith wrapper utilities.yarn.lockand package exports.Written by Cursor Bugbot for commit 244595a. This will update automatically on new commits. Configure here.