From db0667064832165da34a853c27c7a97df19f4e44 Mon Sep 17 00:00:00 2001 From: Dion Low Date: Thu, 18 Jun 2026 14:22:10 -0700 Subject: [PATCH 1/3] fix(Configure): polish API error/remedy display and callout spacing Replace the visible [Remedy] token with a shared ProblemMessage component that renders detail and remedy separately, and add spacing between save errors and the read-object success callout in the Configure UI. Co-authored-by: Cursor --- .../content/ConfigureInstallationBase.tsx | 2 +- .../content/fields/ObjectErrorAlert.tsx | 5 +- .../Configure/content/fields/ReadFields.tsx | 10 +++- .../fields/objectErrorAlert.module.css | 1 - src/components/ErrorTextBox/ErrorTextBox.tsx | 4 +- src/components/FormErrorBox.tsx | 12 ++++- .../ProblemMessage/ProblemMessage.tsx | 30 ++++++++++++ .../ProblemMessage/problemMessage.module.css | 33 +++++++++++++ src/utils/apiProblem.ts | 49 +++++++++++++++++++ src/utils/handleServerError.ts | 5 +- 10 files changed, 143 insertions(+), 8 deletions(-) create mode 100644 src/components/ProblemMessage/ProblemMessage.tsx create mode 100644 src/components/ProblemMessage/problemMessage.module.css create mode 100644 src/utils/apiProblem.ts diff --git a/src/components/Configure/content/ConfigureInstallationBase.tsx b/src/components/Configure/content/ConfigureInstallationBase.tsx index db656a235..d36ae0497 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/ObjectErrorAlert.tsx b/src/components/Configure/content/fields/ObjectErrorAlert.tsx index d93dd6e0d..696edf772 100644 --- a/src/components/Configure/content/fields/ObjectErrorAlert.tsx +++ b/src/components/Configure/content/fields/ObjectErrorAlert.tsx @@ -1,4 +1,5 @@ import { ExclamationTriangleIcon } from "@radix-ui/react-icons"; +import { ProblemMessage } from "src/components/ProblemMessage/ProblemMessage"; import classes from "./objectErrorAlert.module.css"; @@ -13,7 +14,9 @@ export function ObjectErrorAlert({ error }: ObjectErrorAlertProps) { Unable to load object -
{error}
+
+ +
); } diff --git a/src/components/Configure/content/fields/ReadFields.tsx b/src/components/Configure/content/fields/ReadFields.tsx index b4558b8b7..d991b607f 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/Configure/content/fields/objectErrorAlert.module.css b/src/components/Configure/content/fields/objectErrorAlert.module.css index 12c96ad96..63a09d436 100644 --- a/src/components/Configure/content/fields/objectErrorAlert.module.css +++ b/src/components/Configure/content/fields/objectErrorAlert.module.css @@ -32,6 +32,5 @@ color: var(--amp-colors-status-critical-dark); line-height: 1.5; padding-left: 1.5rem; - font-family: ui-monospace, SFMono-Regular, "SF Mono", Menlo, Monaco, Consolas, "Liberation Mono", "Courier New", monospace; word-break: break-word; } diff --git a/src/components/ErrorTextBox/ErrorTextBox.tsx b/src/components/ErrorTextBox/ErrorTextBox.tsx index 02cd0284e..d1c166c9e 100644 --- a/src/components/ErrorTextBox/ErrorTextBox.tsx +++ b/src/components/ErrorTextBox/ErrorTextBox.tsx @@ -1,5 +1,7 @@ import { ErrorIcon } from "assets/ErrorIcon"; +import { ProblemMessage } from "components/ProblemMessage/ProblemMessage"; + import { Box } from "../ui-base/Box/Box"; import { Container } from "../ui-base/Container/Container"; @@ -16,7 +18,7 @@ export function InnerErrorTextBox({ message }: { message: string }) {
-

{message}

+ ); } diff --git a/src/components/FormErrorBox.tsx b/src/components/FormErrorBox.tsx index d6692fcab..d71d3d90f 100644 --- a/src/components/FormErrorBox.tsx +++ b/src/components/FormErrorBox.tsx @@ -1,5 +1,7 @@ import { Box } from "src/components/ui-base/Box/Box"; +import { ProblemMessage } from "components/ProblemMessage/ProblemMessage"; + const defaultStyle = { backgroundColor: "var(--amp-colors-status-critical-muted)", borderColor: "var(--amp-colors-status-critical-muted)", @@ -13,5 +15,13 @@ type FormErrorBoxProps = { }; export function FormErrorBox({ children, style }: FormErrorBoxProps) { - return {children}; + return ( + + {typeof children === "string" ? ( + + ) : ( + children + )} + + ); } diff --git a/src/components/ProblemMessage/ProblemMessage.tsx b/src/components/ProblemMessage/ProblemMessage.tsx new file mode 100644 index 000000000..0c437d769 --- /dev/null +++ b/src/components/ProblemMessage/ProblemMessage.tsx @@ -0,0 +1,30 @@ +import { ApiProblemParts, parseApiProblemMessage } from "src/utils/apiProblem"; + +import classes from "./problemMessage.module.css"; + +type ProblemMessageProps = + | { message: string; detail?: never; remedy?: never } + | { message?: never; detail: string; remedy?: string }; + +function getParts(props: ProblemMessageProps): ApiProblemParts { + if ("message" in props && props.message !== undefined) { + return parseApiProblemMessage(props.message); + } + return { detail: props.detail, remedy: props.remedy }; +} + +export function ProblemMessage(props: ProblemMessageProps) { + const { detail, remedy } = getParts(props); + + return ( +
+

{detail}

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

{remedy}

+
+ )} +
+ ); +} diff --git a/src/components/ProblemMessage/problemMessage.module.css b/src/components/ProblemMessage/problemMessage.module.css new file mode 100644 index 000000000..b390fbfd9 --- /dev/null +++ b/src/components/ProblemMessage/problemMessage.module.css @@ -0,0 +1,33 @@ +.root { + display: flex; + flex-direction: column; + gap: 0; +} + +.detail { + margin: 0; + line-height: 1.5; + white-space: pre-line; +} + +.remedyBlock { + margin-top: 0.75rem; + padding-top: 0.75rem; + border-top: 1px solid var(--amp-colors-status-critical-dark, rgba(0, 0, 0, 0.15)); +} + +.remedyLabel { + display: block; + font-size: 0.75rem; + font-weight: 600; + text-transform: uppercase; + letter-spacing: 0.025em; + margin-bottom: 0.25rem; + opacity: 0.85; +} + +.remedy { + margin: 0; + line-height: 1.5; + white-space: pre-line; +} diff --git a/src/utils/apiProblem.ts b/src/utils/apiProblem.ts new file mode 100644 index 000000000..31acf1757 --- /dev/null +++ b/src/utils/apiProblem.ts @@ -0,0 +1,49 @@ +export type ApiProblemParts = { + detail: string; + remedy?: string; +}; + +/** Internal delimiter between detail and remedy in serialized error strings. */ +export const API_PROBLEM_REMEDY_DELIMITER = "\x1e"; + +const LEGACY_REMEDY_PATTERN = /\n\n\[Remedy\]\s*(.*)$/s; + +export function getApiProblemParts(body: { + causes?: string[]; + detail?: string; + remedy?: string; +}): ApiProblemParts { + const detail = body.causes?.join("\n") || body.detail || ""; + const remedy = body.remedy || undefined; + return remedy ? { detail, remedy } : { detail }; +} + +export function serializeApiProblem(parts: ApiProblemParts): string { + const trimmedDetail = parts.detail.trimEnd(); + if (!parts.remedy) { + return trimmedDetail; + } + return `${trimmedDetail}${API_PROBLEM_REMEDY_DELIMITER}${parts.remedy}`; +} + +export function parseApiProblemMessage(message: string): ApiProblemParts { + const delimiterIndex = message.indexOf(API_PROBLEM_REMEDY_DELIMITER); + if (delimiterIndex !== -1) { + return { + detail: message.slice(0, delimiterIndex), + remedy: message.slice( + delimiterIndex + API_PROBLEM_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 }; +} diff --git a/src/utils/handleServerError.ts b/src/utils/handleServerError.ts index 236ecc7c3..fd2a71c1d 100644 --- a/src/utils/handleServerError.ts +++ b/src/utils/handleServerError.ts @@ -1,4 +1,5 @@ import { ResponseError } from "@generated/api/src"; +import { getApiProblemParts, serializeApiProblem } from "src/utils/apiProblem"; /** * handles server error generated by sdk (Response Error) and calls setError callback if provided @@ -37,7 +38,9 @@ export const handleServerError = async ( } } - const combinedErrorMessage = `${errorMsg} ${errorBody?.remedy ? `\n\n[Remedy] ${errorBody.remedy}` : ""}`; + const combinedErrorMessage = serializeApiProblem( + getApiProblemParts(errorBody), + ); if (setError) setError(combinedErrorMessage); } catch (e) { console.error("Error parsing error response body:", e); // the response body could already be parsed From c303d05e8085ce201ef54bf04226804c3657bc16 Mon Sep 17 00:00:00 2001 From: Dion Low Date: Thu, 18 Jun 2026 14:27:19 -0700 Subject: [PATCH 2/3] refactor: simplify error/remedy fix to FormErrorBox and spacing only Drop shared apiProblem util and ProblemMessage component. Keep remedy rendering inline in FormErrorBox and spacing changes in Configure. Co-authored-by: Cursor --- .../content/fields/ObjectErrorAlert.tsx | 5 +- .../fields/objectErrorAlert.module.css | 1 + src/components/ErrorTextBox/ErrorTextBox.tsx | 4 +- src/components/FormErrorBox.tsx | 68 ++++++++++++++++++- .../ProblemMessage/ProblemMessage.tsx | 30 -------- .../ProblemMessage/problemMessage.module.css | 33 --------- src/utils/apiProblem.ts | 49 ------------- src/utils/handleServerError.ts | 10 +-- 8 files changed, 74 insertions(+), 126 deletions(-) delete mode 100644 src/components/ProblemMessage/ProblemMessage.tsx delete mode 100644 src/components/ProblemMessage/problemMessage.module.css delete mode 100644 src/utils/apiProblem.ts diff --git a/src/components/Configure/content/fields/ObjectErrorAlert.tsx b/src/components/Configure/content/fields/ObjectErrorAlert.tsx index 696edf772..d93dd6e0d 100644 --- a/src/components/Configure/content/fields/ObjectErrorAlert.tsx +++ b/src/components/Configure/content/fields/ObjectErrorAlert.tsx @@ -1,5 +1,4 @@ import { ExclamationTriangleIcon } from "@radix-ui/react-icons"; -import { ProblemMessage } from "src/components/ProblemMessage/ProblemMessage"; import classes from "./objectErrorAlert.module.css"; @@ -14,9 +13,7 @@ export function ObjectErrorAlert({ error }: ObjectErrorAlertProps) { Unable to load object -
- -
+
{error}
); } diff --git a/src/components/Configure/content/fields/objectErrorAlert.module.css b/src/components/Configure/content/fields/objectErrorAlert.module.css index 63a09d436..12c96ad96 100644 --- a/src/components/Configure/content/fields/objectErrorAlert.module.css +++ b/src/components/Configure/content/fields/objectErrorAlert.module.css @@ -32,5 +32,6 @@ color: var(--amp-colors-status-critical-dark); line-height: 1.5; padding-left: 1.5rem; + font-family: ui-monospace, SFMono-Regular, "SF Mono", Menlo, Monaco, Consolas, "Liberation Mono", "Courier New", monospace; word-break: break-word; } diff --git a/src/components/ErrorTextBox/ErrorTextBox.tsx b/src/components/ErrorTextBox/ErrorTextBox.tsx index d1c166c9e..02cd0284e 100644 --- a/src/components/ErrorTextBox/ErrorTextBox.tsx +++ b/src/components/ErrorTextBox/ErrorTextBox.tsx @@ -1,7 +1,5 @@ import { ErrorIcon } from "assets/ErrorIcon"; -import { ProblemMessage } from "components/ProblemMessage/ProblemMessage"; - import { Box } from "../ui-base/Box/Box"; import { Container } from "../ui-base/Container/Container"; @@ -18,7 +16,7 @@ export function InnerErrorTextBox({ message }: { message: string }) {
- +

{message}

); } diff --git a/src/components/FormErrorBox.tsx b/src/components/FormErrorBox.tsx index d71d3d90f..3709021ef 100644 --- a/src/components/FormErrorBox.tsx +++ b/src/components/FormErrorBox.tsx @@ -1,7 +1,5 @@ import { Box } from "src/components/ui-base/Box/Box"; -import { ProblemMessage } from "components/ProblemMessage/ProblemMessage"; - const defaultStyle = { backgroundColor: "var(--amp-colors-status-critical-muted)", borderColor: "var(--amp-colors-status-critical-muted)", @@ -9,6 +7,70 @@ 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; @@ -18,7 +80,7 @@ export function FormErrorBox({ children, style }: FormErrorBoxProps) { return ( {typeof children === "string" ? ( - + ) : ( children )} diff --git a/src/components/ProblemMessage/ProblemMessage.tsx b/src/components/ProblemMessage/ProblemMessage.tsx deleted file mode 100644 index 0c437d769..000000000 --- a/src/components/ProblemMessage/ProblemMessage.tsx +++ /dev/null @@ -1,30 +0,0 @@ -import { ApiProblemParts, parseApiProblemMessage } from "src/utils/apiProblem"; - -import classes from "./problemMessage.module.css"; - -type ProblemMessageProps = - | { message: string; detail?: never; remedy?: never } - | { message?: never; detail: string; remedy?: string }; - -function getParts(props: ProblemMessageProps): ApiProblemParts { - if ("message" in props && props.message !== undefined) { - return parseApiProblemMessage(props.message); - } - return { detail: props.detail, remedy: props.remedy }; -} - -export function ProblemMessage(props: ProblemMessageProps) { - const { detail, remedy } = getParts(props); - - return ( -
-

{detail}

- {remedy && ( -
- How to fix -

{remedy}

-
- )} -
- ); -} diff --git a/src/components/ProblemMessage/problemMessage.module.css b/src/components/ProblemMessage/problemMessage.module.css deleted file mode 100644 index b390fbfd9..000000000 --- a/src/components/ProblemMessage/problemMessage.module.css +++ /dev/null @@ -1,33 +0,0 @@ -.root { - display: flex; - flex-direction: column; - gap: 0; -} - -.detail { - margin: 0; - line-height: 1.5; - white-space: pre-line; -} - -.remedyBlock { - margin-top: 0.75rem; - padding-top: 0.75rem; - border-top: 1px solid var(--amp-colors-status-critical-dark, rgba(0, 0, 0, 0.15)); -} - -.remedyLabel { - display: block; - font-size: 0.75rem; - font-weight: 600; - text-transform: uppercase; - letter-spacing: 0.025em; - margin-bottom: 0.25rem; - opacity: 0.85; -} - -.remedy { - margin: 0; - line-height: 1.5; - white-space: pre-line; -} diff --git a/src/utils/apiProblem.ts b/src/utils/apiProblem.ts deleted file mode 100644 index 31acf1757..000000000 --- a/src/utils/apiProblem.ts +++ /dev/null @@ -1,49 +0,0 @@ -export type ApiProblemParts = { - detail: string; - remedy?: string; -}; - -/** Internal delimiter between detail and remedy in serialized error strings. */ -export const API_PROBLEM_REMEDY_DELIMITER = "\x1e"; - -const LEGACY_REMEDY_PATTERN = /\n\n\[Remedy\]\s*(.*)$/s; - -export function getApiProblemParts(body: { - causes?: string[]; - detail?: string; - remedy?: string; -}): ApiProblemParts { - const detail = body.causes?.join("\n") || body.detail || ""; - const remedy = body.remedy || undefined; - return remedy ? { detail, remedy } : { detail }; -} - -export function serializeApiProblem(parts: ApiProblemParts): string { - const trimmedDetail = parts.detail.trimEnd(); - if (!parts.remedy) { - return trimmedDetail; - } - return `${trimmedDetail}${API_PROBLEM_REMEDY_DELIMITER}${parts.remedy}`; -} - -export function parseApiProblemMessage(message: string): ApiProblemParts { - const delimiterIndex = message.indexOf(API_PROBLEM_REMEDY_DELIMITER); - if (delimiterIndex !== -1) { - return { - detail: message.slice(0, delimiterIndex), - remedy: message.slice( - delimiterIndex + API_PROBLEM_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 }; -} diff --git a/src/utils/handleServerError.ts b/src/utils/handleServerError.ts index fd2a71c1d..f378f80d5 100644 --- a/src/utils/handleServerError.ts +++ b/src/utils/handleServerError.ts @@ -1,5 +1,7 @@ import { ResponseError } from "@generated/api/src"; -import { getApiProblemParts, serializeApiProblem } from "src/utils/apiProblem"; + +/** 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 @@ -38,9 +40,9 @@ export const handleServerError = async ( } } - const combinedErrorMessage = serializeApiProblem( - getApiProblemParts(errorBody), - ); + 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 From 38285d86c50d54b85d81ff2519abc9d6cf408b9b Mon Sep 17 00:00:00 2001 From: Dion Low Date: Thu, 18 Jun 2026 14:28:37 -0700 Subject: [PATCH 3/3] test: add handleServerError unit tests for remedy serialization Configure Jest moduleNameMapper for path aliases. Tests cover detail-only, causes, and detail+remedy delimiter format matching the minimal fix. Co-authored-by: Cursor --- package.json | 14 ++- test/utils/handleServerError.test.ts | 124 +++++++++++++++++++++++++++ 2 files changed, 136 insertions(+), 2 deletions(-) create mode 100644 test/utils/handleServerError.test.ts diff --git a/package.json b/package.json index 473972838..c6069d83e 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/test/utils/handleServerError.test.ts b/test/utils/handleServerError.test.ts new file mode 100644 index 000000000..76f254626 --- /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", + ); + }); + }); +});