Fix inconsistent error behavior (#1285)

* Revert "Clean up - reorg to remove inverted/deep imports, clean up dead code, alias (#109)" This reverts commit dfd757f. Revert "Revert "Clean up - reorg to remove inverted/deep imports, clean up dead code, alias (#109)"" This reverts commit 9ea39ca. * serializeError * stack * Revert "Clean up - reorg to remove inverted/deep imports, clean up dead code, alias (#109)" This reverts commit dfd757f. # Conflicts: # apps/testapp/src/components/Layout.tsx # apps/testapp/src/context/CBWSDKReactContextProvider.tsx # packages/wallet-sdk/jest.config.ts # packages/wallet-sdk/src/CoinbaseWalletSDK.test.ts # packages/wallet-sdk/src/CoinbaseWalletSDK.ts # packages/wallet-sdk/src/WalletLinkSigner.test.ts # packages/wallet-sdk/src/WalletLinkSigner.ts # packages/wallet-sdk/src/core/ScopedLocalStorage.test.ts # packages/wallet-sdk/src/core/ScopedLocalStorage.ts # packages/wallet-sdk/src/core/constants.ts # packages/wallet-sdk/src/core/error/utils.test.ts # packages/wallet-sdk/src/core/error/utils.ts # packages/wallet-sdk/src/core/provider/interface.ts # packages/wallet-sdk/src/core/storage/ScopedLocalStorage.test.ts # packages/wallet-sdk/src/core/storage/ScopedLocalStorage.ts # packages/wallet-sdk/src/core/type/ProviderInterface.ts # packages/wallet-sdk/src/index.ts # packages/wallet-sdk/src/mocks/relay.ts # packages/wallet-sdk/src/provider/CoinbaseWalletProvider.test.ts # packages/wallet-sdk/src/provider/CoinbaseWalletProvider.ts # packages/wallet-sdk/src/provider/DiagnosticLogger.test.ts # packages/wallet-sdk/src/provider/DiagnosticLogger.ts # packages/wallet-sdk/src/provider/FilterPolyfill.test.ts # packages/wallet-sdk/src/provider/FilterPolyfill.ts # packages/wallet-sdk/src/provider/ProviderInterface.ts # packages/wallet-sdk/src/provider/handler/FilterRequestHandler.ts # packages/wallet-sdk/src/provider/handler/JSONRPCRequestHandler.ts # packages/wallet-sdk/src/provider/handler/RequestHandler.ts # packages/wallet-sdk/src/provider/handler/SignRequestHandler/SignRequestHandler.ts # packages/wallet-sdk/src/provider/handler/SignRequestHandler/UpdateListener.ts # packages/wallet-sdk/src/provider/handler/StateRequestHandler.ts # packages/wallet-sdk/src/provider/handler/SubscriptionManager.ts # packages/wallet-sdk/src/provider/helpers/eip1193Utils.test.ts # packages/wallet-sdk/src/provider/helpers/eip1193Utils.ts # packages/wallet-sdk/src/relay/RelayAbstract.ts # packages/wallet-sdk/src/relay/RelayEventManager.ts # packages/wallet-sdk/src/relay/RelayUI.ts # packages/wallet-sdk/src/relay/walletlink/WLRelayAdapter.test.ts # packages/wallet-sdk/src/relay/walletlink/WLRelayAdapter.ts # packages/wallet-sdk/src/relay/walletlink/WalletLinkRelay.test.ts # packages/wallet-sdk/src/relay/walletlink/WalletLinkRelay.ts # packages/wallet-sdk/src/relay/walletlink/connection/WalletLinkCipher.test.ts # packages/wallet-sdk/src/relay/walletlink/connection/WalletLinkCipher.ts # packages/wallet-sdk/src/relay/walletlink/connection/WalletLinkConnection.test.ts # packages/wallet-sdk/src/relay/walletlink/connection/WalletLinkConnection.ts # packages/wallet-sdk/src/relay/walletlink/constants.ts # packages/wallet-sdk/src/relay/walletlink/type/WalletLinkSession.ts # packages/wallet-sdk/src/relay/walletlink/type/Web3Request.ts # packages/wallet-sdk/src/relay/walletlink/type/Web3Response.ts # packages/wallet-sdk/src/sign/UpdateListenerInterface.ts # packages/wallet-sdk/src/sign/scw/message/SCWCipher.test.ts # packages/wallet-sdk/src/sign/scw/message/SCWCipher.ts # packages/wallet-sdk/src/sign/walletlink/relay/WLRelayAdapter.test.ts # packages/wallet-sdk/src/sign/walletlink/relay/WLRelayAdapter.ts # packages/wallet-sdk/src/signer/SignerInterface.ts # packages/wallet-sdk/src/signer/interface.ts # packages/wallet-sdk/src/signer/scw/SCWSigner.ts # packages/wallet-sdk/src/signer/scw/SCWStateManager.ts # packages/wallet-sdk/src/signer/scw/protocol/key/Cipher.test.ts # packages/wallet-sdk/src/signer/scw/protocol/key/Cipher.ts # packages/wallet-sdk/src/signer/scw/protocol/key/SCWKeyManager.test.ts # packages/wallet-sdk/src/signer/scw/protocol/key/SCWKeyManager.ts # packages/wallet-sdk/src/signer/scw/protocol/type/Action.ts # packages/wallet-sdk/src/signer/scw/protocol/type/ActionResult.ts # packages/wallet-sdk/src/signer/scw/protocol/type/Request.ts # packages/wallet-sdk/src/signer/scw/protocol/type/Response.ts # packages/wallet-sdk/src/signer/util.test.ts # packages/wallet-sdk/src/signer/util.ts # packages/wallet-sdk/src/signer/walletlink/WLSigner.ts # packages/wallet-sdk/src/transport/Communicator.test.ts # packages/wallet-sdk/src/transport/Communicator.ts # packages/wallet-sdk/src/transport/ConfigMessage.ts # packages/wallet-sdk/src/transport/CrossDomainCommunicator.ts # packages/wallet-sdk/src/transport/PopUpCommunicator.ts # packages/wallet-sdk/src/transport/util.ts # packages/wallet-sdk/src/util/ScopedLocalStorage.test.ts # packages/wallet-sdk/src/util/ScopedLocalStorage.ts # packages/wallet-sdk/src/util/cipher.test.ts # packages/wallet-sdk/src/util/cipher.ts # packages/wallet-sdk/src/version.ts # packages/wallet-sdk/tsconfig.build.json # packages/wallet-sdk/tsconfig.json # yarn.lock * asdf * revert * asdf * sadf * fix checkErrorForInvalidRequestArgs * method * duh * ugjh
coinbase · May 9, 2024 · 765f36c · 765f36c
1 parent 75fb749
commit 765f36c
Show file tree

Hide file tree

Showing 10 changed files with 429 additions and 45 deletions.
diff --git a/packages/wallet-sdk/src/CoinbaseWalletProvider.test.ts b/packages/wallet-sdk/src/CoinbaseWalletProvider.test.ts
@@ -34,7 +34,10 @@ describe('CoinbaseWalletProvider', () => {
       const provider = createProvider();
       // eslint-disable-next-line @typescript-eslint/ban-ts-comment
       // @ts-expect-error // testing invalid request args
-      await expect(provider.request({})).rejects.toThrow();
+      await expect(provider.request({})).rejects.toMatchObject({
+        code: -32602,
+        message: "'args.method' must be a non-empty string.",
+      });
     });
   });
 });

