Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions actions/setup/js/check_workflow_timestamp_api.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const path = require("path");
const { getErrorMessage } = require("./error_helpers.cjs");
const { extractHashFromLockFile, extractBodyHashFromLockFile, computeFrontmatterHash, computeBodyHash, createGitHubFileReader } = require("./frontmatter_hash_pure.cjs");
const { getFileContent } = require("./github_api_helpers.cjs");
const { ERR_CONFIG } = require("./error_codes.cjs");
const { ERR_CONFIG, SAFE_OUTPUT_E009, CONFIG_HASH_MISMATCH } = require("./error_codes.cjs");

// Matches GitHub workflow ref paths of the form "owner/repo/...[@ref]"
// and captures: [1] owner, [2] repo, [3] optional ref
Expand Down Expand Up @@ -423,7 +423,7 @@ async function main() {
await summary.write();

core.setOutput("stale_lock_file_failed", "true");
core.setFailed(`${ERR_CONFIG}: ${warningMessage}`);
core.setFailed(`${ERR_CONFIG}: ${SAFE_OUTPUT_E009} ${CONFIG_HASH_MISMATCH}: ${warningMessage}`);
} else if (hashComparison.match) {
// Hashes match - lock file is up to date
core.info("✅ Lock file is up to date (hashes match)");
Expand All @@ -450,7 +450,7 @@ async function main() {
core.setOutput("stale_lock_file_failed", "true");

// Fail the step to prevent workflow from running with outdated configuration
core.setFailed(`${ERR_CONFIG}: ${warningMessage}`);
core.setFailed(`${ERR_CONFIG}: ${SAFE_OUTPUT_E009} ${CONFIG_HASH_MISMATCH}: ${warningMessage}`);
}
}

Expand Down
4 changes: 4 additions & 0 deletions actions/setup/js/check_workflow_timestamp_api.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,8 @@ model: claude-sonnet-4
await main();

expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("Lock file"));
expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("E009"));
expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("CONFIG_HASH_MISMATCH"));
expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("is outdated"));
expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("frontmatter has changed"));
expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("gh aw compile"));
Expand Down Expand Up @@ -241,6 +243,8 @@ jobs:
await main();

expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("Could not compare frontmatter hashes"));
expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("E009"));
expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("CONFIG_HASH_MISMATCH"));
expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("is outdated"));
expect(mockCore.summary.addRaw).toHaveBeenCalled();
expect(mockCore.summary.write).toHaveBeenCalled();
Expand Down
16 changes: 16 additions & 0 deletions actions/setup/js/error_codes.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,21 @@ const ERR_SYSTEM = "ERR_SYSTEM";
/** @type {string} Safe output validation/input errors (legacy numeric taxonomy) */
const SAFE_OUTPUT_E001 = "E001";

/** @type {string} Safe output lock file frontmatter hash mismatch (legacy numeric taxonomy) */
const SAFE_OUTPUT_E009 = "E009";

/** @type {string} Safe output retry exhaustion due to rate limiting (legacy numeric taxonomy) */
const SAFE_OUTPUT_E010 = "E010";

/** @type {string} Safe output operation/runtime failures (legacy numeric taxonomy) */
const SAFE_OUTPUT_E099 = "E099";

/** @type {string} Named code for lock file frontmatter hash mismatch */
const CONFIG_HASH_MISMATCH = "CONFIG_HASH_MISMATCH";

/** @type {string} Named code for rate limit retry exhaustion */
const RATE_LIMIT_EXCEEDED = "RATE_LIMIT_EXCEEDED";

