Skip to content

fix: retry GitHub REST API calls when HTML is returned instead of JSON #22800

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 3 commits into from
Mar 25, 2026
Merged

fix: retry GitHub REST API calls when HTML is returned instead of JSON #22800

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
62dc9f8
Initial plan
Copilot Mar 25, 2026
1a748b6
fix: retry GitHub REST API calls when HTML is returned instead of JSON
Copilot Mar 25, 2026
7426e68
refactor: optimize isTransientError to avoid redundant string processing
Copilot Mar 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions actions/setup/js/error_recovery.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,15 @@ const DEFAULT_RETRY_CONFIG = {
* @returns {boolean} True if the error is transient and should be retried
*/
function isTransientError(error) {
const errorMsg = getErrorMessage(error).toLowerCase();
const errorMsg = getErrorMessage(error);
const errorMsgLower = errorMsg.trimStart().toLowerCase();

// 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("<!doctype html")) {
return true;
}

// Network-related errors that are likely transient
const transientPatterns = [
Expand All @@ -58,7 +66,7 @@ function isTransientError(error) {
"no server is currently available", // GitHub API server unavailability
];

return transientPatterns.some(pattern => errorMsg.includes(pattern));
return transientPatterns.some(pattern => errorMsgLower.includes(pattern));
}

/**
Expand Down
7 changes: 7 additions & 0 deletions actions/setup/js/error_recovery.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,13 @@ describe("error_recovery", () => {
expect(isTransientError(new Error("no server is currently available"))).toBe(true);
});

it("should identify HTML responses (GitHub 500 Unicorn page) as transient", () => {
expect(isTransientError(new Error("<!DOCTYPE html><html><head><title>Unicorn!</title></head></html>"))).toBe(true);
expect(isTransientError(new Error("<!doctype html>\n<html>..."))).toBe(true);
// With leading whitespace
expect(isTransientError(new Error(" <!DOCTYPE html><html>..."))).toBe(true);
});

it("should not identify validation errors as transient", () => {
expect(isTransientError(new Error("Invalid input"))).toBe(false);
expect(isTransientError(new Error("Field is required"))).toBe(false);
Expand Down
4 changes: 3 additions & 1 deletion actions/setup/js/update_handler_factory.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const { logStagedPreviewInfo } = require("./staged_preview.cjs");
const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs");
const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs");
const { sanitizeContent } = require("./sanitize_content.cjs");
const { withRetry, isTransientError } = require("./error_recovery.cjs");

/**
* @typedef {Object} UpdateHandlerConfig
Expand Down Expand Up @@ -253,8 +254,9 @@ function createUpdateHandlerFactory(handlerConfig) {
// Execute the update using the authenticated client and effective context.
// githubClient uses config["github-token"] when set (for cross-repo), otherwise global github.
// effectiveContext.repo contains the target repo owner/name for cross-repo routing.
// Retry on transient errors (e.g. GitHub API returning HTML instead of JSON on 500 crashes).
try {
const updatedItem = await executeUpdate(githubClient, effectiveContext, itemNumber, updateData);
const updatedItem = await withRetry(() => executeUpdate(githubClient, effectiveContext, itemNumber, updateData), { maxRetries: 1, initialDelayMs: 2000, shouldRetry: isTransientError }, `update ${itemTypeName} #${itemNumber}`);
core.info(`Successfully updated ${itemTypeName} #${itemNumber}: ${updatedItem.html_url || updatedItem.url}`);
Comment on lines +257 to 260
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change introduces new behavior (retrying executeUpdate on transient errors), but update_handler_factory.test.cjs doesn't currently assert that retries happen (e.g., executeUpdate called twice when the first attempt throws a transient error). Adding a test for the retry path would help prevent regressions and confirm the correct retry config (attempt count + delay/backoff).

This issue also appears in the following locations of the same file:

  • line 257
  • line 259

Copilot uses AI. Check for mistakes.

// Format and return success result
Expand Down
2 changes: 1 addition & 1 deletion actions/setup/js/update_handler_factory.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ describe("update_handler_factory.cjs", () => {
const result = await handler({ title: "Test" });

expect(result.success).toBe(false);
expect(result.error).toBe("API Error");
expect(result.error).toContain("API Error");
expect(mockCore.error).toHaveBeenCalledWith(expect.stringContaining("Failed to update test item"));
});

Expand Down
Loading