From 11a705fa42ecf0f17234bbf536a7e7b8254aeed9 Mon Sep 17 00:00:00 2001 From: Quentin Jaccarino Date: Fri, 10 May 2024 13:46:52 +0200 Subject: [PATCH] fix: serializeError handling and allow passthrough for unknown errors [LIVE-12524] (#349) Add tests for error handling --- .changeset/short-kings-tell.md | 5 + packages/core/src/JSONRPC/RpcNode.ts | 11 +- packages/core/src/errors/creators.ts | 3 +- packages/core/src/errors/types.ts | 20 ++-- packages/simulator/package.json | 1 + packages/simulator/tests/simulator.spec.ts | 126 +++++++++++++++++++++ pnpm-lock.yaml | 4 +- 7 files changed, 158 insertions(+), 12 deletions(-) create mode 100644 .changeset/short-kings-tell.md diff --git a/.changeset/short-kings-tell.md b/.changeset/short-kings-tell.md new file mode 100644 index 00000000..f54aeb0d --- /dev/null +++ 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 index c1c32bc5..2c6203ab 100644 --- a/packages/core/src/JSONRPC/RpcNode.ts +++ b/packages/core/src/JSONRPC/RpcNode.ts @@ -140,11 +140,18 @@ export abstract class RpcNode { if (error instanceof RpcError) { throw error; } + + let serializedError = serializeError( + error as Parameters[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 index f49e0826..cba92f2c 100644 --- 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 index 9351a8ac..c61e63a0 100644 --- a/packages/core/src/errors/types.ts +++ b/packages/core/src/errors/types.ts @@ -78,22 +78,26 @@ export type PermissionDenied = z.infer; 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; + 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; /* - PERMISSION_DENIED + UNAUTHORIZED_STORE */ export const schemaUnauthorizedStore = z.object({ diff --git a/packages/simulator/package.json b/packages/simulator/package.json index b573c474..3ce342bb 100644 --- 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 index 193ed39f..c734b505 100644 --- 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 index 07ea2ec9..3174ec73 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -568,6 +568,9 @@ importers: specifier: ^8.16.0 version: 8.16.0 devDependencies: + '@ledgerhq/errors': + specifier: ^6.16.2 + version: 6.16.2 '@ledgerhq/jest-shared-config': specifier: workspace:* version: link:../jest-shared-config @@ -1859,7 +1862,6 @@ packages: /@ledgerhq/errors@6.16.2: resolution: {integrity: sha512-jFpohaSW+p1Obp3NDT9QSByEtT3gtBZIjVNu8m25gnrH5zdtfPVlPwH6UiuS50s+2dHQyehV8hF+IfreKDWAZA==} - dev: false /@ledgerhq/hw-transport@6.30.4: resolution: {integrity: sha512-VBcVd7UG8HDrjWMoZI5rqBDz+PBxLHTIPZOGY/fdMoEUwaBbss0Z3MxuJanMyerlfaLqnBSVuL0blz7rOyagkw==}