diff --git a/package.json b/package.json index 47397283..c6069d83 100644 --- a/package.json +++ b/package.json @@ -50,8 +50,18 @@ }, "jest": { "testPathIgnorePatterns": [ - "build" - ] + "build", + ".claude/worktrees" + ], + "moduleNameMapper": { + "^@generated/(.*)$": "/generated-sources/$1", + "^src/(.*)$": "/src/$1", + "^components/(.*)$": "/src/components/$1", + "^assets/(.*)$": "/src/assets/$1", + "^context/(.*)$": "/src/context/$1", + "^hooks/(.*)$": "/src/hooks/$1", + "^services/(.*)$": "/src/services/$1" + } }, "devDependencies": { "@babel/preset-env": "^7.23.2", diff --git a/src/components/Configure/content/ConfigureInstallationBase.tsx b/src/components/Configure/content/ConfigureInstallationBase.tsx index db656a23..d36ae049 100644 --- a/src/components/Configure/content/ConfigureInstallationBase.tsx +++ b/src/components/Configure/content/ConfigureInstallationBase.tsx @@ -192,7 +192,7 @@ export function ConfigureInstallationBase({ }} > {errorMsg && ( - + {typeof errorMsg === "string" ? errorMsg : "Installation Failed."} )} diff --git a/src/components/Configure/content/fields/ReadFields.tsx b/src/components/Configure/content/fields/ReadFields.tsx index b4558b8b..d991b607 100644 --- a/src/components/Configure/content/fields/ReadFields.tsx +++ b/src/components/Configure/content/fields/ReadFields.tsx @@ -11,7 +11,13 @@ export function ReadFields() { useClearOldFieldMappings(); return ( - <> +
@@ -20,6 +26,6 @@ export function ReadFields() { - +
); } diff --git a/src/components/FormErrorBox.tsx b/src/components/FormErrorBox.tsx index d6692fca..3709021e 100644 --- a/src/components/FormErrorBox.tsx +++ b/src/components/FormErrorBox.tsx @@ -7,11 +7,83 @@ const defaultStyle = { padding: ".5rem 1rem", }; +/** Separates detail and remedy in serialized error strings from handleServerError. */ +const REMEDY_DELIMITER = "\x1e"; + +const LEGACY_REMEDY_PATTERN = /\n\n\[Remedy\]\s*(.*)$/s; + +function parseErrorMessage(message: string): { detail: string; remedy?: string } { + const delimiterIndex = message.indexOf(REMEDY_DELIMITER); + if (delimiterIndex !== -1) { + return { + detail: message.slice(0, delimiterIndex), + remedy: message.slice(delimiterIndex + REMEDY_DELIMITER.length), + }; + } + + const legacyMatch = message.match(LEGACY_REMEDY_PATTERN); + if (legacyMatch) { + return { + detail: message.slice(0, legacyMatch.index), + remedy: legacyMatch[1], + }; + } + + return { detail: message }; +} + +function ErrorMessageContent({ message }: { message: string }) { + const { detail, remedy } = parseErrorMessage(message); + + return ( + <> +

+ {detail} +

+ {remedy && ( +
+ + How to fix + +

+ {remedy} +

+
+ )} + + ); +} + type FormErrorBoxProps = { children: React.ReactNode; style?: React.CSSProperties; }; export function FormErrorBox({ children, style }: FormErrorBoxProps) { - return {children}; + return ( + + {typeof children === "string" ? ( + + ) : ( + children + )} + + ); } diff --git a/src/utils/handleServerError.ts b/src/utils/handleServerError.ts index 236ecc7c..f378f80d 100644 --- a/src/utils/handleServerError.ts +++ b/src/utils/handleServerError.ts @@ -1,5 +1,8 @@ import { ResponseError } from "@generated/api/src"; +/** Separates detail and remedy in error strings passed to setError. */ +const REMEDY_DELIMITER = "\x1e"; + /** * handles server error generated by sdk (Response Error) and calls setError callback if provided * @param error could be unknown error or ResponseError @@ -37,7 +40,9 @@ export const handleServerError = async ( } } - const combinedErrorMessage = `${errorMsg} ${errorBody?.remedy ? `\n\n[Remedy] ${errorBody.remedy}` : ""}`; + const combinedErrorMessage = errorBody?.remedy + ? `${errorMsg?.trimEnd() ?? ""}${REMEDY_DELIMITER}${errorBody.remedy}` + : (errorMsg ?? ""); if (setError) setError(combinedErrorMessage); } catch (e) { console.error("Error parsing error response body:", e); // the response body could already be parsed diff --git a/test/utils/handleServerError.test.ts b/test/utils/handleServerError.test.ts new file mode 100644 index 00000000..76f25462 --- /dev/null +++ b/test/utils/handleServerError.test.ts @@ -0,0 +1,124 @@ +import { ResponseError } from "@generated/api/src"; +import { handleServerError } from "src/utils/handleServerError"; + +/** Matches the delimiter in handleServerError. */ +const REMEDY_DELIMITER = "\x1e"; + +function createMockResponse( + status: number, + statusText: string, + body: object, +): Response { + return new Response(JSON.stringify(body), { + status, + statusText, + headers: { "Content-Type": "application/json" }, + }); +} + +describe("handleServerError", () => { + let consoleErrorSpy: jest.SpyInstance; + + beforeEach(() => { + consoleErrorSpy = jest.spyOn(console, "error").mockImplementation(() => {}); + }); + + afterEach(() => { + consoleErrorSpy.mockRestore(); + }); + + describe("ResponseError handling", () => { + it("calls setError with causes joined by newline", async () => { + const response = createMockResponse(400, "Bad Request", { + causes: ["field X is required", "field Y is invalid"], + detail: "validation failed", + }); + const error = new ResponseError(response, "Bad Request"); + const setError = jest.fn(); + + await handleServerError(error, setError); + + expect(setError).toHaveBeenCalledWith( + "field X is required\nfield Y is invalid", + ); + }); + + it("calls setError with detail when causes is absent", async () => { + const response = createMockResponse(400, "Bad Request", { + detail: "something went wrong", + }); + const error = new ResponseError(response, "Bad Request"); + const setError = jest.fn(); + + await handleServerError(error, setError); + + expect(setError).toHaveBeenCalledWith("something went wrong"); + }); + + it("includes remedy in the error message when present", async () => { + const response = createMockResponse(400, "Bad Request", { + detail: "missing scope", + remedy: "Add the 'read' scope to your OAuth app", + }); + const error = new ResponseError(response, "Bad Request"); + const setError = jest.fn(); + + await handleServerError(error, setError); + + expect(setError).toHaveBeenCalledWith( + `missing scope${REMEDY_DELIMITER}Add the 'read' scope to your OAuth app`, + ); + }); + + it("does not call setError when setError is not provided", async () => { + const response = createMockResponse(500, "Internal Server Error", { + detail: "db error", + }); + const error = new ResponseError(response, "Internal Server Error"); + + await handleServerError(error); + }); + + it("handles non-JSON response body gracefully", async () => { + const response = new Response("not json", { + status: 500, + statusText: "Internal Server Error", + }); + const error = new ResponseError(response, "Internal Server Error"); + const setError = jest.fn(); + + await handleServerError(error, setError); + + expect(setError).not.toHaveBeenCalled(); + expect(consoleErrorSpy).toHaveBeenNthCalledWith( + 2, + "Error parsing error response body:", + expect.objectContaining({ + message: expect.stringContaining("not valid JSON"), + }), + ); + }); + }); + + describe("non-ResponseError handling", () => { + it("calls setError with error.message for non-ResponseError", async () => { + const error = new Error("network failure"); + const setError = jest.fn(); + + await handleServerError(error, setError); + + expect(setError).toHaveBeenCalledWith("network failure"); + }); + + it("does not call setError when setError is not provided for non-ResponseError", async () => { + const error = new Error("network failure"); + + await handleServerError(error); + + expect(consoleErrorSpy).toHaveBeenCalledWith( + "Unexpected error:", + "network failure", + ); + }); + }); +});