diff --git a/packages/wallet-sdk/src/CoinbaseWalletProvider.ts b/packages/wallet-sdk/src/CoinbaseWalletProvider.ts
@@ -1,6 +1,7 @@
 import EventEmitter from 'eventemitter3';
 
 import { standardErrorCodes, standardErrors } from './core/error';
+import { serializeError } from './core/error/serialize';
 import {
   AppMetadata,
   ConstructorOptions,
@@ -52,11 +53,15 @@ export class CoinbaseWalletProvider extends EventEmitter implements ProviderInte
   }
 
   public async request<T>(args: RequestArguments): Promise<T> {
-    const invalidArgsError = checkErrorForInvalidRequestArgs(args);
-    if (invalidArgsError) throw invalidArgsError;
-    // unrecognized methods are treated as fetch requests
-    const category = determineMethodCategory(args.method) ?? 'fetch';
-    return this.handlers[category](args) as T;
+    try {
+      const invalidArgsError = checkErrorForInvalidRequestArgs(args);
+      if (invalidArgsError) throw invalidArgsError;
+      // unrecognized methods are treated as fetch requests
+      const category = determineMethodCategory(args.method) ?? 'fetch';
+      return this.handlers[category](args) as T;
+    } catch (error) {
+      return Promise.reject(serializeError(error, args.method));
+    }
   }
 
   protected readonly handlers = {

diff --git a/packages/wallet-sdk/src/core/error/errors.ts b/packages/wallet-sdk/src/core/error/errors.ts
@@ -1,4 +1,5 @@
-import { errorValues, standardErrorCodes } from './constants';
+import { standardErrorCodes } from './constants';
+import { getMessageFromCode } from './utils';
 
 export const standardErrors = {
   rpc: {
@@ -88,28 +89,6 @@ export const standardErrors = {
 
 // Internal
 
-function isJsonRpcServerError(code: number): boolean {
-  return code >= -32099 && code <= -32000;
-}
-
-function hasKey(obj: Record<string, unknown>, key: string) {
-  return Object.prototype.hasOwnProperty.call(obj, key);
-}
-
-function getMessageFromCode(code: number | undefined): string {
-  if (code && Number.isInteger(code)) {
-    const codeString = code.toString();
-
-    if (hasKey(errorValues, codeString)) {
-      return errorValues[codeString as keyof typeof errorValues].message;
-    }
-    if (isJsonRpcServerError(code)) {
-      return 'Unspecified server error.';
-    }
-  }
-  return 'Unspecified error message.';
-}
-
 function getEthJsonRpcError<T>(code: number, arg?: EthErrorsArg<T>): EthereumRpcError<T> {
   const [message, data] = parseOpts(arg);
   return new EthereumRpcError(code, message || getMessageFromCode(code), data);

diff --git a/packages/wallet-sdk/src/core/error/index.ts b/packages/wallet-sdk/src/core/error/index.ts
@@ -1,3 +1,3 @@
 export { standardErrorCodes } from './constants';
 export { standardErrors } from './errors';
-export type { SerializedEthereumRpcError } from './type';
+export type { SerializedEthereumRpcError } from './utils';
diff --git a/packages/wallet-sdk/src/core/error/serialize.test.ts b/packages/wallet-sdk/src/core/error/serialize.test.ts
@@ -0,0 +1,122 @@
+import { Web3Response } from '../../sign/walletlink/relay/type/Web3Response';
+import { standardErrorCodes } from './constants';
+import { standardErrors } from './errors';
+import { serializeError } from './serialize';
+
+describe('serializeError', () => {
+  test('with ErrorResponse object', () => {
+    const errorResponse: Web3Response = {
+      method: 'generic',
+      errorMessage: 'test ErrorResponse object',
+      errorCode: standardErrorCodes.provider.unsupportedMethod,
+    };
+
+    const serialized = serializeError(errorResponse, '');
+    expect(serialized.code).toEqual(standardErrorCodes.provider.unsupportedMethod);
+    expect(serialized.message).toEqual('test ErrorResponse object');
+    expect(serialized.docUrl).toMatch(/.*version=\d+\.\d+\.\d+.*/);
+    expect(serialized.docUrl).toContain(`code=${standardErrorCodes.provider.unsupportedMethod}`);
+  });
+
+  test('with standardError', () => {
+    const error = standardErrors.provider.userRejectedRequest({});
+
+    const serialized = serializeError(error, 'test_request');
+    expect(serialized.code).toEqual(standardErrorCodes.provider.userRejectedRequest);
+    expect(serialized.message).toEqual(error.message);
+    expect(serialized.stack).toEqual(expect.stringContaining('User rejected'));
+    expect(serialized.docUrl).toMatch(/.*version=\d+\.\d+\.\d+.*/);
+    expect(serialized.docUrl).toContain(`code=${standardErrorCodes.provider.userRejectedRequest}`);
+  });
+
+  test('with unsupportedChain', () => {
+    const error = standardErrors.provider.unsupportedChain();
+
+    const serialized = serializeError(error);
+    expect(serialized.code).toEqual(standardErrorCodes.provider.unsupportedChain);
+    expect(serialized.message).toEqual(error.message);
+    expect(serialized.stack).toEqual(expect.stringContaining('Unrecognized chain ID'));
+    expect(serialized.docUrl).toMatch(/.*version=\d+\.\d+\.\d+.*/);
+    expect(serialized.docUrl).toContain(`code=${standardErrorCodes.provider.unsupportedChain}`);
+  });
+
+  test('with Error object', () => {
+    const error = new Error('test Error object');
+
+    const serialized = serializeError(error, 'test_request');
+    expect(serialized.code).toEqual(standardErrorCodes.rpc.internal);
+    expect(serialized.message).toEqual('test Error object');
+    expect(serialized.stack).toEqual(expect.stringContaining('test Error object'));
+    expect(serialized.docUrl).toMatch(/.*version=\d+\.\d+\.\d+.*/);
+    expect(serialized.docUrl).toContain(`code=${standardErrorCodes.rpc.internal}`);
+  });
+
+  test('with string', () => {
+    const error = 'test error with just string';
+
+    const serialized = serializeError(error, 'test_request');
+    expect(serialized.code).toEqual(standardErrorCodes.rpc.internal);
+    expect(serialized.message).toEqual('test error with just string');
+    expect(serialized.docUrl).toMatch(/.*version=\d+\.\d+\.\d+.*/);
+    expect(serialized.docUrl).toContain(`code=${standardErrorCodes.rpc.internal}`);
+  });
+
+  test('with unknown type', () => {
+    const error = { unknown: 'error' };
+    const serialized = serializeError(error, 'test_request');
+    expect(serialized.code).toEqual(standardErrorCodes.rpc.internal);
+    expect(serialized.message).toEqual('Unspecified error message.');
+    expect(serialized.docUrl).toMatch(/.*version=\d+\.\d+\.\d+.*/);
+    expect(serialized.docUrl).toContain(`code=${standardErrorCodes.rpc.internal}`);
+  });
+});
+
+describe('serializeError to retrieve the request method', () => {
+  test('with JSONRPCRequest object', () => {
+    const jsonRpcRequest = {
+      jsonrpc: '2.0',
+      id: 1,
+      method: 'requestEthereumAccounts',
+      params: [],
+    };
+
+    const error = standardErrors.provider.userRejectedRequest({});
+
+    const serialized = serializeError(error, jsonRpcRequest);
+    expect(serialized.code).toEqual(standardErrorCodes.provider.userRejectedRequest);
+    expect(serialized.docUrl).toContain('method=requestEthereumAccounts');
+  });
+
+  test('with string', () => {
+    const method = 'test_method';
+
+    const error = standardErrors.provider.userRejectedRequest({});
+
+    const serialized = serializeError(error, method);
+    expect(serialized.code).toEqual(standardErrorCodes.provider.userRejectedRequest);
+    expect(serialized.docUrl).toContain(`method=${method}`);
+  });
+
+  test('with JSONRPCRequest array', () => {
+    const jsonRpcRequests = [
+      {
+        jsonrpc: '2.0',
+        id: 1,
+        method: 'requestEthereumAccounts',
+        params: [],
+      },
+      {
+        jsonrpc: '2.0',
+        id: 1,
+        method: 'signEthereumMessage',
+        params: [],
+      },
+    ];
+
+    const error = standardErrors.provider.userRejectedRequest({});
+
+    const serialized = serializeError(error, jsonRpcRequests);
+    expect(serialized.code).toEqual(standardErrorCodes.provider.userRejectedRequest);
+    expect(serialized.docUrl).toContain('method=requestEthereumAccounts');
+  });
+});
diff --git a/packages/wallet-sdk/src/core/error/serialize.ts b/packages/wallet-sdk/src/core/error/serialize.ts
@@ -0,0 +1,85 @@
+// TODO: error should not depend on walletlink. revisit this.
+import { isErrorResponse, Web3Response } from '../../sign/walletlink/relay/type/Web3Response';
+import { LIB_VERSION } from '../../version';
+import { standardErrorCodes } from './constants';
+import { serialize, SerializedEthereumRpcError } from './utils';
+
+/**
+ * Serializes an error to a format that is compatible with the Ethereum JSON RPC error format.
+ * See https://docs.cloud.coinbase.com/wallet-sdk/docs/errors
+ * for more information.
+ */
+export function serializeError(
+  error: unknown,
+  requestOrMethod?: JSONRPCRequest | JSONRPCRequest[] | string
+): SerializedError {
+  const serialized = serialize(getErrorObject(error), {
+    shouldIncludeStack: true,
+  });
+
+  const docUrl = new URL('https://docs.cloud.coinbase.com/wallet-sdk/docs/errors');
+  docUrl.searchParams.set('version', LIB_VERSION);
+  docUrl.searchParams.set('code', serialized.code.toString());
+  const method = getMethod(serialized.data, requestOrMethod);
+  if (method) {
+    docUrl.searchParams.set('method', method);
+  }
+  docUrl.searchParams.set('message', serialized.message);
+
+  return {
+    ...serialized,
+    docUrl: docUrl.href,
+  };
+}
+
+/**
+ * Converts an error to a serializable object.
+ */
+function getErrorObject(error: string | Web3Response | unknown) {
+  if (typeof error === 'string') {
+    return {
+      message: error,
+      code: standardErrorCodes.rpc.internal,
+    };
+  } else if (isErrorResponse(error)) {
+    return {
+      ...error,
+      message: error.errorMessage,
+      code: error.errorCode,
+      data: { method: error.method },
+    };
+  }
+  return error;
+}
+
+/**
+ * Gets the method name from the serialized data or the request.
+ */
+function getMethod(
+  serializedData: unknown,
+  request?: JSONRPCRequest | JSONRPCRequest[] | string
+): string | undefined {
+  const methodInData = (serializedData as { method: string })?.method;
+  if (methodInData) {
+    return methodInData;
+  }
+
+  if (request === undefined) {
+    return undefined;
+  } else if (typeof request === 'string') {
+    return request;
+  } else if (!Array.isArray(request)) {
+    return request.method;
+  } else if (request.length > 0) {
+    return request[0].method;
+  }
+  return undefined;
+}
+
+export interface SerializedError extends SerializedEthereumRpcError {
+  docUrl: string;
+}
+
+interface JSONRPCRequest {
+  method: string;
+}
diff --git a/packages/wallet-sdk/src/core/error/type.ts b/packages/wallet-sdk/src/core/error/type.ts
diff --git a/packages/wallet-sdk/src/core/error/utils.test.ts b/packages/wallet-sdk/src/core/error/utils.test.ts
@@ -0,0 +1,50 @@
+import { isErrorResponse, Web3Response } from '../../sign/walletlink/relay/type/Web3Response';
+import { standardErrorCodes } from './constants';
+import { standardErrors } from './errors';
+import { getErrorCode, getMessageFromCode } from './utils';
+
+describe('errors', () => {
+  test('getErrorCode', () => {
+    expect(getErrorCode(4137)).toEqual(4137);
+
+    expect(getErrorCode({ code: 4137 })).toEqual(4137);
+    expect(getErrorCode({ errorCode: 4137 })).toEqual(4137);
+    expect(getErrorCode({ code: 4137, errorCode: 4137 })).toEqual(4137);
+
+    expect(getErrorCode({ code: '4137' })).toEqual(undefined);
+    expect(getErrorCode({ code: undefined })).toEqual(undefined);
+    expect(getErrorCode({ errorCode: '4137' })).toEqual(undefined);
+    expect(getErrorCode({ errorCode: undefined })).toEqual(undefined);
+
+    expect(getErrorCode({})).toEqual(undefined);
+    expect(getErrorCode('4137')).toEqual(undefined);
+    expect(getErrorCode(new Error('generic error'))).toEqual(undefined);
+
+    expect(getErrorCode(null)).toEqual(undefined);
+    expect(getErrorCode(undefined)).toEqual(undefined);
+
+    const errorResponse: Web3Response = {
+      method: 'generic',
+      errorMessage: 'test error message',
+      errorCode: 4137,
+    };
+    expect(isErrorResponse(errorResponse)).toEqual(true);
+    expect(getErrorCode(errorResponse)).toEqual(4137);
+  });
+
+  test('standardErrorMessage', () => {
+    // default error message
+    expect(getMessageFromCode(standardErrorCodes.provider.userRejectedRequest)).toEqual(
+      expect.stringContaining('rejected')
+    );
+
+    // non-standard error code
+    expect(getMessageFromCode(0)).toEqual('Unspecified error message.');
+  });
+
+  test('unsupportedChain error', () => {
+    const errorWithoutChainID = standardErrors.provider.unsupportedChain();
+    expect(errorWithoutChainID.code).toEqual(standardErrorCodes.provider.unsupportedChain);
+    expect(errorWithoutChainID.message).toEqual(expect.stringContaining('Unrecognized chain ID'));
+  });
+});