feat: use shared trackError in bitcoin

packages/bitcoin-wallet-snap/snap.manifest.json

@@ -7,7 +7,7 @@
     "url": "https://github.com/MetaMask/internal-snaps.git"
   },
   "source": {
-    "shasum": "ZEMZtj4BZq+pj3bzb81K1Vk5WQuqMA0NArfIhrIppds=",
+    "shasum": "1Gyxepuru4+QGV7t0OWvBvv9BP59TnaMa50o57WQ4ik=",
     "location": {
       "npm": {
         "filePath": "dist/bundle.js",

packages/bitcoin-wallet-snap/src/handlers/HandlerMiddleware.test.ts

@@ -1,9 +1,14 @@
 import type { GetPreferencesResult } from '@metamask/snaps-sdk';
 import { mock } from 'jest-mock-extended';
 
-import { BaseError, ExternalServiceError, UserActionError } from '../entities';
+import { BaseError, ExternalServiceError } from '../entities';
 import type { Logger, SnapClient, Translator } from '../entities';
-import { HandlerMiddleware, shouldTrackError } from './HandlerMiddleware';
+import { trackError } from '../utils/errors';
+import { HandlerMiddleware } from './HandlerMiddleware';
+
+jest.mock('../utils/errors', () => ({
+  trackError: jest.fn(),
+}));
 
 describe('HandlerMiddleware', () => {
   const mockLogger = mock<Logger>();
@@ -13,6 +18,7 @@ describe('HandlerMiddleware', () => {
   const mockTranslator = mock<Translator>({
     load: jest.fn(),
   });
+  const mockTrackError = jest.mocked(trackError);
 
   const middleware = new HandlerMiddleware(
     mockLogger,
@@ -22,33 +28,13 @@ describe('HandlerMiddleware', () => {
 
   beforeEach(() => {
     jest.clearAllMocks();
+    mockTrackError.mockResolvedValue(undefined);
     mockSnapClient.getPreferences.mockResolvedValue({
       locale: 'en',
     } as GetPreferencesResult);
     mockTranslator.load.mockResolvedValue({});
   });
 
-  describe('shouldTrackError', () => {
-    it('returns false for canceled confirmation errors', () => {
-      expect(
-        shouldTrackError(
-          new UserActionError('User canceled the confirmation'),
-          mockLogger,
-        ),
-      ).toBe(false);
-    });
-
-    it('returns true for other errors', () => {
-      expect(shouldTrackError(new Error('boom'), mockLogger)).toBe(true);
-      expect(
-        shouldTrackError(
-          new UserActionError('Another user action'),
-          mockLogger,
-        ),
-      ).toBe(true);
-    });
-  });
-
   describe('handle', () => {
     it('executes the function successfully', async () => {
       const mockFn = jest.fn().mockResolvedValue('success');
@@ -66,23 +52,24 @@ describe('HandlerMiddleware', () => {
       expect(mockSnapClient.getPreferences).toHaveBeenCalled();
       expect(mockTranslator.load).toHaveBeenCalledWith('en');
       expect(mockLogger.error).toHaveBeenCalledWith(error);
+      expect(mockTrackError).toHaveBeenCalledWith(error);
     });
 
     it('tracks an unexpected Error before rethrowing it as a SnapError', async () => {
       const error = new Error('tracked boom');
       const mockFn = jest.fn().mockRejectedValue(error);
 
       await expect(middleware.handle(mockFn)).rejects.toThrow('tracked boom');
-      expect(mockSnapClient.emitTrackingError).toHaveBeenCalledWith(error);
+      expect(mockTrackError).toHaveBeenCalledWith(error);
     });
 
-    it('continues to throw a SnapError when emitTrackingError fails', async () => {
+    it('continues to throw a SnapError when trackError is invoked', async () => {
       const error = new Error('boom after tracking failure');
       const mockFn = jest.fn().mockRejectedValue(error);
 
       await expect(middleware.handle(mockFn)).rejects.toThrow(error);
 
-      expect(mockSnapClient.emitTrackingError).toHaveBeenCalledWith(error);
+      expect(mockTrackError).toHaveBeenCalledWith(error);
       expect(mockSnapClient.getPreferences).toHaveBeenCalled();
     });
 
@@ -91,6 +78,7 @@ describe('HandlerMiddleware', () => {
 
       await expect(middleware.handle(mockFn)).rejects.toThrow('string failure');
       expect(mockLogger.error).toHaveBeenCalledWith('string failure');
+      expect(mockTrackError).toHaveBeenCalledWith('string failure');
     });
 
     it('wraps a thrown plain object by stringifying it', async () => {
@@ -101,6 +89,7 @@ describe('HandlerMiddleware', () => {
         '[object Object]',
       );
       expect(mockLogger.error).toHaveBeenCalledWith(thrown);
+      expect(mockTrackError).toHaveBeenCalledWith(thrown);
     });
 
     it('uses the message property if it exists on a thrown plain object', async () => {
@@ -111,6 +100,7 @@ describe('HandlerMiddleware', () => {
         'InsufficientFunds',
       );
       expect(mockLogger.error).toHaveBeenCalledWith(thrown);
+      expect(mockTrackError).toHaveBeenCalledWith(thrown);
     });
 
     it('handles error successfully if instance of BaseError', async () => {
@@ -124,7 +114,7 @@ describe('HandlerMiddleware', () => {
       expect(mockSnapClient.getPreferences).toHaveBeenCalled();
       expect(mockTranslator.load).toHaveBeenCalledWith('en');
       expect(mockLogger.error).toHaveBeenCalledWith(error, error.data);
-      expect(mockSnapClient.emitTrackingError).toHaveBeenCalledWith(error);
+      expect(mockTrackError).toHaveBeenCalledWith(error);
     });
 
     it('includes the concrete external service failure in the returned error message', async () => {
@@ -139,7 +129,7 @@ describe('HandlerMiddleware', () => {
       await expect(middleware.handle(mockFn)).rejects.toThrow(
         'Connection error: Failed to synchronize account',
       );
-      expect(mockSnapClient.emitTrackingError).toHaveBeenCalledWith(error);
+      expect(mockTrackError).toHaveBeenCalledWith(error);
     });
   });
 });

packages/bitcoin-wallet-snap/src/handlers/HandlerMiddleware.ts

@@ -26,24 +26,7 @@ import {
   WalletError,
   AssertionError,
 } from '../entities';
-
-/**
- * Determines whether an error should be reported through `snap_trackError`.
- *
- * @param error - The error to evaluate.
- * @param logger - logger for error
- * @returns `true` when the error should be tracked.
- */
-export function shouldTrackError(error: unknown, logger: Logger): boolean {
-  try {
-    return !(
-      (error as UserActionError)?.message === 'User canceled the confirmation'
-    );
-  } catch {
-    logger.error(error, 'Failed to determine if error should be tracked');
-    return false;
-  }
-}
+import { trackError } from '../utils/errors';
 
 export class HandlerMiddleware {
   readonly #logger: Logger;
@@ -62,9 +45,7 @@ export class HandlerMiddleware {
     try {
       return await fn();
     } catch (error) {
-      if (shouldTrackError(error, this.#logger)) {
-        await this.#snapClient.emitTrackingError(error as Error);
-      }
+      await trackError(error);
 
       const { locale } = await this.#snapClient.getPreferences();
       const messages = await this.#translator.load(locale);

packages/bitcoin-wallet-snap/src/index.ts

@@ -1,5 +1,4 @@
 import { handleKeyringRequest } from '@metamask/keyring-snap-sdk/v2';
-import { Logger } from '@metamask/snap-networks-utils';
 import type {
   OnAssetsConversionHandler,
   OnAssetsLookupHandler,
@@ -37,9 +36,9 @@ import {
   ConfirmationUseCases,
   SendFlowUseCases,
 } from './use-cases';
+import logger from './utils/logger';
 
 // Infra layer
-const logger = new Logger({ level: Config.logLevel });
 const snapClient = new SnapClientAdapter(logger, Config.encrypt);
 const chainClient = new EsploraClientAdapter(Config.chain);
 const assetRatesClient = new PriceApiClientAdapter(Config.priceApi);

packages/bitcoin-wallet-snap/src/infra/SnapClientAdapter.test.ts

@@ -4,6 +4,7 @@ import { mock } from 'jest-mock-extended';
 
 import type { BitcoinAccount, Logger } from '../entities';
 import { TrackingSnapEvent } from '../entities';
+import logger from '../utils/logger';
 import { SnapClientAdapter } from './SnapClientAdapter';
 
 jest.mock('@metamask/bitcoindevkit', () => ({
@@ -16,6 +17,13 @@ jest.mock('@metamask/bitcoindevkit', () => ({
   },
 }));
 
+jest.mock('../utils/logger', () => ({
+  __esModule: true,
+  default: {
+    error: jest.fn(),
+  },
+}));
+
 const setupTest = () => {
   const mockLogger = mock<Logger>();
   const mockRequest = jest.fn();
@@ -27,10 +35,19 @@ const setupTest = () => {
     writable: true,
   });
 
-  return { snapClient, mockLogger, mockRequest };
+  return {
+    snapClient,
+    mockLogger,
+    mockRequest,
+    mockTrackingLogger: jest.mocked(logger),
+  };
 };
 
 describe('SnapClientAdapter', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
   describe('emitTrackingEvent', () => {
     it("doesn't throw and logs when event tracking fails", async () => {
       const { snapClient, mockLogger, mockRequest } = setupTest();
@@ -79,7 +96,7 @@ describe('SnapClientAdapter', () => {
 
   describe('emitTrackingError', () => {
     it('sends the tracking error payload to the snap client', async () => {
-      const { snapClient, mockRequest } = setupTest();
+      const { snapClient, mockRequest, mockTrackingLogger } = setupTest();
 
       const error = new Error('boom');
       mockRequest.mockResolvedValue(undefined);
@@ -90,19 +107,25 @@ describe('SnapClientAdapter', () => {
         method: 'snap_trackError',
         params: { error: getJsonError(error) },
       });
+      expect(mockTrackingLogger.error).not.toHaveBeenCalled();
     });
 
     it("doesn't break execution when error tracking fails", async () => {
-      const { snapClient, mockLogger, mockRequest } = setupTest();
+      const { snapClient, mockRequest, mockTrackingLogger } = setupTest();
 
       const error = new Error('boom');
       const trackingError = new Error('track failed');
       mockRequest.mockRejectedValue(trackingError);
 
       expect(await snapClient.emitTrackingError(error)).toBeUndefined();
-      expect(mockLogger.error).toHaveBeenCalledWith(
+
+      expect(mockRequest).toHaveBeenCalledWith({
+        method: 'snap_trackError',
+        params: { error: getJsonError(error) },
+      });
+      expect(mockTrackingLogger.error).toHaveBeenCalledWith(
+        { error: trackingError },
         'Failed to track error',
-        trackingError,
       );
     });
   });

packages/bitcoin-wallet-snap/src/infra/SnapClientAdapter.ts

@@ -12,7 +12,7 @@ import type {
   GetPreferencesResult,
   Json,
 } from '@metamask/snaps-sdk';
-import { DialogType, getJsonError } from '@metamask/snaps-sdk';
+import { DialogType } from '@metamask/snaps-sdk';
 
 import type { BitcoinAccount, Logger, SnapClient } from '../entities';
 import {
@@ -27,6 +27,7 @@ import {
   networkToScope,
 } from '../handlers';
 import { mapToTransaction } from '../handlers/mappings';
+import { trackError } from '../utils/errors';
 
 export class SnapClientAdapter implements SnapClient {
   readonly #encrypt: boolean;
@@ -286,14 +287,7 @@ export class SnapClientAdapter implements SnapClient {
   }
 
   async emitTrackingError(error: Error): Promise<void> {
-    try {
-      await snap.request({
-        method: 'snap_trackError',
-        params: { error: getJsonError(error) },
-      });
-    } catch (trackingError) {
-      this.#logger.error('Failed to track error', trackingError);
-    }
+    await trackError(error);
   }
 
   async startTrace(name: string): Promise<boolean> {

packages/bitcoin-wallet-snap/src/infra/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;
+}