fix(testmanagement): return the upstream message from updateTestCase failures

src/lib/error.ts

@@ -1,6 +1,70 @@
 import { AxiosError } from "axios";
 import { CallToolResult } from "@modelcontextprotocol/sdk/types.js";
 
+const MAX_ERROR_TEXT = 500;
+const BODY_FIELDS = ["message", "error", "errors"] as const;
+
+function clip(text: string): string {
+  return text.length > MAX_ERROR_TEXT ? text.slice(0, MAX_ERROR_TEXT) : text;
+}
+
+const HTML_DOCUMENT = /^\s*<(!doctype|html)\b/i;
+
+function stringify(value: unknown): string {
+  if (typeof value === "string") {
+    return HTML_DOCUMENT.test(value) ? "" : value.trim();
+  }
+  if (value === undefined || value === null) return "";
+  try {
+    const json = JSON.stringify(value);
+    return json === "{}" || json === "[]" ? "" : (json ?? "");
+  } catch {
+    return "";
+  }
+}
+
+/**
+ * Text taken from an API error body: the first non-empty `message`, `error`
+ * or `errors` field when any of them is present, otherwise the whole body.
+ * Returns undefined when the body carries no usable text.
+ */
+function bodyText(data: unknown): string | undefined {
+  if (typeof data !== "object" || data === null) {
+    const text = stringify(data);
+    return text ? clip(text) : undefined;
+  }
+  const record = data as Record<string, unknown>;
+  const named = BODY_FIELDS.filter((key) => key in record);
+  if (named.length > 0) {
+    for (const key of named) {
+      const text = stringify(record[key]);
+      if (text) return clip(text);
+    }
+    return undefined;
+  }
+  const text = stringify(record);
+  return text ? clip(text) : undefined;
+}
+
+/**
+ * Returns the most specific human-readable text for an error raised by an
+ * API call, capped at 500 characters. For an AxiosError with a response body
+ * it prefers the body's `message`, `error` or `errors` field, serialising
+ * objects, and uses the whole body when none of those fields exist. An empty,
+ * whitespace-only or HTML-document body falls back to the error's own message.
+ * @param err - The error object to describe
+ * @param defaultText - The fallback error message
+ */
+export function errorText(err: unknown, defaultText: string): string {
+  if (err instanceof AxiosError) {
+    const text = bodyText(err.response?.data);
+    if (text) return text;
+    return clip(err.message || defaultText);
+  }
+  if (err instanceof Error) return clip(err.message || defaultText);
+  return defaultText;
+}
+
 /**
  * Formats an AxiosError into a CallToolResult with an appropriate message.
  * @param err - The error object to format
@@ -10,21 +74,8 @@ export function formatAxiosError(
   err: unknown,
   defaultText: string,
 ): CallToolResult {
-  let text = defaultText;
-
-  if (err instanceof AxiosError && err.response?.data) {
-    const message =
-      err.response.data.message ||
-      err.response.data.error ||
-      err.message ||
-      defaultText;
-    text = message;
-  } else if (err instanceof Error) {
-    text = err.message;
-  }
-
   return {
-    content: [{ type: "text", text }],
+    content: [{ type: "text", text: errorText(err, defaultText) }],
     isError: true,
   };
 }

src/tools/testmanagement-utils/update-testcase.ts

@@ -1,7 +1,7 @@
 import { apiClient } from "../../lib/apiClient.js";
 import { z } from "zod";
 import { CallToolResult } from "@modelcontextprotocol/sdk/types.js";
-import { formatAxiosError } from "../../lib/error.js";
+import { errorText } from "../../lib/error.js";
 import {
   fetchFormFields,
   normalizeDefaultFields as normalizeDefaultFieldsFromForm,
@@ -306,12 +306,15 @@ Error details: ${JSON.stringify(err.response?.data || err.message)}`,
       };
     }
 
-    const errorMessage = formatAxiosError(err, "Failed to update test case");
+    const status = err.response?.status;
+    const detail = errorText(err, "unknown error");
+    const hint =
+      status === 401 ? " Check the BrowserStack credentials in use." : "";
     return {
       content: [
         {
           type: "text",
-          text: `Failed to update test case: ${errorMessage}. Please verify your credentials and try again.`,
+          text: `Failed to update test case${status ? ` (HTTP ${status})` : ""}: ${detail}.${hint}`,
         },
       ],
       isError: true,

tests/lib/error.test.ts

@@ -0,0 +1,109 @@
+import { describe, expect, it } from "vitest";
+import { AxiosError } from "axios";
+import { errorText, formatAxiosError } from "../../src/lib/error";
+
+const axiosError = (
+  status: number,
+  data: unknown,
+  message = `Request failed with status code ${status}`,
+) =>
+  Object.assign(Object.create(AxiosError.prototype), {
+    message,
+    response: { status, data },
+  });
+
+describe("errorText", () => {
+  it("prefers message, then error, then errors", () => {
+    expect(errorText(axiosError(422, { message: "m", error: "e" }), "d")).toBe(
+      "m",
+    );
+    expect(errorText(axiosError(422, { error: "e", errors: ["x"] }), "d")).toBe(
+      "e",
+    );
+    expect(errorText(axiosError(422, { errors: ["x"] }), "d")).toBe('["x"]');
+  });
+
+  it("serialises object fields instead of printing [object Object]", () => {
+    expect(errorText(axiosError(400, { message: { nested: 1 } }), "d")).toBe(
+      '{"nested":1}',
+    );
+    expect(
+      errorText(axiosError(400, { errors: { name: ["is too long"] } }), "d"),
+    ).toBe('{"name":["is too long"]}');
+  });
+
+  it("uses a plain string body", () => {
+    expect(errorText(axiosError(401, "Unauthorized"), "d")).toBe(
+      "Unauthorized",
+    );
+  });
+
+  it("uses the whole body when none of the known fields exist", () => {
+    expect(
+      errorText(axiosError(400, { detail: { hint: "try low" } }), "d"),
+    ).toBe('{"detail":{"hint":"try low"}}');
+  });
+
+  it("falls back to the error message on an empty or whitespace body", () => {
+    expect(errorText(axiosError(500, ""), "d")).toBe(
+      "Request failed with status code 500",
+    );
+    expect(errorText(axiosError(500, "   \n"), "d")).toBe(
+      "Request failed with status code 500",
+    );
+    expect(errorText(axiosError(500, {}), "d")).toBe(
+      "Request failed with status code 500",
+    );
+    expect(errorText(axiosError(500, { message: "  " }), "d")).toBe(
+      "Request failed with status code 500",
+    );
+    expect(errorText(axiosError(500, undefined), "d")).toBe(
+      "Request failed with status code 500",
+    );
+  });
+
+  it("falls back to the default text when the error has no message either", () => {
+    expect(errorText(axiosError(502, "", ""), "Failed to list")).toBe(
+      "Failed to list",
+    );
+  });
+
+  it("ignores an HTML error page and reports the status instead", () => {
+    const page =
+      "<html><head><title>504 Gateway Time-out</title></head><body>nginx</body></html>";
+    expect(errorText(axiosError(504, page), "d")).toBe(
+      "Request failed with status code 504",
+    );
+    expect(
+      errorText(axiosError(502, "<!DOCTYPE html><html></html>"), "d"),
+    ).toBe("Request failed with status code 502");
+  });
+
+  it("caps the surfaced text at 500 characters", () => {
+    expect(errorText(axiosError(504, "x".repeat(10000)), "d")).toHaveLength(
+      500,
+    );
+    const big = {
+      errors: Array.from({ length: 500 }, (_, i) => `field ${i} is invalid`),
+    };
+    expect(errorText(axiosError(422, big), "d")).toHaveLength(500);
+  });
+
+  it("handles non-axios errors and non-errors", () => {
+    expect(errorText(new Error("read ECONNRESET"), "d")).toBe(
+      "read ECONNRESET",
+    );
+    expect(errorText(new Error(""), "d")).toBe("d");
+    expect(errorText("boom", "d")).toBe("d");
+    expect(errorText(undefined, "d")).toBe("d");
+  });
+});
+
+describe("formatAxiosError", () => {
+  it("wraps errorText in an error result", () => {
+    expect(formatAxiosError(axiosError(422, { error: "bad" }), "d")).toEqual({
+      content: [{ type: "text", text: "bad" }],
+      isError: true,
+    });
+  });
+});

tests/tools/testmanagement.test.ts

@@ -1604,6 +1604,90 @@ describe('updateTestCase — multi-select custom_fields', () => {
   });
 });
 
+// Error path of updateTestCase: the upstream message must reach the caller as text.
+describe('updateTestCase — upstream error messages', () => {
+  let updateTestCaseReal: typeof import('../../src/tools/testmanagement-utils/update-testcase').updateTestCase;
+  let apiClientMock: typeof import('../../src/lib/apiClient').apiClient;
+  let AxiosErrorCtor: typeof import('axios').AxiosError;
+
+  const axiosError = (status: number, data: unknown) =>
+    Object.assign(Object.create(AxiosErrorCtor.prototype), {
+      message: `Request failed with status code ${status}`,
+      response: { status, data },
+    });
+
+  beforeAll(async () => {
+    const actual = await vi.importActual<
+      typeof import('../../src/tools/testmanagement-utils/update-testcase')
+    >('../../src/tools/testmanagement-utils/update-testcase');
+    updateTestCaseReal = actual.updateTestCase;
+    apiClientMock = (await import('../../src/lib/apiClient')).apiClient;
+    AxiosErrorCtor = (await import('axios')).AxiosError;
+  });
+
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });
+
+  const args = { project_identifier: 'PR-1', test_case_identifier: 'TC-1', description: 'd' };
+
+  it('surfaces a string error from the response body with the HTTP status', async () => {
+    (apiClientMock.patch as Mock).mockRejectedValueOnce(
+      axiosError(422, { success: false, error: 'Priority is not valid for this project' }),
+    );
+
+    const result = await updateTestCaseReal(args, mockConfig as any);
+
+    expect(result.isError).toBe(true);
+    const text = (result.content[0] as any).text as string;
+    expect(text).toBe('Failed to update test case (HTTP 422): Priority is not valid for this project.');
+    expect(text).not.toContain('[object Object]');
+  });
+
+  it('serialises an object error body instead of printing [object Object]', async () => {
+    (apiClientMock.patch as Mock).mockRejectedValueOnce(
+      axiosError(400, { errors: { name: ['is too long'] } }),
+    );
+
+    const result = await updateTestCaseReal(args, mockConfig as any);
+
+    const text = (result.content[0] as any).text as string;
+    expect(text).toContain('(HTTP 400)');
+    expect(text).toContain('{"name":["is too long"]}');
+    expect(text).not.toContain('[object Object]');
+  });
+
+  it('returns the whole body when it has no message, error or errors field', async () => {
+    (apiClientMock.patch as Mock).mockRejectedValueOnce(
+      axiosError(400, { detail: { hint: 'priority must be one of: low, medium, high' } }),
+    );
+
+    const result = await updateTestCaseReal(args, mockConfig as any);
+
+    const text = (result.content[0] as any).text as string;
+    expect(text).toBe(
+      'Failed to update test case (HTTP 400): {"detail":{"hint":"priority must be one of: low, medium, high"}}.',
+    );
+  });
+
+  it('adds the credentials hint only on 401', async () => {
+    (apiClientMock.patch as Mock).mockRejectedValueOnce(axiosError(401, 'Unauthorized'));
+
+    const result = await updateTestCaseReal(args, mockConfig as any);
+
+    const text = (result.content[0] as any).text as string;
+    expect(text).toBe('Failed to update test case (HTTP 401): Unauthorized. Check the BrowserStack credentials in use.');
+  });
+
+  it('falls back to the error message when there is no response', async () => {
+    (apiClientMock.patch as Mock).mockRejectedValueOnce(new Error('read ECONNRESET'));
+
+    const result = await updateTestCaseReal(args, mockConfig as any);
+
+    expect((result.content[0] as any).text).toBe('Failed to update test case: read ECONNRESET.');
+  });
+});
+
 // listTestCaseTemplates resolves a template name to a template_id.
 describe('listTemplates', () => {
   let listTemplatesReal: typeof import('../../src/tools/testmanagement-utils/list-templates').listTemplates;