fix: serializeError handling and allow passthrough for unknown errors…

… [LIVE-12524] (#349) Add tests for error handling
LedgerHQ · May 10, 2024 · 11a705f · 11a705f
1 parent f1afe05
commit 11a705f
Show file tree

Hide file tree

Showing 7 changed files with 158 additions and 12 deletions.
diff --git a/.changeset/short-kings-tell.md b/.changeset/short-kings-tell.md
@@ -0,0 +1,5 @@
+---
+"@ledgerhq/wallet-api-core": patch
+---
+
+fix: serializeError handling and allow passthrough for unknown errors
diff --git a/packages/core/src/JSONRPC/RpcNode.ts b/packages/core/src/JSONRPC/RpcNode.ts
@@ -140,11 +140,18 @@ export abstract class RpcNode<TSHandlers, TCHandlers> {
       if (error instanceof RpcError) {
         throw error;
       }
+
+      let serializedError = serializeError(
+        error as Parameters<typeof serializeError>[0],
+      );
+      serializedError =
+        typeof serializedError === "string" || !serializedError
+          ? { message: serializedError }
+          : serializedError;
       throw new RpcError({
         code: RpcErrorCode.SERVER_ERROR,
         message: "unexpected server error",
-        // @ts-expect-error: Bad typings on serialize error !!
-        data: createUnknownError(serializeError(error)),
+        data: createUnknownError(serializedError),
       });
     }
   }

diff --git a/packages/core/src/errors/creators.ts b/packages/core/src/errors/creators.ts
@@ -5,6 +5,7 @@ import type {
   PermissionDenied,
   UnauthorizedStore,
   UnknownError,
+  UnknownErrorData,
 } from "./types";
 
 export function createNotImplementedByWallet(
@@ -49,7 +50,7 @@ export function createAccountNotFound(accountId: string): AccountNotFound {
   };
 }
 
-export function createUnknownError(error: UnknownError["data"]): UnknownError {
+export function createUnknownError(error: UnknownErrorData): UnknownError {
   return {
     code: "UNKNOWN_ERROR",
     message: "an unhandled error was thrown",

diff --git a/packages/core/src/errors/types.ts b/packages/core/src/errors/types.ts
@@ -78,22 +78,26 @@ export type PermissionDenied = z.infer<typeof schemaPermissionDenied>;
     UNKNOWN_ERROR
 */
 
+export const schemaUnknownErrorData = z.object({
+  name: z.string().optional(),
+  message: z.string().optional(),
+  stack: z.string().optional(),
+  cause: z.unknown().optional(),
+  code: z.string().optional(),
+});
+
+export type UnknownErrorData = z.infer<typeof schemaUnknownErrorData>;
+
 export const schemaUnknownError = z.object({
   code: z.literal(schemaServerErrorCode.enum.UNKNOWN_ERROR),
   message: z.string(),
-  data: z.object({
-    name: z.string().optional(),
-    message: z.string().optional(),
-    stack: z.string().optional(),
-    cause: z.unknown().optional(),
-    code: z.string().optional(),
-  }),
+  data: schemaUnknownErrorData.passthrough(),
 });
 
 export type UnknownError = z.infer<typeof schemaUnknownError>;
 
 /*
-    PERMISSION_DENIED
+    UNAUTHORIZED_STORE
 */
 
 export const schemaUnauthorizedStore = z.object({

diff --git a/packages/simulator/package.json b/packages/simulator/package.json
@@ -19,6 +19,7 @@
     "test": "jest"
   },
   "devDependencies": {
+    "@ledgerhq/errors": "^6.16.2",
     "@ledgerhq/jest-shared-config": "workspace:*",
     "@types/jest": "^29.5.12",
     "@types/node": "^20.11.19",

diff --git a/packages/simulator/tests/simulator.spec.ts b/packages/simulator/tests/simulator.spec.ts
@@ -1,3 +1,4 @@
+import { TransportStatusError } from "@ledgerhq/errors";
 import {
   WalletAPIClient,
   deserializeAccount,
@@ -451,4 +452,129 @@ describe("Simulator", () => {
       await expect(client.wallet.userId()).rejects.toThrow("permission");
     });
   });
+
+  describe("errors", () => {
+    it("should handle unknown errors", async () => {
+      // GIVEN
+      const profileWithErrors = {
+        ...profiles.STANDARD,
+        methods: {
+          ...profiles.STANDARD.methods,
+          "message.sign": () => {
+            throw new TransportStatusError(27905);
+          },
+        },
+      };
+      const transport = getSimulatorTransport(profileWithErrors);
+      const client = new WalletAPIClient(transport);
+
+      // THEN
+      await expect(
+        client.message.sign("account-btc-1", Buffer.from("")),
+      ).rejects.toThrow("Ledger device: UNKNOWN_ERROR (0x6d01)");
+    });
+
+    it("should handle simple string errors", async () => {
+      // GIVEN
+      const profileWithErrors = {
+        ...profiles.STANDARD,
+        methods: {
+          ...profiles.STANDARD.methods,
+          "message.sign": () => {
+            throw new Error("simple string");
+          },
+        },
+      };
+      const transport = getSimulatorTransport(profileWithErrors);
+      const client = new WalletAPIClient(transport);
+
+      // THEN
+      await expect(
+        client.message.sign("account-btc-1", Buffer.from("")),
+      ).rejects.toThrow("simple string");
+    });
+
+    it("should handle empty string errors", async () => {
+      // GIVEN
+      const profileWithErrors = {
+        ...profiles.STANDARD,
+        methods: {
+          ...profiles.STANDARD.methods,
+          "message.sign": () => {
+            throw new Error();
+          },
+        },
+      };
+      const transport = getSimulatorTransport(profileWithErrors);
+      const client = new WalletAPIClient(transport);
+
+      // THEN
+      await expect(
+        client.message.sign("account-btc-1", Buffer.from("")),
+      ).rejects.toThrow("");
+    });
+
+    it("should handle simple string", async () => {
+      // GIVEN
+      const profileWithErrors = {
+        ...profiles.STANDARD,
+        methods: {
+          ...profiles.STANDARD.methods,
+          "message.sign": () => {
+            throw "simple string";
+          },
+        },
+      };
+      const transport = getSimulatorTransport(profileWithErrors);
+      const client = new WalletAPIClient(transport);
+
+      // THEN
+      await expect(
+        client.message.sign("account-btc-1", Buffer.from("")),
+      ).rejects.toThrow("simple string");
+    });
+
+    it("should handle empty string", async () => {
+      // GIVEN
+      const profileWithErrors = {
+        ...profiles.STANDARD,
+        methods: {
+          ...profiles.STANDARD.methods,
+          "message.sign": () => {
+            throw "";
+          },
+        },
+      };
+      const transport = getSimulatorTransport(profileWithErrors);
+      const client = new WalletAPIClient(transport);
+
+      // THEN
+      await expect(
+        client.message.sign("account-btc-1", Buffer.from("")),
+      ).rejects.toThrow("");
+    });
+
+    it("should handle undefined", async () => {
+      // GIVEN
+      const profileWithErrors = {
+        ...profiles.STANDARD,
+        methods: {
+          ...profiles.STANDARD.methods,
+          "message.sign": () => {
+            throw undefined;
+          },
+        },
+      };
+      const transport = getSimulatorTransport(profileWithErrors);
+      const client = new WalletAPIClient(transport);
+
+      // THEN
+      try {
+        await client.message.sign("account-btc-1", Buffer.from(""));
+        expect(false).toBeTruthy();
+      } catch (error) {
+        expect(error).toBeUndefined();
+      }
+    });
+  });
 });
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml