From 12e150072b87e906053a037b33b5e9803c9796f3 Mon Sep 17 00:00:00 2001 From: Christian Fehmer Date: Tue, 4 Feb 2025 11:08:46 +0100 Subject: [PATCH] fix: fix error handling in parseWithSchema (@fehmer) (#6229) `instanceof ZodError` is not working if the code is packaged as a module for unknown reason. Found while adding tests for the url-handler in #6207. --- frontend/__tests__/utils/url-handler.spec.ts | 277 +++++++++++++++++++ frontend/src/ts/test/custom-text.ts | 2 +- frontend/src/ts/utils/url-handler.ts | 7 +- packages/util/src/json.ts | 7 +- 4 files changed, 287 insertions(+), 6 deletions(-) create mode 100644 frontend/__tests__/utils/url-handler.spec.ts diff --git a/frontend/__tests__/utils/url-handler.spec.ts b/frontend/__tests__/utils/url-handler.spec.ts new file mode 100644 index 000000000..f4dd7a13e --- /dev/null +++ b/frontend/__tests__/utils/url-handler.spec.ts @@ -0,0 +1,277 @@ +import { Difficulty, Mode, Mode2 } from "@monkeytype/contracts/schemas/shared"; +import { compressToURI } from "lz-ts"; +import * as UpdateConfig from "../../src/ts/config"; +import * as Notifications from "../../src/ts/elements/notifications"; +import { CustomTextSettings } from "../../src/ts/test/custom-text"; +import * as TestLogic from "../../src/ts/test/test-logic"; +import * as TestState from "../../src/ts/test/test-state"; +import * as Misc from "../../src/ts/utils/misc"; +import { loadTestSettingsFromUrl } from "../../src/ts/utils/url-handler"; + +//mock modules to avoid dependencies +vi.mock("../../src/ts/test/test-logic", () => ({ + restart: vi.fn(), +})); + +describe("url-handler", () => { + describe("loadTestSettingsFromUrl", () => { + const findGetParameterMock = vi.spyOn(Misc, "findGetParameter"); + + const setModeMock = vi.spyOn(UpdateConfig, "setMode"); + const setTimeConfigMock = vi.spyOn(UpdateConfig, "setTimeConfig"); + const setWordCountMock = vi.spyOn(UpdateConfig, "setWordCount"); + const setQuoteLengthMock = vi.spyOn(UpdateConfig, "setQuoteLength"); + const setSelectedQuoteIdMock = vi.spyOn(TestState, "setSelectedQuoteId"); + const setPunctuationMock = vi.spyOn(UpdateConfig, "setPunctuation"); + const setNumbersMock = vi.spyOn(UpdateConfig, "setNumbers"); + const setLanguageMock = vi.spyOn(UpdateConfig, "setLanguage"); + const setDifficultyMock = vi.spyOn(UpdateConfig, "setDifficulty"); + const setFunboxMock = vi.spyOn(UpdateConfig, "setFunbox"); + + const restartTestMock = vi.spyOn(TestLogic, "restart"); + const addNotificationMock = vi.spyOn(Notifications, "add"); + + beforeEach(() => { + [ + findGetParameterMock, + setModeMock, + setTimeConfigMock, + setWordCountMock, + setQuoteLengthMock, + setSelectedQuoteIdMock, + setPunctuationMock, + setNumbersMock, + setLanguageMock, + setDifficultyMock, + setFunboxMock, + restartTestMock, + addNotificationMock, + ].forEach((it) => it.mockReset()); + + findGetParameterMock.mockImplementation((override) => override); + }); + afterEach(() => {}); + + it("handles null", () => { + //GIVEN + findGetParameterMock.mockReturnValue("null"); + + //WHEN + loadTestSettingsFromUrl(""); + + //THEN + expect(setModeMock).not.toHaveBeenCalled(); + }); + it("handles mode2 as number", () => { + //GIVEN + findGetParameterMock.mockReturnValue( + urlData({ mode: "time", mode2: 60 }) + ); + + //WHEN + loadTestSettingsFromUrl(""); + + //THEN + expect(setModeMock).toHaveBeenCalledWith("time", true); + expect(setTimeConfigMock).toHaveBeenCalledWith(60, true); + expect(restartTestMock).toHaveBeenCalled(); + }); + it("sets time", () => { + //GIVEN + findGetParameterMock.mockReturnValue( + urlData({ mode: "time", mode2: "30" }) + ); + + //WHEN + loadTestSettingsFromUrl(""); + + //THEN + expect(setModeMock).toHaveBeenCalledWith("time", true); + expect(setTimeConfigMock).toHaveBeenCalledWith(30, true); + expect(restartTestMock).toHaveBeenCalled(); + }); + it("sets word count", () => { + //GIVEN + findGetParameterMock.mockReturnValue( + urlData({ mode: "words", mode2: "50" }) + ); + + //WHEN + loadTestSettingsFromUrl(""); + + //THEN + expect(setModeMock).toHaveBeenCalledWith("words", true); + expect(setWordCountMock).toHaveBeenCalledWith(50, true); + expect(restartTestMock).toHaveBeenCalled(); + }); + it("sets quote length", () => { + //GIVEN + findGetParameterMock.mockReturnValue( + urlData({ mode: "quote", mode2: "512" }) + ); + + //WHEN + loadTestSettingsFromUrl(""); + + //THEN + expect(setModeMock).toHaveBeenCalledWith("quote", true); + expect(setQuoteLengthMock).toHaveBeenCalledWith(-2, false); + expect(setSelectedQuoteIdMock).toHaveBeenCalledWith(512); + expect(restartTestMock).toHaveBeenCalled(); + }); + it("sets punctuation", () => { + //GIVEN + findGetParameterMock.mockReturnValue(urlData({ punctuation: true })); + + //WHEN + loadTestSettingsFromUrl(""); + + //THEN + expect(setPunctuationMock).toHaveBeenCalledWith(true, true); + expect(restartTestMock).toHaveBeenCalled(); + }); + it("sets numbers", () => { + //GIVEN + findGetParameterMock.mockReturnValue(urlData({ numbers: false })); + + //WHEN + loadTestSettingsFromUrl(""); + + //THEN + expect(setNumbersMock).toHaveBeenCalledWith(false, true); + expect(restartTestMock).toHaveBeenCalled(); + }); + it("sets language", () => { + //GIVEN + findGetParameterMock.mockReturnValue(urlData({ language: "english" })); + + //WHEN + loadTestSettingsFromUrl(""); + + //THEN + expect(setLanguageMock).toHaveBeenCalledWith("english", true); + expect(restartTestMock).toHaveBeenCalled(); + }); + it("sets difficulty", () => { + //GIVEN + findGetParameterMock.mockReturnValue(urlData({ difficulty: "master" })); + + //WHEN + loadTestSettingsFromUrl(""); + + //THEN + expect(setDifficultyMock).toHaveBeenCalledWith("master", true); + expect(restartTestMock).toHaveBeenCalled(); + }); + it("sets funbox", () => { + //GIVEN + findGetParameterMock.mockReturnValue( + urlData({ funbox: "crt#choo_choo" }) + ); + + //WHEN + loadTestSettingsFromUrl(""); + + //THEN + expect(setFunboxMock).toHaveBeenCalledWith("crt#choo_choo", true); + expect(restartTestMock).toHaveBeenCalled(); + }); + it("adds notification", () => { + //GIVEN + findGetParameterMock.mockReturnValue( + urlData({ + mode: "time", + mode2: "60", + customText: { + text: ["abcabc"], + limit: { value: 5, mode: "time" }, + mode: "random", + pipeDelimiter: true, + }, + punctuation: true, + numbers: true, + language: "english", + difficulty: "master", + funbox: "a#b", + }) + ); + + //WHEN + loadTestSettingsFromUrl(""); + + //THEN + expect(addNotificationMock).toHaveBeenCalledWith( + "Settings applied from URL:

mode: time
mode2: 60
custom text settings
punctuation: on
numbers: on
language: english
difficulty: master
funbox: a#b
", + 1, + { + duration: 10, + allowHTML: true, + } + ); + }); + it("rejects invalid values", () => { + //GIVEN + findGetParameterMock.mockReturnValue( + urlData({ + mode: "invalidMode", + mode2: "invalidMode2", + customText: { + text: "invalid", + limit: "invalid", + mode: "invalid", + pipeDelimiter: "invalid", + }, + punctuation: "invalid", + numbers: "invalid", + language: "invalid", + difficulty: "invalid", + funbox: ["invalid"], + } as any) + ); + + //WHEN + loadTestSettingsFromUrl(""); + + //THEN + expect(addNotificationMock).toHaveBeenCalledWith( + `Failed to load test settings from URL: \"0\" Invalid enum value. Expected 'time' | 'words' | 'quote' | 'custom' | 'zen', received 'invalidMode' +\"1\" Needs to be a number or a number represented as a string e.g. \"10\". +\"2.text\" Expected array, received string +\"2.mode\" Invalid enum value. Expected 'repeat' | 'random' | 'shuffle', received 'invalid' +\"2.limit\" Expected object, received string +\"2.pipeDelimiter\" Expected boolean, received string +\"3\" Expected boolean, received string +\"4\" Expected boolean, received string +\"6\" Invalid enum value. Expected 'normal' | 'expert' | 'master', received 'invalid' +\"7\" Expected string, received array`, + 0 + ); + }); + }); +}); + +const urlData = ( + data: Partial<{ + mode: Mode | undefined; + mode2: Mode2 | number; + customText: CustomTextSettings; + punctuation: boolean; + numbers: boolean; + language: string; + difficulty: Difficulty; + funbox: string; + }> +): string => { + return compressToURI( + JSON.stringify([ + data.mode ?? null, + data.mode2 ?? null, + data.customText ?? null, + data.punctuation ?? null, + data.numbers ?? null, + data.language ?? null, + data.difficulty ?? null, + data.funbox ?? null, + ]) + ); +}; diff --git a/frontend/src/ts/test/custom-text.ts b/frontend/src/ts/test/custom-text.ts index 9837fc554..7be63df6a 100644 --- a/frontend/src/ts/test/custom-text.ts +++ b/frontend/src/ts/test/custom-text.ts @@ -36,7 +36,7 @@ export const CustomTextSettingsSchema = z.object({ pipeDelimiter: z.boolean(), }); -type CustomTextSettings = z.infer; +export type CustomTextSettings = z.infer; type CustomTextLimit = z.infer["limit"]; diff --git a/frontend/src/ts/utils/url-handler.ts b/frontend/src/ts/utils/url-handler.ts index 4f577fea6..f0d16bbdb 100644 --- a/frontend/src/ts/utils/url-handler.ts +++ b/frontend/src/ts/utils/url-handler.ts @@ -169,12 +169,13 @@ export function loadTestSettingsFromUrl(getOverride?: string): void { applied["mode"] = de[0]; } + const mode = de[0] ?? Config.mode; if (de[1] !== null) { - if (Config.mode === "time") { + if (mode === "time") { UpdateConfig.setTimeConfig(parseInt(de[1], 10), true); - } else if (Config.mode === "words") { + } else if (mode === "words") { UpdateConfig.setWordCount(parseInt(de[1], 10), true); - } else if (Config.mode === "quote") { + } else if (mode === "quote") { UpdateConfig.setQuoteLength(-2, false); TestState.setSelectedQuoteId(parseInt(de[1], 10)); ManualRestart.set(); diff --git a/packages/util/src/json.ts b/packages/util/src/json.ts index 7606ff60f..fc1dc9a30 100644 --- a/packages/util/src/json.ts +++ b/packages/util/src/json.ts @@ -16,8 +16,11 @@ export function parseWithSchema( // eslint-disable-next-line @typescript-eslint/no-unsafe-return return schema.parse(jsonParsed) as z.infer; } catch (error) { - if (error instanceof ZodError) { - throw new Error(error.issues.map(prettyErrorMessage).join("\n")); + // instanceof ZodError is not working from our module + if ((error as ZodError)["issues"] !== undefined) { + throw new Error( + (error as ZodError).issues.map(prettyErrorMessage).join("\n") + ); } else { throw error; }