module.exports = {
ERR_VALIDATION,
ERR_PERMISSION,
Expand All @@ -57,5 +69,9 @@ module.exports = {
ERR_PARSE,
ERR_SYSTEM,
SAFE_OUTPUT_E001,
SAFE_OUTPUT_E009,
SAFE_OUTPUT_E010,
SAFE_OUTPUT_E099,
CONFIG_HASH_MISMATCH,
RATE_LIMIT_EXCEEDED,
};
40 changes: 37 additions & 3 deletions actions/setup/js/error_codes.test.cjs
Original file line number Diff line number Diff line change
@@ -1,6 +1,20 @@
// @ts-check
import { describe, it, expect } from "vitest";
const { ERR_VALIDATION, ERR_PERMISSION, ERR_API, ERR_CONFIG, ERR_NOT_FOUND, ERR_PARSE, ERR_SYSTEM, SAFE_OUTPUT_E001, SAFE_OUTPUT_E099 } = require("./error_codes.cjs");
const {
ERR_VALIDATION,
ERR_PERMISSION,
ERR_API,
ERR_CONFIG,
ERR_NOT_FOUND,
ERR_PARSE,
ERR_SYSTEM,
SAFE_OUTPUT_E001,
SAFE_OUTPUT_E009,
SAFE_OUTPUT_E010,
SAFE_OUTPUT_E099,
CONFIG_HASH_MISMATCH,
RATE_LIMIT_EXCEEDED,
} = require("./error_codes.cjs");

describe("error_codes", () => {
describe("module exports", () => {
Expand All @@ -13,11 +27,15 @@ describe("error_codes", () => {
expect(ERR_PARSE).toBeDefined();
expect(ERR_SYSTEM).toBeDefined();
expect(SAFE_OUTPUT_E001).toBeDefined();
expect(SAFE_OUTPUT_E009).toBeDefined();
expect(SAFE_OUTPUT_E010).toBeDefined();
expect(SAFE_OUTPUT_E099).toBeDefined();
expect(CONFIG_HASH_MISMATCH).toBeDefined();
expect(RATE_LIMIT_EXCEEDED).toBeDefined();
});

it("exports string values only", () => {
const codes = [ERR_VALIDATION, ERR_PERMISSION, ERR_API, ERR_CONFIG, ERR_NOT_FOUND, ERR_PARSE, ERR_SYSTEM, SAFE_OUTPUT_E001, SAFE_OUTPUT_E099];
const codes = [ERR_VALIDATION, ERR_PERMISSION, ERR_API, ERR_CONFIG, ERR_NOT_FOUND, ERR_PARSE, ERR_SYSTEM, SAFE_OUTPUT_E001, SAFE_OUTPUT_E009, SAFE_OUTPUT_E010, SAFE_OUTPUT_E099, CONFIG_HASH_MISMATCH, RATE_LIMIT_EXCEEDED];
for (const code of codes) {
expect(typeof code).toBe("string");
}
Expand Down Expand Up @@ -59,9 +77,25 @@ describe("error_codes", () => {
expect(SAFE_OUTPUT_E001).toBe("E001");
});

it("SAFE_OUTPUT_E009 is 'E009'", () => {
expect(SAFE_OUTPUT_E009).toBe("E009");
});

it("SAFE_OUTPUT_E010 is 'E010'", () => {
expect(SAFE_OUTPUT_E010).toBe("E010");
});

it("SAFE_OUTPUT_E099 is 'E099'", () => {
expect(SAFE_OUTPUT_E099).toBe("E099");
});

it("CONFIG_HASH_MISMATCH is 'CONFIG_HASH_MISMATCH'", () => {
expect(CONFIG_HASH_MISMATCH).toBe("CONFIG_HASH_MISMATCH");
});

it("RATE_LIMIT_EXCEEDED is 'RATE_LIMIT_EXCEEDED'", () => {
expect(RATE_LIMIT_EXCEEDED).toBe("RATE_LIMIT_EXCEEDED");
});
});

describe("usage as error message prefixes", () => {
Expand All @@ -82,7 +116,7 @@ describe("error_codes", () => {
});

it("legacy codes are distinct from primary codes", () => {
const all = [ERR_VALIDATION, ERR_PERMISSION, ERR_API, ERR_CONFIG, ERR_NOT_FOUND, ERR_PARSE, ERR_SYSTEM, SAFE_OUTPUT_E001, SAFE_OUTPUT_E099];
const all = [ERR_VALIDATION, ERR_PERMISSION, ERR_API, ERR_CONFIG, ERR_NOT_FOUND, ERR_PARSE, ERR_SYSTEM, SAFE_OUTPUT_E001, SAFE_OUTPUT_E009, SAFE_OUTPUT_E010, SAFE_OUTPUT_E099, CONFIG_HASH_MISMATCH, RATE_LIMIT_EXCEEDED];
const unique = new Set(all);
expect(unique.size).toBe(all.length);
});
Expand Down
60 changes: 56 additions & 4 deletions actions/setup/js/error_recovery.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

const { getErrorMessage } = require("./error_helpers.cjs");
const { ERR_API } = require("./error_codes.cjs");
const { ERR_API, SAFE_OUTPUT_E010, RATE_LIMIT_EXCEEDED } = require("./error_codes.cjs");
const { logRetryEvent } = require("./github_rate_limit_logger.cjs");

/**
Expand Down Expand Up @@ -53,6 +53,22 @@ const RATE_LIMIT_RETRY_CONFIG = {
shouldRetry: isTransientError,
};

/**
* Message indicators used to detect GitHub API rate-limit errors.
* Matched case-insensitively against lower-cased error messages.
* Entries are kept lower-case because we lower-case input messages before substring matching.
* @type {string[]}
*/
const RATE_LIMIT_INDICATORS = ["rate limit", "secondary rate limit", "abuse detection", "too many requests"];

/**
* @param {string} messageLower - Lower-cased error message
* @returns {boolean} True when message indicates GitHub rate limiting
*/
function hasRateLimitIndicator(messageLower) {
return RATE_LIMIT_INDICATORS.some(pattern => messageLower.includes(pattern));
}

/**
* Determine if an error is transient and worth retrying
* @param {any} error - The error to check
Expand All @@ -69,6 +85,13 @@ function isTransientError(error) {
return true;
}

// Status-based rate-limit detection handles cases where the message text is
// not descriptive enough (e.g. Octokit errors with status 429 and a generic
// "Request failed" message). Must be checked before the text patterns.
if (isRateLimitError(error)) {
return true;
}

// Network-related errors that are likely transient
const transientPatterns = [
"network",
Expand All @@ -81,9 +104,7 @@ function isTransientError(error) {
"502 bad gateway",
"503 service unavailable",
"504 gateway timeout",
"rate limit", // GitHub API rate limiting
"secondary rate limit", // GitHub secondary rate limits
"abuse detection", // GitHub abuse detection
...RATE_LIMIT_INDICATORS, // GitHub rate limiting signals (including HTTP 429 text)
"temporarily unavailable",
"no server is currently available", // GitHub API server unavailability
];
Expand Down Expand Up @@ -155,6 +176,25 @@ function getRetryAfterMs(error) {
return null;
}

/**
* Determine whether an error is specifically a GitHub API rate-limit error.
* @param {any} error - The error to classify
* @returns {boolean} True when the error indicates primary or secondary rate limiting
*/
function isRateLimitError(error) {
const status = error?.response?.status ?? error?.status ?? null;
const headers = error?.response?.headers ?? error?.headers ?? null;
const remainingHeader = headers?.["x-ratelimit-remaining"];
const retryAfterHeader = headers?.["retry-after"];
const hasRateLimitHeaders = status === 403 && (retryAfterHeader != null || (remainingHeader != null && parseInt(remainingHeader, 10) === 0));
if (status === 429 || hasRateLimitHeaders) {
return true;
}

const errorMsg = getErrorMessage(error).toLowerCase();
return hasRateLimitIndicator(errorMsg);
}

/**
* Execute an operation with retry logic and exponential backoff
* @template T
Expand Down Expand Up @@ -204,6 +244,18 @@ async function withRetry(operation, config = {}, operationName = "operation") {
// If this was the last attempt, throw the enhanced error
if (attempt === fullConfig.maxRetries) {
core.warning(`${operationName} failed after ${fullConfig.maxRetries} retry attempts: ${errorMsg}`);
// E010 applies only when retry budget is at least 3 retries.
const hasEnoughRetriesForE010 = fullConfig.maxRetries >= 3;
if (hasEnoughRetriesForE010 && isRateLimitError(error)) {
throw enhanceError(error, {
operation: operationName,
attempt: attempt + 1,
maxRetries: fullConfig.maxRetries,
retryable: true,
code: `${SAFE_OUTPUT_E010} ${RATE_LIMIT_EXCEEDED}`,
suggestion: "RATE_LIMIT_EXCEEDED: GitHub API rate limit persisted after 3 retries.",
});
}
throw enhanceError(error, {
operation: operationName,
attempt: attempt + 1,
Expand Down
46 changes: 46 additions & 0 deletions actions/setup/js/error_recovery.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ describe("error_recovery", () => {
it("should identify rate limit errors as transient", () => {
expect(isTransientError(new Error("Rate limit exceeded"))).toBe(true);
expect(isTransientError(new Error("Secondary rate limit hit"))).toBe(true);
expect(isTransientError(new Error("Too Many Requests"))).toBe(true);
expect(isTransientError(new Error("Abuse detection triggered"))).toBe(true);
});

Expand All @@ -50,6 +51,12 @@ describe("error_recovery", () => {
expect(isTransientError(new Error(" <!DOCTYPE html><html>..."))).toBe(true);
});

it("should identify status-only 429 errors as transient even with a non-descriptive message", () => {
// Octokit can produce errors where status=429 but the message contains no rate-limit text.
expect(isTransientError({ status: 429, message: "Request failed" })).toBe(true);
expect(isTransientError({ response: { status: 429 }, message: "Request failed" })).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 Expand Up @@ -103,6 +110,45 @@ describe("error_recovery", () => {
expect(core.warning).toHaveBeenCalledWith(expect.stringContaining("failed after 2 retry attempts"));
});

it("should emit E010 RATE_LIMIT_EXCEEDED when rate limit persists after 3 retries", async () => {
const rateLimitError = {
message: "Too Many Requests",
response: { status: 429, headers: { "retry-after": "1" } },
};
const operation = vi.fn().mockRejectedValue(rateLimitError);

await expect(withRetry(operation, { maxRetries: 3, initialDelayMs: 1 }, "test-operation")).rejects.toThrow("E010 RATE_LIMIT_EXCEEDED");

expect(operation).toHaveBeenCalledTimes(4); // Initial + 3 retries
});

it("should emit E010 for exhausted retries on 403 with retry-after header", async () => {
const rateLimitError = {
message: "secondary rate limit",
response: { status: 403, headers: { "retry-after": "1" } },
};
const operation = vi.fn().mockRejectedValue(rateLimitError);

await expect(withRetry(operation, { maxRetries: 3, initialDelayMs: 1 }, "test-operation")).rejects.toThrow("E010 RATE_LIMIT_EXCEEDED");
});

it("should emit E010 for exhausted retries on abuse-detection message without status", async () => {
const rateLimitError = new Error("Abuse detection mechanism triggered");
const operation = vi.fn().mockRejectedValue(rateLimitError);

await expect(withRetry(operation, { maxRetries: 3, initialDelayMs: 1 }, "test-operation")).rejects.toThrow("E010 RATE_LIMIT_EXCEEDED");
});

it("should emit E010 for status-only 429 with non-descriptive message persisting after 3 retries", async () => {
// Verifies that an Octokit-style error with status=429 and a generic message
// is retried (isTransientError returns true via isRateLimitError) and emits E010 on exhaustion.
const rateLimitError = { status: 429, message: "Request failed" };
const operation = vi.fn().mockRejectedValue(rateLimitError);

await expect(withRetry(operation, { maxRetries: 3, initialDelayMs: 1 }, "test-operation")).rejects.toThrow("E010 RATE_LIMIT_EXCEEDED");
expect(operation).toHaveBeenCalledTimes(4); // Initial + 3 retries
});

it("should use exponential backoff", async () => {
const operation = vi.fn().mockRejectedValueOnce(new Error("Network timeout")).mockRejectedValueOnce(new Error("Network timeout")).mockResolvedValue("success");

Expand Down
Loading