From c4e1ec88f417d05b7bc765bdcf00699663c2ef03 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 16 Jun 2026 18:50:03 +0000 Subject: [PATCH 1/7] fix: sanitize HTML error pages in getErrorMessage to avoid noisy logs 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: ... Now shows: Review finalization failed: GitHub returned an unexpected HTML response (HTTP 504) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/error_helpers.cjs | 34 ++++++++++++-- actions/setup/js/error_helpers.test.cjs | 62 ++++++++++++++++++++++++- 2 files changed, 90 insertions(+), 6 deletions(-) diff --git a/actions/setup/js/error_helpers.cjs b/actions/setup/js/error_helpers.cjs index aaba9e84839..467da5df2b5 100644 --- 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*]/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 index 7aa5357215c..87ce0fdda50 100644 --- 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 = "\nUnicorn!..."; + 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 = "\nUnicorn!..."; + const error = new Error(html); + expect(getErrorMessage(error)).toBe("GitHub returned an unexpected HTML response"); + }); + + it("should sanitize bare error response with status", () => { + const html = "Service Unavailable..."; + 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..."; + 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("")).toBe(true); + }); + + it("should return true for bare html tag", () => { + expect(isHtmlContent("")).toBe(true); + }); + + it("should return true with leading whitespace", () => { + expect(isHtmlContent("\n ...")).toBe(true); + }); + + it("should return true for case-insensitive DOCTYPE", () => { + expect(isHtmlContent("")).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", () => { From 48d518bfd34058d8ad991b6278fb11c0983e661d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 16 Jun 2026 20:47:09 +0000 Subject: [PATCH 2/7] fix: address HTML sanitization review and transient retry regression Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/error_helpers.cjs | 5 +++-- actions/setup/js/error_helpers.test.cjs | 7 ++++++- actions/setup/js/error_recovery.cjs | 2 +- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/actions/setup/js/error_helpers.cjs b/actions/setup/js/error_helpers.cjs index 467da5df2b5..92c72a5c836 100644 --- a/actions/setup/js/error_helpers.cjs +++ b/actions/setup/js/error_helpers.cjs @@ -24,17 +24,18 @@ function isHtmlContent(str) { * @returns {string} The error message as a string */ function getErrorMessage(error) { + const err = /** @type {any} */ error; let message; if (error instanceof Error) { message = error.message; } else if (error && typeof error === "object" && "message" in error && typeof error.message === "string") { message = error.message; } else { - return String(error); + message = String(error); } if (isHtmlContent(message)) { - const status = error && typeof error === "object" && "status" in error && typeof (/** @type {any} */ error.status) === "number" ? /** @type {any} */ error.status : null; + const status = err && typeof err === "object" && typeof err.status === "number" ? err.status : null; return status != null ? `GitHub returned an unexpected HTML response (HTTP ${status})` : "GitHub returned an unexpected HTML response"; } diff --git a/actions/setup/js/error_helpers.test.cjs b/actions/setup/js/error_helpers.test.cjs index 87ce0fdda50..4e31f7ff3a7 100644 --- a/actions/setup/js/error_helpers.test.cjs +++ b/actions/setup/js/error_helpers.test.cjs @@ -64,6 +64,11 @@ describe("error_helpers", () => { expect(getErrorMessage(error)).toBe("GitHub returned an unexpected HTML response"); }); + it("should sanitize raw HTML string throw", () => { + const html = "Unicorn"; + expect(getErrorMessage(html)).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"); @@ -92,7 +97,7 @@ describe("error_helpers", () => { }); it("should return false for JSON-like content", () => { - expect(isHtmlContent('{"message":"Not Found","documentation_url":"..."}')); + expect(isHtmlContent('{"message":"Not Found","documentation_url":"..."}')).toBe(false); }); it("should return false for empty string", () => { diff --git a/actions/setup/js/error_recovery.cjs b/actions/setup/js/error_recovery.cjs index 96a5ab9edd2..fb32048be84 100644 --- a/actions/setup/js/error_recovery.cjs +++ b/actions/setup/js/error_recovery.cjs @@ -65,7 +65,7 @@ function isTransientError(error) { // GitHub REST APIs may crash and return an HTML error page (e.g. the "Unicorn!" // 500 page) instead of JSON. Detect this by checking for an HTML doctype at the // start of the error message and treat it as a transient server error. - if (errorMsgLower.startsWith(" Date: Tue, 16 Jun 2026 20:55:12 +0000 Subject: [PATCH 3/7] fix: satisfy js-typecheck for HTML status sanitization Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/error_helpers.cjs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/actions/setup/js/error_helpers.cjs b/actions/setup/js/error_helpers.cjs index 92c72a5c836..c51d009805c 100644 --- a/actions/setup/js/error_helpers.cjs +++ b/actions/setup/js/error_helpers.cjs @@ -24,7 +24,7 @@ function isHtmlContent(str) { * @returns {string} The error message as a string */ function getErrorMessage(error) { - const err = /** @type {any} */ error; + const err = /** @type {{ status?: unknown } | null | undefined} */ error; let message; if (error instanceof Error) { message = error.message; @@ -35,7 +35,7 @@ function getErrorMessage(error) { } if (isHtmlContent(message)) { - const status = err && typeof err === "object" && typeof err.status === "number" ? err.status : null; + const status = err != null && typeof err.status === "number" ? err.status : null; return status != null ? `GitHub returned an unexpected HTML response (HTTP ${status})` : "GitHub returned an unexpected HTML response"; } From 026c8177d4c3f65147a3774af8fba996adf03d87 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 16 Jun 2026 21:00:32 +0000 Subject: [PATCH 4/7] refactor: clarify status extraction variable naming Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/error_helpers.cjs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/actions/setup/js/error_helpers.cjs b/actions/setup/js/error_helpers.cjs index c51d009805c..46512f41b58 100644 --- a/actions/setup/js/error_helpers.cjs +++ b/actions/setup/js/error_helpers.cjs @@ -24,7 +24,7 @@ function isHtmlContent(str) { * @returns {string} The error message as a string */ function getErrorMessage(error) { - const err = /** @type {{ status?: unknown } | null | undefined} */ error; + const errorWithStatus = /** @type {{ status?: unknown } | null | undefined} */ error; let message; if (error instanceof Error) { message = error.message; @@ -35,7 +35,7 @@ function getErrorMessage(error) { } if (isHtmlContent(message)) { - const status = err != null && typeof err.status === "number" ? err.status : null; + const status = errorWithStatus != null && typeof errorWithStatus.status === "number" ? errorWithStatus.status : null; return status != null ? `GitHub returned an unexpected HTML response (HTTP ${status})` : "GitHub returned an unexpected HTML response"; } From 8522332189c5a415c523a98e819acbef27f689f8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 16 Jun 2026 21:14:57 +0000 Subject: [PATCH 5/7] fix: use @type {any} cast to resolve TypeScript typecheck error for status access Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/error_helpers.cjs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/actions/setup/js/error_helpers.cjs b/actions/setup/js/error_helpers.cjs index 46512f41b58..12ec50156ae 100644 --- a/actions/setup/js/error_helpers.cjs +++ b/actions/setup/js/error_helpers.cjs @@ -24,7 +24,7 @@ function isHtmlContent(str) { * @returns {string} The error message as a string */ function getErrorMessage(error) { - const errorWithStatus = /** @type {{ status?: unknown } | null | undefined} */ error; + const errorAsAny = /** @type {any} */ error; let message; if (error instanceof Error) { message = error.message; @@ -35,7 +35,7 @@ function getErrorMessage(error) { } if (isHtmlContent(message)) { - const status = errorWithStatus != null && typeof errorWithStatus.status === "number" ? errorWithStatus.status : null; + const status = errorAsAny != null && typeof errorAsAny.status === "number" ? errorAsAny.status : null; return status != null ? `GitHub returned an unexpected HTML response (HTTP ${status})` : "GitHub returned an unexpected HTML response"; } From 4a82c821baa3fc4adc5f60b51a894c360cf6039b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 16 Jun 2026 21:50:53 +0000 Subject: [PATCH 6/7] fix: wrap error cast in parentheses for TypeScript compatibility Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/error_helpers.cjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actions/setup/js/error_helpers.cjs b/actions/setup/js/error_helpers.cjs index 12ec50156ae..d7603815046 100644 --- a/actions/setup/js/error_helpers.cjs +++ b/actions/setup/js/error_helpers.cjs @@ -24,7 +24,7 @@ function isHtmlContent(str) { * @returns {string} The error message as a string */ function getErrorMessage(error) { - const errorAsAny = /** @type {any} */ error; + const errorAsAny = /** @type {any} */ (error); let message; if (error instanceof Error) { message = error.message; From f058e44eb77872b670e3813fb4adb3b86be1d0be Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 16 Jun 2026 22:52:35 +0000 Subject: [PATCH 7/7] fix: add prettier-ignore to preserve JSDoc cast parentheses Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/error_helpers.cjs | 1 + 1 file changed, 1 insertion(+) diff --git a/actions/setup/js/error_helpers.cjs b/actions/setup/js/error_helpers.cjs index d7603815046..692bea862ef 100644 --- a/actions/setup/js/error_helpers.cjs +++ b/actions/setup/js/error_helpers.cjs @@ -24,6 +24,7 @@ function isHtmlContent(str) { * @returns {string} The error message as a string */ function getErrorMessage(error) { + // prettier-ignore const errorAsAny = /** @type {any} */ (error); let message; if (error instanceof Error) {