Skip to content

Commit

Permalink
Do not render error messages as HTML by default
Browse files Browse the repository at this point in the history
  • Loading branch information
dskloetd committed Jul 17, 2024
1 parent 79fef92 commit 5913874
Show file tree
Hide file tree
Showing 7 changed files with 65 additions and 51 deletions.
2 changes: 1 addition & 1 deletion frontend/src/lib/constants/ledger-app.constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
7 changes: 0 additions & 7 deletions frontend/src/lib/stores/toasts.store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/lib/types/ledger.errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 4 additions & 2 deletions frontend/src/lib/utils/error.utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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 };

Expand Down
19 changes: 8 additions & 11 deletions frontend/src/tests/lib/identities/ledger.identity.spec.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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();
});

Expand All @@ -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();
});
});
Expand Down
27 changes: 23 additions & 4 deletions frontend/src/tests/lib/utils/error.utils.spec.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { LedgerErrorKey } from "$lib/types/ledger.errors";

import { HardwareWalletAttachError } from "$lib/canisters/nns-dapp/nns-dapp.errors";
import {
errorToString,
Expand Down Expand Up @@ -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");

Expand All @@ -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", () => {
Expand All @@ -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,
});
});
});

Expand Down
51 changes: 27 additions & 24 deletions frontend/src/tests/lib/utils/ledger.utils.spec.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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", () => {
Expand All @@ -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", () => {
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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}).`
)
);
});

Expand All @@ -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", () => {
Expand All @@ -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})`
)
);
});

Expand Down

0 comments on commit 5913874

Please sign in to comment.