From 5913874ee7bfd432e77fc8a014430f3d9ccfffc4 Mon Sep 17 00:00:00 2001 From: David de Kloet Date: Wed, 17 Jul 2024 16:12:53 +0200 Subject: [PATCH] Do not render error messages as HTML by default --- .../src/lib/constants/ledger-app.constants.ts | 2 +- frontend/src/lib/stores/toasts.store.ts | 7 --- frontend/src/lib/types/ledger.errors.ts | 4 +- frontend/src/lib/utils/error.utils.ts | 6 ++- .../lib/identities/ledger.identity.spec.ts | 19 +++---- .../src/tests/lib/utils/error.utils.spec.ts | 27 ++++++++-- .../src/tests/lib/utils/ledger.utils.spec.ts | 51 ++++++++++--------- 7 files changed, 65 insertions(+), 51 deletions(-) diff --git a/frontend/src/lib/constants/ledger-app.constants.ts b/frontend/src/lib/constants/ledger-app.constants.ts index 59d35beb100..fef496d64f6 100644 --- a/frontend/src/lib/constants/ledger-app.constants.ts +++ b/frontend/src/lib/constants/ledger-app.constants.ts @@ -9,4 +9,4 @@ export const CANDID_PARSER_VERSION = "2.2.1"; // Version published in February 2023 export const SNS_SUPPORT_VERSION = "2.3.0"; // Version published in October 2023. Includes all transactions supported in Candid -export const ALL_CANDID_TXS_VERSION = "3.4.9"; +export const ALL_CANDID_TXS_VERSION = "2.4.9"; diff --git a/frontend/src/lib/stores/toasts.store.ts b/frontend/src/lib/stores/toasts.store.ts index 2f8cddacdaa..2f1b22481d4 100644 --- a/frontend/src/lib/stores/toasts.store.ts +++ b/frontend/src/lib/stores/toasts.store.ts @@ -57,13 +57,6 @@ export const toastsError = ({ console.error(err); } - console.log( - "dskloetx toastsError", - labelKey, - err, - substitutions, - new Error().stack - ); return toastsShow({ labelKey, level: "error", diff --git a/frontend/src/lib/types/ledger.errors.ts b/frontend/src/lib/types/ledger.errors.ts index 583fcbebb8c..ddace4fc5cd 100644 --- a/frontend/src/lib/types/ledger.errors.ts +++ b/frontend/src/lib/types/ledger.errors.ts @@ -4,12 +4,12 @@ export class LedgerErrorKey extends Error { // Optional substitutions values that can be used to fill the error message substitutions?: I18nSubstitutions; // Whether to render the error message as HTML. - renderAsHtml?: boolean; + renderAsHtml: boolean; constructor({ message, substitutions, - renderAsHtml, + renderAsHtml = false, }: { message?: string; substitutions?: I18nSubstitutions; diff --git a/frontend/src/lib/utils/error.utils.ts b/frontend/src/lib/utils/error.utils.ts index d6085f5f70a..79c38d58588 100644 --- a/frontend/src/lib/utils/error.utils.ts +++ b/frontend/src/lib/utils/error.utils.ts @@ -130,7 +130,7 @@ export const toToastError = ({ renderAsHtml: boolean; } => { let errorKey = false; - const error = err as Error; + const error = err as Error | undefined; const message: string | undefined = error?.message; if (message !== undefined) { @@ -139,7 +139,9 @@ export const toToastError = ({ } const renderAsHtml = - "renderAsHtml" in error ? (error.renderAsHtml as boolean) : false; + nonNullish(error) && "renderAsHtml" in error + ? (error.renderAsHtml as boolean) + : false; type ErrorSubstitutions = { substitutions?: I18nSubstitutions }; diff --git a/frontend/src/tests/lib/identities/ledger.identity.spec.ts b/frontend/src/tests/lib/identities/ledger.identity.spec.ts index b1bba1a03ab..8ca7aece9f0 100644 --- a/frontend/src/tests/lib/identities/ledger.identity.spec.ts +++ b/frontend/src/tests/lib/identities/ledger.identity.spec.ts @@ -1,6 +1,5 @@ import { LedgerIdentity } from "$lib/identities/ledger.identity"; import { Secp256k1PublicKey } from "$lib/keys/secp256k1"; -import { LedgerErrorKey } from "$lib/types/ledger.errors"; import { getRequestId } from "$lib/utils/ledger.utils"; import { mockPrincipal } from "$tests/mocks/auth.store.mock"; import { mockCanisterId } from "$tests/mocks/canisters.mock"; @@ -245,11 +244,10 @@ describe("LedgerIdentity", () => { const identity = await LedgerIdentity.create(); const call = () => identity.transformRequest(mockHttpRequest1); - expect(call).rejects.toThrowError( - new LedgerErrorKey({ - message: "error__ledger.app_version_not_supported", - }) - ); + expect(call).rejects.toMatchObject({ + message: "error__ledger.app_version_not_supported", + renderAsHtml: true, + }); expect(mockLedgerApp.signUpdateCall).not.toBeCalled(); }); @@ -270,11 +268,10 @@ describe("LedgerIdentity", () => { identity.flagUpcomingStakeNeuron(); const call = () => identity.transformRequest(mockHttpRequest1); - expect(call).rejects.toThrowError( - new LedgerErrorKey({ - message: "error__ledger.app_version_not_supported", - }) - ); + expect(call).rejects.toMatchObject({ + message: "error__ledger.app_version_not_supported", + renderAsHtml: true, + }); expect(mockLedgerApp.sign).not.toBeCalled(); }); }); diff --git a/frontend/src/tests/lib/utils/error.utils.spec.ts b/frontend/src/tests/lib/utils/error.utils.spec.ts index 0a63a013358..e9ed0b84a7e 100644 --- a/frontend/src/tests/lib/utils/error.utils.spec.ts +++ b/frontend/src/tests/lib/utils/error.utils.spec.ts @@ -1,3 +1,5 @@ +import { LedgerErrorKey } from "$lib/types/ledger.errors"; + import { HardwareWalletAttachError } from "$lib/canisters/nns-dapp/nns-dapp.errors"; import { errorToString, @@ -44,7 +46,7 @@ describe("error-utils", () => { fallbackErrorLabelKey: "test.test", err: undefined, }) - ).toEqual({ labelKey: "test.test", renderAsHtml: true }); + ).toEqual({ labelKey: "test.test", renderAsHtml: false }); const err = new HardwareWalletAttachError("test"); @@ -53,14 +55,14 @@ describe("error-utils", () => { fallbackErrorLabelKey: "test.test", err, }) - ).toEqual({ labelKey: "test.test", err, renderAsHtml: true }); + ).toEqual({ labelKey: "test.test", err, renderAsHtml: false }); expect( toToastError({ fallbackErrorLabelKey: "test.test", err, }) - ).toEqual({ labelKey: "test.test", err, renderAsHtml: true }); + ).toEqual({ labelKey: "test.test", err, renderAsHtml: false }); }); it("should use error message key", () => { @@ -71,7 +73,24 @@ describe("error-utils", () => { fallbackErrorLabelKey: "test.test", err, }) - ).toEqual({ labelKey: "error.rename_subaccount", renderAsHtml: true }); + ).toEqual({ labelKey: "error.rename_subaccount", renderAsHtml: false }); + }); + + it("should pass on renderAsHtml", () => { + const err = new LedgerErrorKey({ + message: "error__ledger.app_version_not_supported", + renderAsHtml: true, + }); + + expect( + toToastError({ + fallbackErrorLabelKey: "test.test", + err, + }) + ).toEqual({ + labelKey: "error__ledger.app_version_not_supported", + renderAsHtml: true, + }); }); }); diff --git a/frontend/src/tests/lib/utils/ledger.utils.spec.ts b/frontend/src/tests/lib/utils/ledger.utils.spec.ts index 85dff0e4752..17803267a63 100644 --- a/frontend/src/tests/lib/utils/ledger.utils.spec.ts +++ b/frontend/src/tests/lib/utils/ledger.utils.spec.ts @@ -1,5 +1,5 @@ import { ExtendedLedgerError } from "$lib/constants/ledger.constants"; -import { LedgerErrorKey } from "$lib/types/ledger.errors"; +import { LedgerErrorMessage } from "$lib/types/ledger.errors"; import { decodePublicKey, decodeSignature } from "$lib/utils/ledger.utils"; import { mockPrincipalText } from "$tests/mocks/auth.store.mock"; import { @@ -26,9 +26,10 @@ describe("ledger-utils", () => { returnCode: ExtendedLedgerError.AppNotOpen as unknown as LedgerError, } as ResponseAddress); - expect(call).rejects.toThrow( - new LedgerErrorKey({ message: "error__ledger.please_open" }) - ); + expect(call).rejects.toMatchObject({ + message: "error__ledger.please_open", + renderAsHtml: false, + }); }); it("should throw an error because ledger is locked", () => { @@ -39,9 +40,10 @@ describe("ledger-utils", () => { returnCode: LedgerError.TransactionRejected, } as ResponseAddress); - expect(call).rejects.toThrow( - new LedgerErrorKey({ message: "error__ledger.locked" }) - ); + expect(call).rejects.toMatchObject({ + message: "error__ledger.locked", + renderAsHtml: false, + }); }); it("should throw an error because public key cannot be fetched", () => { @@ -53,9 +55,10 @@ describe("ledger-utils", () => { ExtendedLedgerError.CannotFetchPublicKey as unknown as LedgerError, } as ResponseAddress); - expect(call).rejects.toThrow( - new LedgerErrorKey({ message: "error__ledger.fetch_public_key" }) - ); + expect(call).rejects.toMatchObject({ + message: "error__ledger.fetch_public_key", + renderAsHtml: false, + }); }); it("should throw an error because principal does not match", async () => { @@ -65,9 +68,10 @@ describe("ledger-utils", () => { publicKey: fromHexString(rawPublicKeyHex) as unknown as Buffer, } as ResponseAddress); - expect(call).rejects.toThrow( - new LedgerErrorKey({ message: "error__ledger.principal_not_match" }) - ); + expect(call).rejects.toMatchObject({ + message: "error__ledger.principal_not_match", + renderAsHtml: false, + }); }); it("should return a Secp256k1 public key", async () => { @@ -90,9 +94,9 @@ describe("ledger-utils", () => { } as unknown as ResponseSign); expect(call).rejects.toThrow( - new LedgerErrorKey({ - message: `A ledger error happened during signature. undefined (code ${LedgerError.UnknownError}).`, - }) + new LedgerErrorMessage( + `A ledger error happened during signature. undefined (code ${LedgerError.UnknownError}).` + ) ); }); @@ -103,11 +107,10 @@ describe("ledger-utils", () => { returnCode: LedgerError.TransactionRejected, } as unknown as ResponseSign); - expect(call).rejects.toThrow( - new LedgerErrorKey({ - message: "error__ledger.user_rejected_transaction", - }) - ); + expect(call).rejects.toMatchObject({ + message: "error__ledger.user_rejected_transaction", + renderAsHtml: false, + }); }); it("should throw an error if signature too short", () => { @@ -120,9 +123,9 @@ describe("ledger-utils", () => { } as unknown as ResponseSign); expect(call).rejects.toThrow( - new LedgerErrorKey({ - message: `Signature must be 64 bytes long (is ${test.length})`, - }) + new LedgerErrorMessage( + `Signature must be 64 bytes long (is ${test.length})` + ) ); });