test: add handleServerError unit tests for remedy serialization

package.json

@@ -50,8 +50,18 @@
   },
   "jest": {
     "testPathIgnorePatterns": [
-      "build"
-    ]
+      "build",
+      ".claude/worktrees"
+    ],
+    "moduleNameMapper": {
+      "^@generated/(.*)$": "<rootDir>/generated-sources/$1",
+      "^src/(.*)$": "<rootDir>/src/$1",
+      "^components/(.*)$": "<rootDir>/src/components/$1",
+      "^assets/(.*)$": "<rootDir>/src/assets/$1",
+      "^context/(.*)$": "<rootDir>/src/context/$1",
+      "^hooks/(.*)$": "<rootDir>/src/hooks/$1",
+      "^services/(.*)$": "<rootDir>/src/services/$1"
+    }
   },
   "devDependencies": {
     "@babel/preset-env": "^7.23.2",

src/components/Configure/content/ConfigureInstallationBase.tsx

@@ -192,7 +192,7 @@ export function ConfigureInstallationBase({
         }}
       >
         {errorMsg && (
-          <FormErrorBox>
+          <FormErrorBox style={{ marginBottom: "1rem" }}>
             {typeof errorMsg === "string" ? errorMsg : "Installation Failed."}
           </FormErrorBox>
         )}

src/components/Configure/content/fields/ReadFields.tsx

@@ -11,7 +11,13 @@ export function ReadFields() {
   useClearOldFieldMappings();
 
   return (
-    <>
+    <div
+      style={{
+        display: "flex",
+        flexDirection: "column",
+        gap: "1rem",
+      }}
+    >
       <ReEnableReadObject />
       <ReadObjectMapping />
       <RequiredFields />
@@ -20,6 +26,6 @@ export function ReadFields() {
       <ValueMappings />
       <OptionalFields />
       <DisableReadObject />
-    </>
+    </div>
   );
 }

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 (
+    <>
+      <p style={{ margin: 0, lineHeight: 1.5, whiteSpace: "pre-line" }}>
+        {detail}
+      </p>
+      {remedy && (
+        <div
+          style={{
+            marginTop: "0.75rem",
+            paddingTop: "0.75rem",
+            borderTop:
+              "1px solid var(--amp-colors-status-critical-dark, rgba(0, 0, 0, 0.15))",
+          }}
+        >
+          <span
+            style={{
+              display: "block",
+              fontSize: "0.75rem",
+              fontWeight: 600,
+              textTransform: "uppercase",
+              letterSpacing: "0.025em",
+              marginBottom: "0.25rem",
+              opacity: 0.85,
+            }}
+          >
+            How to fix
+          </span>
+          <p style={{ margin: 0, lineHeight: 1.5, whiteSpace: "pre-line" }}>
+            {remedy}
+          </p>
+        </div>
+      )}
+    </>
+  );
+}
+
 type FormErrorBoxProps = {
   children: React.ReactNode;
   style?: React.CSSProperties;
 };
 
 export function FormErrorBox({ children, style }: FormErrorBoxProps) {
-  return <Box style={{ ...defaultStyle, ...style }}>{children}</Box>;
+  return (
+    <Box style={{ ...defaultStyle, ...style }}>
+      {typeof children === "string" ? (
+        <ErrorMessageContent message={children} />
+      ) : (
+        children
+      )}
+    </Box>
+  );
 }

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

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",
+      );
+    });
+  });
+});