diff --git a/packages/snap-networks-utils/src/utils/errors/trackError.ts b/packages/snap-networks-utils/src/utils/errors/trackError.ts index e772ca04..55d7d858 100644 --- a/packages/snap-networks-utils/src/utils/errors/trackError.ts +++ b/packages/snap-networks-utils/src/utils/errors/trackError.ts @@ -109,7 +109,8 @@ export function createTrackError({ * @param options.getSnapProvider - Returns the Snap provider used for `snap_trackError`. * @param options.prepareError - Optional error normalizer before Sentry serialization. * @param options.shouldTrack - Optional tracking filter passed to {@link createTrackError}. - * @returns Bound `trackError` and `withCatchAndThrowSnapError` functions. + * @returns Bound {@link createTrackError trackError} and + * {@link createWithCatchAndThrowSnapError withCatchAndThrowSnapError} functions. */ export function createSnapErrorHandling< TProvider extends TrackErrorCapableProvider, diff --git a/packages/tron-wallet-snap/src/clients/snap/SnapClient.test.ts b/packages/tron-wallet-snap/src/clients/snap/SnapClient.test.ts index 0706c5ca..b41b14ee 100644 --- a/packages/tron-wallet-snap/src/clients/snap/SnapClient.test.ts +++ b/packages/tron-wallet-snap/src/clients/snap/SnapClient.test.ts @@ -4,6 +4,11 @@ import { UserRejectedRequestError } from '@metamask/snaps-sdk'; import { mockLogger } from '../../utils/mockLogger'; import { SnapClient } from './SnapClient'; +jest.mock('../../utils/logger', () => ({ + __esModule: true, + default: jest.requireActual('../../utils/mockLogger').mockLogger, +})); + // Mock the global snap object const mockSnapRequest = jest.fn(); (globalThis as any).snap = { @@ -25,7 +30,7 @@ async function withSnapClient( }) => void | Promise, ) { mockSnapRequest.mockReset(); - const snapClient = new SnapClient({ logger: mockLogger }); + const snapClient = new SnapClient(); await testFn({ snapClient, mockSnapRequest, mockLogger }); } @@ -131,7 +136,7 @@ describe('SnapClient', () => { expect(result).toBeUndefined(); expect(mockRequest).not.toHaveBeenCalled(); - expect(logger.warn).not.toHaveBeenCalled(); + expect(logger.error).not.toHaveBeenCalled(); }, ); }); @@ -161,12 +166,12 @@ describe('SnapClient', () => { }), }, }); - expect(logger.warn).not.toHaveBeenCalled(); + expect(logger.error).not.toHaveBeenCalled(); }, ); }); - it('swallows RPC failures and logs a warning', async () => { + it('swallows RPC failures and logs an error', async () => { await withSnapClient( async ({ snapClient, @@ -179,11 +184,10 @@ describe('SnapClient', () => { const result = await snapClient.trackError(new Error('x')); expect(result).toBeUndefined(); - expect(logger.warn).toHaveBeenCalledTimes(1); - expect(logger.warn).toHaveBeenCalledWith( - expect.any(String), - expect.objectContaining({ rpcError }), - expect.stringContaining('Failed to track error'), + expect(logger.error).toHaveBeenCalledTimes(1); + expect(logger.error).toHaveBeenCalledWith( + { error: rpcError }, + 'Failed to track error', ); }, ); diff --git a/packages/tron-wallet-snap/src/clients/snap/SnapClient.ts b/packages/tron-wallet-snap/src/clients/snap/SnapClient.ts index 16f671ad..eb2a8605 100644 --- a/packages/tron-wallet-snap/src/clients/snap/SnapClient.ts +++ b/packages/tron-wallet-snap/src/clients/snap/SnapClient.ts @@ -1,7 +1,5 @@ import type { JsonSLIP10Node } from '@metamask/key-tree'; import type { EntropySourceId } from '@metamask/keyring-api'; -import type { Logger } from '@metamask/snap-networks-utils'; -import { getJsonError, UserRejectedRequestError } from '@metamask/snaps-sdk'; import type { DialogResult, EntropySource, @@ -13,19 +11,13 @@ import type { import { SecurityEventType, TransactionEventType } from '../../types/analytics'; import type { Preferences } from '../../types/snap'; -import { sanitizeSensitiveError } from '../../utils/sensitiveErrors'; +import { trackError as reportErrorToSentry } from '../../utils/errors'; /** * Client for interacting with the Snap API. * Provides methods for managing interfaces, dialogs, preferences, and background events. */ export class SnapClient { - readonly #logger: Logger; - - constructor({ logger }: { logger: Logger }) { - this.#logger = logger.withPrefix('[📡 SnapClient]'); - } - /** * Retrieves a `SLIP10NodeInterface` object for the specified path and curve. * @@ -258,22 +250,7 @@ export class SnapClient { * @returns The Sentry event ID on success, or `undefined` on failure or if the error is skipped. */ async trackError(error: Error): Promise { - if (error instanceof UserRejectedRequestError) { - return undefined; - } - - try { - return await snap.request({ - method: 'snap_trackError', - params: { error: getJsonError(sanitizeSensitiveError(error)) }, - }); - } catch (rpcError) { - this.#logger.warn( - { rpcError }, - 'Failed to track error via snap_trackError', - ); - return undefined; - } + return reportErrorToSentry(error); } /** diff --git a/packages/tron-wallet-snap/src/clients/snap/getSnapProvider.ts b/packages/tron-wallet-snap/src/clients/snap/getSnapProvider.ts new file mode 100644 index 00000000..fffb00f1 --- /dev/null +++ b/packages/tron-wallet-snap/src/clients/snap/getSnapProvider.ts @@ -0,0 +1,11 @@ +import type { SnapsProvider } from '@metamask/snaps-sdk'; + +/** + * Returns the Snap provider. + * + * @returns The Snap provider. + */ +export function getSnapProvider(): SnapsProvider { + // snap is a global variable provided by the Snap SDK + return snap; +} diff --git a/packages/tron-wallet-snap/src/context.ts b/packages/tron-wallet-snap/src/context.ts index 700d3510..c400f144 100644 --- a/packages/tron-wallet-snap/src/context.ts +++ b/packages/tron-wallet-snap/src/context.ts @@ -69,7 +69,7 @@ const state = new State({ }, }); -const snapClient = new SnapClient({ logger }); +const snapClient = new SnapClient(); // Repositories - depend on State const accountsRepository = new AccountsRepository(state); diff --git a/packages/tron-wallet-snap/src/utils/errors.test.ts b/packages/tron-wallet-snap/src/utils/errors.test.ts index bed6cfc5..e890ff98 100644 --- a/packages/tron-wallet-snap/src/utils/errors.test.ts +++ b/packages/tron-wallet-snap/src/utils/errors.test.ts @@ -1,33 +1,58 @@ import { SnapError, UserRejectedRequestError } from '@metamask/snaps-sdk'; -import { withCatchAndThrowSnapError } from './errors'; +import { trackError, withCatchAndThrowSnapError } from './errors'; import { mockLogger } from './mockLogger'; -jest.mock('../clients/snap/SnapClient', () => { - const trackError = jest.fn(); - - return { - trackError, - SnapClient: jest.fn().mockImplementation(() => ({ - trackError, - })), - }; -}); - jest.mock('./logger', () => ({ __esModule: true, default: jest.requireActual('./mockLogger').mockLogger, })); -const { trackError } = jest.requireMock('../clients/snap/SnapClient'); +const setupTest = (): { mockSnapRequest: jest.Mock } => { + jest.clearAllMocks(); + + const mockSnapRequest = jest.fn(); + Object.defineProperty(globalThis, 'snap', { + configurable: true, + value: { request: mockSnapRequest }, + writable: true, + }); + + return { mockSnapRequest }; +}; describe('errors', () => { - beforeEach(() => { - jest.clearAllMocks(); + describe('trackError', () => { + it('does not track UserRejectedRequestError', async () => { + const { mockSnapRequest } = setupTest(); + + expect(await trackError(new UserRejectedRequestError())).toBeUndefined(); + + expect(mockSnapRequest).not.toHaveBeenCalled(); + }); + + it('sanitizes errors before tracking', async () => { + const { mockSnapRequest } = setupTest(); + mockSnapRequest.mockResolvedValue('tracked-error-id'); + + await trackError(new Error('Failed to derive private key')); + + expect(mockSnapRequest).toHaveBeenCalledWith({ + method: 'snap_trackError', + params: { + error: expect.objectContaining({ + message: + 'Key derivation failed. Please check your connection and try again.', + }), + }, + }); + }); }); describe('withCatchAndThrowSnapError', () => { it('returns the result when the function succeeds', async () => { + setupTest(); + const mockFn = jest.fn().mockResolvedValue('success'); const result = await withCatchAndThrowSnapError(mockFn); @@ -38,6 +63,9 @@ describe('errors', () => { }); it('tracks, logs, and re-throws errors as SnapError', async () => { + const { mockSnapRequest } = setupTest(); + mockSnapRequest.mockResolvedValue('tracked-error-id'); + const originalError = new Error('Test error'); const mockFn = jest.fn().mockRejectedValue(originalError); @@ -45,12 +73,13 @@ describe('errors', () => { SnapError, ); - expect(trackError).toHaveBeenCalledWith(originalError); + expect(mockSnapRequest).toHaveBeenCalledTimes(1); expect(mockFn).toHaveBeenCalledTimes(1); expect(mockLogger.error).toHaveBeenCalledTimes(1); }); - it('delegates tracking to SnapClient for user rejections', async () => { + it('skips tracking RPC for user rejections', async () => { + const { mockSnapRequest } = setupTest(); const mockFn = jest .fn() .mockRejectedValue(new UserRejectedRequestError()); @@ -59,9 +88,7 @@ describe('errors', () => { UserRejectedRequestError, ); - expect(trackError).toHaveBeenCalledWith( - expect.any(UserRejectedRequestError), - ); + expect(mockSnapRequest).not.toHaveBeenCalled(); }); }); }); diff --git a/packages/tron-wallet-snap/src/utils/errors.ts b/packages/tron-wallet-snap/src/utils/errors.ts index d2d33e01..b8b2a7de 100644 --- a/packages/tron-wallet-snap/src/utils/errors.ts +++ b/packages/tron-wallet-snap/src/utils/errors.ts @@ -1,13 +1,14 @@ -import { createWithCatchAndThrowSnapError } from '@metamask/snap-networks-utils'; +import { createSnapErrorHandling } from '@metamask/snap-networks-utils'; -import { SnapClient } from '../clients/snap/SnapClient'; +import { getSnapProvider } from '../clients/snap/getSnapProvider'; import logger from './logger'; +import { sanitizeSensitiveError } from './sensitiveErrors'; export { isSnapRpcError, sanitizeSensitiveError } from './sensitiveErrors'; -const snapClient = new SnapClient({ logger }); - -export const withCatchAndThrowSnapError = createWithCatchAndThrowSnapError({ - logError: logger.error.bind(logger), - trackError: (error) => snapClient.trackError(error as Error), -}); +export const { trackError, withCatchAndThrowSnapError } = + createSnapErrorHandling({ + getSnapProvider, + logError: logger.error.bind(logger), + prepareError: sanitizeSensitiveError, + });