fix: handle errors on the edge

Author: aganglada

packages/snap/snap.manifest.json

@@ -7,7 +7,7 @@
     "url": "https://github.com/MetaMask/snap-solana-wallet.git"
   },
   "source": {
-    "shasum": "5dayxdLk26hWbBj7hAl3wzDaPIPWYYsnsvQcIKuo8KA=",
+    "shasum": "SNeUb5hAzMEExl6/sODXFY5l7FrC7cEFb2j2lL8PhGM=",
     "location": {
       "npm": {
         "filePath": "dist/bundle.js",

packages/snap/src/core/utils/errors.ts

@@ -1,90 +1,21 @@
-import {
-  MethodNotFoundError,
-  UserRejectedRequestError,
-  MethodNotSupportedError,
-  ParseError,
-  ResourceNotFoundError,
-  ResourceUnavailableError,
-  TransactionRejected,
-  ChainDisconnectedError,
-  DisconnectedError,
-  UnauthorizedError,
-  UnsupportedMethodError,
-  InternalError,
-  InvalidInputError,
-  InvalidParamsError,
-  InvalidRequestError,
-  LimitExceededError,
-  SnapError,
-} from '@metamask/snaps-sdk';
+import { SnapError } from '@metamask/snaps-sdk';
 
-export class CustomError extends Error {
-  name!: string;
+import logger from './logger';
 
-  constructor(message?: string) {
-    super(message);
+export const handle = async <ResponseT>(
+  scope: string,
+  fn: () => Promise<ResponseT>,
+): Promise<ResponseT> => {
+  try {
+    return await fn();
+  } catch (errorInstance: any) {
+    const error = new SnapError(errorInstance as Error);
 
-    // set error name as constructor name, make it not enumerable to keep native Error behavior
-    // see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/new.target#new.target_in_constructors
-    // see https://github.com/adriengibrat/ts-custom-error/issues/30
-    Object.defineProperty(this, 'name', {
-      value: new.target.name,
-      enumerable: false,
-      configurable: true,
-    });
+    logger.error(
+      { error },
+      `[${scope}] Error occurred: ${JSON.stringify(error.toJSON(), null, 2)}`,
+    );
 
-    // fix the extended error prototype chain
-    // because typescript __extends implementation can't
-    // see https://github.com/Microsoft/TypeScript-wiki/blob/master/Breaking-Changes.md#extending-built-ins-like-error-array-and-map-may-no-longer-work
-    Object.setPrototypeOf(this, new.target.prototype);
-    // remove constructor from stack trace
-    Error.captureStackTrace(this, this.constructor);
+    throw error;
   }
-}
-
-/**
- * Compacts an error to a specific error instance.
- *
- * @param error - The error instance to be compacted.
- * @param ErrCtor - The error constructor for the desired error instance.
- * @returns The compacted error instance.
- */
-export function compactError<ErrorInstance extends Error>(
-  error: ErrorInstance,
-  ErrCtor: new (message?: string) => ErrorInstance,
-): ErrorInstance {
-  if (error instanceof ErrCtor) {
-    return error;
-  }
-  return new ErrCtor(error.message);
-}
-
-/**
- * Determines if the given error is a Snap RPC error.
- *
- * @param error - The error instance to be checked.
- * @returns A boolean indicating whether the error is a Snap RPC error.
- */
-export function isSnapRpcError(error: Error): boolean {
-  const errors = [
-    SnapError,
-    MethodNotFoundError,
-    UserRejectedRequestError,
-    MethodNotSupportedError,
-    MethodNotFoundError,
-    ParseError,
-    ResourceNotFoundError,
-    ResourceUnavailableError,
-    TransactionRejected,
-    ChainDisconnectedError,
-    DisconnectedError,
-    UnauthorizedError,
-    UnsupportedMethodError,
-    InternalError,
-    InvalidInputError,
-    InvalidParamsError,
-    InvalidRequestError,
-    LimitExceededError,
-  ];
-  return errors.some((errType) => error instanceof errType);
-}
+};

packages/snap/src/index.ts

@@ -14,7 +14,6 @@ import type {
 } from '@metamask/snaps-sdk';
 import {
   MethodNotFoundError,
-  SnapError,
   type OnRpcRequestHandler,
 } from '@metamask/snaps-sdk';
 import { assert, enums } from '@metamask/superstruct';
@@ -29,7 +28,7 @@ import { CronjobMethod } from './core/handlers/onCronjob/cronjobs/CronjobMethod'
 import { onProtocolRequest as onProtocolRequestHandler } from './core/handlers/onProtocolRequest/onProtocolRequest';
 import { handlers as onRpcRequestHandlers } from './core/handlers/onRpcRequest';
 import { RpcRequestMethod } from './core/handlers/onRpcRequest/types';
-import { isSnapRpcError } from './core/utils/errors';
+import { handle } from './core/utils/errors';
 import { getClientStatus } from './core/utils/interface';
 import logger from './core/utils/logger';
 import { validateOrigin } from './core/validation/validators';
@@ -64,35 +63,27 @@ export const onRpcRequest: OnRpcRequestHandler = async ({
   origin,
   request,
 }) => {
-  try {
-    logger.log('[🔄 onRpcRequest]', request.method, request);
+  logger.log('[🔄 onRpcRequest]', request.method, request);
 
-    const { method } = request;
+  const { method } = request;
 
-    validateOrigin(origin, method);
+  validateOrigin(origin, method);
 
-    const handler = onRpcRequestHandlers[method as RpcRequestMethod];
+  const handler = onRpcRequestHandlers[method as RpcRequestMethod];
 
-    if (!handler) {
-      throw new MethodNotFoundError(
-        `RpcRequest method ${method} not found. Available methods: ${Object.values(
-          RpcRequestMethod,
-        ).toString()}`,
-      ) as unknown as Error;
-    }
+  if (!handler) {
+    throw new MethodNotFoundError(
+      `RpcRequest method ${method} not found. Available methods: ${Object.values(
+        RpcRequestMethod,
+      ).toString()}`,
+    ) as unknown as Error;
+  }
 
-    return handler({ origin, request });
-  } catch (error: any) {
-    let snapError = error;
+  const result = await handle('onRpcRequest', async () =>
+    handler({ origin, request }),
+  );
 
-    if (!isSnapRpcError(error)) {
-      snapError = new SnapError(error);
-    }
-    logger.error(
-      `onRpcRequest error: ${JSON.stringify(snapError.toJSON(), null, 2)}`,
-    );
-    throw snapError;
-  }
+  return result ?? null;
 };
 
 /**
@@ -109,42 +100,25 @@ export const onKeyringRequest: OnKeyringRequestHandler = async ({
   origin,
   request,
 }): Promise<Json> => {
-  try {
-    logger.log('[🔑 onKeyringRequest]', request.method, request);
-
-    validateOrigin(origin, request.method);
-
-    // This is a temporal fix to prevent the swap/bridge functionality breaking
-    // TODO: Remove this once changes in bridge-status-controller are in place
-    if (
-      request.method === KeyringRpcMethod.SubmitRequest &&
-      request.params &&
-      !('origin' in request.params)
-    ) {
-      (request.params as Record<string, Json>).origin = 'https://metamask.io';
-    }
-
-    return (await handleKeyringRequest(
-      keyring,
-      request,
-    )) as unknown as Promise<Json>;
-  } catch (error: any) {
-    let snapError = error;
-
-    if (!isSnapRpcError(error)) {
-      snapError = new SnapError(error);
-    }
+  logger.log('[🔑 onKeyringRequest]', request.method, request);
+
+  validateOrigin(origin, request.method);
+
+  // This is a temporal fix to prevent the swap/bridge functionality breaking
+  // TODO: Remove this once changes in bridge-status-controller are in place
+  if (
+    request.method === KeyringRpcMethod.SubmitRequest &&
+    request.params &&
+    !('origin' in request.params)
+  ) {
+    (request.params as Record<string, Json>).origin = 'https://metamask.io';
+  }
 
-    logger.error(
-      `onKeyringRequest - ${request.method} - Error: ${JSON.stringify(
-        snapError.toJSON(),
-        null,
-        2,
-      )}`,
-    );
+  const result = await handle('onKeyringRequest', async () =>
+    handleKeyringRequest(keyring, request),
+  );
 
-    throw snapError;
-  }
+  return result ?? null;
 };
 
 /**
@@ -183,7 +157,9 @@ export const onUserInput: OnUserInputHandler = async ({
     return;
   }
 
-  await handler({ id, event, context, snapContext });
+  await handle('onUserInput', async () =>
+    handler({ id, event, context, snapContext }),
+  );
 };
 
 /**
@@ -207,59 +183,86 @@ export const onCronjob: OnCronjobHandler = async ({ request }) => {
     ]),
   );
 
-  /**
-   * Don't run cronjobs if client is locked or inactive
-   * - We dont want to call cronjobs if the client is locked
-   * - We Dont want to call cronjobs if the client is inactive (except if we havent run a cronjob in the last 15 minutes)
-   */
-  const { locked, active } =
-    (await getClientStatus()) as GetClientStatusResult & {
-      active: boolean | undefined; // FIXME: Remove this once the snap SDK is updated
-    };
+  const result = await handle('onCronjob', async () => {
+    /**
+     * Don't run cronjobs if client is locked or inactive
+     * - We dont want to call cronjobs if the client is locked
+     * - We Dont want to call cronjobs if the client is inactive (except if we havent run a cronjob in the last 15 minutes)
+     */
+    const { locked, active } =
+      (await getClientStatus()) as GetClientStatusResult & {
+        active: boolean | undefined; // FIXME: Remove this once the snap SDK is updated
+      };
 
-  logger.log('[🔑 onCronjob] Client status', { locked, active });
-
-  if (locked) {
-    return Promise.resolve();
-  }
+    logger.log('[🔑 onCronjob] Client status', { locked, active });
 
-  // explicit check for non-undefined active
-  // to make sure the cronjob is executed if `active` is undefined
-  // eslint-disable-next-line @typescript-eslint/no-unnecessary-boolean-literal-compare
-  if (active === false) {
-    const lastCronjobRun = await state.getKey<number>('lastCronjobRun');
-    const THIRTY_MINUTES = 30 * 60 * 1000; // 30 minutes in milliseconds
-
-    logger.log('[🔑 onCronjob] Last cronjob run', { lastCronjobRun });
-
-    // Only skip if we've run a cronjob in the last 30 minutes
-    if (lastCronjobRun && Date.now() - lastCronjobRun < THIRTY_MINUTES) {
-      logger.log(
-        '[🔑 onCronjob] Skipping cronjob because it has been run in the last 30 minutes',
-      );
+    if (locked) {
       return Promise.resolve();
     }
-    // if `lastCronjobRun` is undefined, we can run the cronjob
-  }
 
-  await state.setKey('lastCronjobRun', Date.now());
+    // explicit check for non-undefined active
+    // to make sure the cronjob is executed if `active` is undefined
+    // eslint-disable-next-line @typescript-eslint/no-unnecessary-boolean-literal-compare
+    if (active === false) {
+      const lastCronjobRun = await state.getKey<number>('lastCronjobRun');
+      const THIRTY_MINUTES = 30 * 60 * 1000; // 30 minutes in milliseconds
+
+      logger.log('[🔑 onCronjob] Last cronjob run', { lastCronjobRun });
+
+      // Only skip if we've run a cronjob in the last 30 minutes
+      if (lastCronjobRun && Date.now() - lastCronjobRun < THIRTY_MINUTES) {
+        logger.log(
+          '[🔑 onCronjob] Skipping cronjob because it has been run in the last 30 minutes',
+        );
+        return Promise.resolve();
+      }
+      // if `lastCronjobRun` is undefined, we can run the cronjob
+    }
+
+    await state.setKey('lastCronjobRun', Date.now());
+
+    logger.log('[🔑 onCronjob] Running cronjob', { method });
 
-  logger.log('[🔑 onCronjob] Running cronjob', { method });
+    const handler = onCronjobHandlers[method];
+    return handler({ request });
+  });
 
-  const handler = onCronjobHandlers[method];
-  return handler({ request });
+  return result ?? null;
 };
 
-export const onAssetsLookup: OnAssetsLookupHandler = onAssetsLookupHandler;
+export const onAssetsLookup: OnAssetsLookupHandler = async (params) => {
+  const result = await handle('onAssetsLookup', async () =>
+    onAssetsLookupHandler(params),
+  );
+  return result ?? null;
+};
 
-export const onAssetsConversion: OnAssetsConversionHandler =
-  onAssetsConversionHandler;
+export const onAssetsConversion: OnAssetsConversionHandler = async (params) => {
+  const result = await handle('onAssetsConversion', async () =>
+    onAssetsConversionHandler(params),
+  );
+  return result ?? null;
+};
 
-export const onProtocolRequest: OnProtocolRequestHandler =
-  onProtocolRequestHandler;
+export const onProtocolRequest: OnProtocolRequestHandler = async (params) => {
+  const result = await handle('onProtocolRequest', async () =>
+    onProtocolRequestHandler(params),
+  );
+  return result ?? null;
+};
 
-export const onAssetHistoricalPrice: OnAssetHistoricalPriceHandler =
-  onAssetHistoricalPriceHandler;
+export const onAssetHistoricalPrice: OnAssetHistoricalPriceHandler = async (
+  params,
+) => {
+  const result = await handle('onAssetHistoricalPrice', async () =>
+    onAssetHistoricalPriceHandler(params),
+  );
+  return result ?? null;
+};
 
-export const onClientRequest: OnClientRequestHandler = async ({ request }) =>
-  clientRequestHandler.handle(request);
+export const onClientRequest: OnClientRequestHandler = async ({ request }) => {
+  const result = await handle('onClientRequest', async () =>
+    clientRequestHandler.handle(request),
+  );
+  return result ?? null;
+};