fix: sanitize HTML error pages in getErrorMessage to avoid noisy logs

Copilot · pelikhan · web-flow · commit c4e1ec88f417 · 2026-06-16T18:50:03.000Z
When GitHub returns a 504/5xx response with an HTML body (e.g. the
"Unicorn" error page), getErrorMessage was returning the full raw HTML
as the error message. This caused CI logs to be flooded with hundreds
of lines of HTML markup.

Add isHtmlContent() to detect DOCTYPE/html responses and modify
getErrorMessage() to replace them with a concise human-readable
message that includes the HTTP status code when available.

Fixes: Review finalization failed: &lt;!DOCTYPE html&gt;...
Now shows: Review finalization failed: GitHub returned an unexpected HTML response (HTTP 504)

Co-authored-by: pelikhan &lt;4175913+pelikhan@users.noreply.github.com&gt;
diff --git a/actions/setup/js/error_helpers.cjs b/actions/setup/js/error_helpers.cjs
@@ -1,20 +1,44 @@
 // @ts-check
 
+/**
+ * Detect whether a string looks like an HTML page rather than a plain error message.
+ * Used to sanitize GitHub's "Unicorn" / gateway error HTML responses so they don't
+ * pollute CI logs with hundreds of lines of markup.
+ *
+ * @param {string} str - The string to inspect
+ * @returns {boolean} True when the string appears to be an HTML document
+ */
+function isHtmlContent(str) {
+  return /^\s*<!DOCTYPE\s/i.test(str) || /^\s*<html[\s>]/i.test(str);
+}
+
 /**
  * Safely extract an error message from an unknown error value.
  * Handles Error instances, objects with message properties, and other values.
  *
+ * When the extracted message looks like an HTML page (e.g. GitHub's "Unicorn"
+ * 504 error page), it is replaced with a concise human-readable description so
+ * that CI logs stay readable.  The HTTP status code is included when available.
+ *
  * @param {unknown} error - The error value to extract a message from
  * @returns {string} The error message as a string
  */
 function getErrorMessage(error) {
+  let message;
   if (error instanceof Error) {
-    return error.message;
+    message = error.message;
+  } else if (error && typeof error === "object" && "message" in error && typeof error.message === "string") {
+    message = error.message;
+  } else {
+    return String(error);
   }
-  if (error && typeof error === "object" && "message" in error && typeof error.message === "string") {
-    return error.message;
+
+  if (isHtmlContent(message)) {
+    const status = error && typeof error === "object" && "status" in error && typeof (/** @type {any} */ error.status) === "number" ? /** @type {any} */ error.status : null;
+    return status != null ? `GitHub returned an unexpected HTML response (HTTP ${status})` : "GitHub returned an unexpected HTML response";
   }
-  return String(error);
+
+  return message;
 }
 
 /**
@@ -54,4 +78,4 @@ function isRateLimitError(error) {
   return /\bapi rate limit\b|\brate limit exceeded\b/i.test(errorMessage);
 }
 
-module.exports = { getErrorMessage, isLockedError, isRateLimitError };
+module.exports = { getErrorMessage, isHtmlContent, isLockedError, isRateLimitError };
diff --git a/actions/setup/js/error_helpers.test.cjs b/actions/setup/js/error_helpers.test.cjs
@@ -1,5 +1,5 @@
 import { describe, it, expect } from "vitest";
-import { getErrorMessage, isLockedError, isRateLimitError } from "./error_helpers.cjs";
+import { getErrorMessage, isHtmlContent, isLockedError, isRateLimitError } from "./error_helpers.cjs";
 
 describe("error_helpers", () => {
   describe("getErrorMessage", () => {
@@ -38,6 +38,66 @@ describe("error_helpers", () => {
       const error = { code: "ERROR_CODE", status: 500 };
       expect(getErrorMessage(error)).toBe("[object Object]");
     });
+
+    it("should sanitize HTML DOCTYPE error response with status", () => {
+      const html = "<!DOCTYPE html>\n<html><head><title>Unicorn!</title></head><body>...</body></html>";
+      const error = new Error(html);
+      /** @type {any} */ error.status = 504;
+      expect(getErrorMessage(error)).toBe("GitHub returned an unexpected HTML response (HTTP 504)");
+    });
+
+    it("should sanitize HTML DOCTYPE error response without status", () => {
+      const html = "<!DOCTYPE html>\n<html><head><title>Unicorn!</title></head><body>...</body></html>";
+      const error = new Error(html);
+      expect(getErrorMessage(error)).toBe("GitHub returned an unexpected HTML response");
+    });
+
+    it("should sanitize bare <html> error response with status", () => {
+      const html = "<html><head><title>Service Unavailable</title></head><body>...</body></html>";
+      const error = { message: html, status: 503 };
+      expect(getErrorMessage(error)).toBe("GitHub returned an unexpected HTML response (HTTP 503)");
+    });
+
+    it("should sanitize html with leading whitespace", () => {
+      const html = "  \n<!DOCTYPE html><html>...</html>";
+      const error = new Error(html);
+      expect(getErrorMessage(error)).toBe("GitHub returned an unexpected HTML response");
+    });
+
+    it("should not sanitize plain-text error messages that happen to mention html", () => {
+      const error = new Error("Validation failed: invalid html content provided");
+      expect(getErrorMessage(error)).toBe("Validation failed: invalid html content provided");
+    });
+  });
+
+  describe("isHtmlContent", () => {
+    it("should return true for DOCTYPE HTML string", () => {
+      expect(isHtmlContent("<!DOCTYPE html><html></html>")).toBe(true);
+    });
+
+    it("should return true for bare html tag", () => {
+      expect(isHtmlContent("<html><head></head><body></body></html>")).toBe(true);
+    });
+
+    it("should return true with leading whitespace", () => {
+      expect(isHtmlContent("\n  <!DOCTYPE html>...")).toBe(true);
+    });
+
+    it("should return true for case-insensitive DOCTYPE", () => {
+      expect(isHtmlContent("<!doctype HTML><html></html>")).toBe(true);
+    });
+
+    it("should return false for plain text", () => {
+      expect(isHtmlContent("Resource not accessible by integration")).toBe(false);
+    });
+
+    it("should return false for JSON-like content", () => {
+      expect(isHtmlContent('{"message":"Not Found","documentation_url":"..."}'));
+    });
+
+    it("should return false for empty string", () => {
+      expect(isHtmlContent("")).toBe(false);
+    });
   });
 
   describe("isLockedError", () => {
