Validate and retry manifest fetch to prevent silent failures (#1332)

* validate and retry manifest fetch

* Refactor error handling in isRateLimitError function for improved clarity

__tests__/install-python.test.ts

@@ -31,6 +31,10 @@ describe('getManifest', () => {
     jest.resetAllMocks();
   });
 
+  afterEach(() => {
+    jest.useRealTimers();
+  });
+
   it('should return manifest from repo', async () => {
     (tc.getManifestFromRepo as jest.Mock).mockResolvedValue(mockManifest);
     const manifest = await getManifest();
@@ -38,14 +42,70 @@ describe('getManifest', () => {
   });
 
   it('should return manifest from URL if repo fetch fails', async () => {
+    jest.useFakeTimers();
     (tc.getManifestFromRepo as jest.Mock).mockRejectedValue(
       new Error('Fetch failed')
     );
     (httpm.HttpClient.prototype.getJson as jest.Mock).mockResolvedValue({
       result: mockManifest
     });
+    const promise = getManifest();
+    await jest.runAllTimersAsync();
+    const manifest = await promise;
+    expect(manifest).toEqual(mockManifest);
+  });
+
+  it('should fall back to URL if repo returns a truncated/empty manifest', async () => {
+    jest.useFakeTimers();
+    (tc.getManifestFromRepo as jest.Mock).mockResolvedValue([]);
+    (httpm.HttpClient.prototype.getJson as jest.Mock).mockResolvedValue({
+      result: mockManifest
+    });
+    const promise = getManifest();
+    await jest.runAllTimersAsync();
+    const manifest = await promise;
+    expect(manifest).toEqual(mockManifest);
+  });
+
+  it('should retry on a transient invalid manifest and then succeed', async () => {
+    jest.useFakeTimers();
+    (tc.getManifestFromRepo as jest.Mock)
+      .mockResolvedValueOnce([])
+      .mockResolvedValueOnce(mockManifest);
+    const promise = getManifest();
+    await jest.runAllTimersAsync();
+    const manifest = await promise;
+    expect(manifest).toEqual(mockManifest);
+    expect(tc.getManifestFromRepo as jest.Mock).toHaveBeenCalledTimes(2);
+  });
+
+  it('should fail loudly when the manifest is truncated/empty on every source', async () => {
+    jest.useFakeTimers();
+    (tc.getManifestFromRepo as jest.Mock).mockResolvedValue([]);
+    (httpm.HttpClient.prototype.getJson as jest.Mock).mockResolvedValue({
+      result: []
+    });
+    const promise = getManifest();
+    // Prevent unhandled rejection before timers advance.
+    const catchPromise = promise.catch(() => {});
+    await jest.runAllTimersAsync();
+    await catchPromise;
+    await expect(promise).rejects.toThrow(
+      'Failed to fetch the Python versions manifest'
+    );
+  });
+
+  it('should not retry the API on a rate-limit error and fall back to URL immediately', async () => {
+    const rateLimitError = Object.assign(new Error('API rate limit exceeded'), {
+      httpStatusCode: 403
+    });
+    (tc.getManifestFromRepo as jest.Mock).mockRejectedValue(rateLimitError);
+    (httpm.HttpClient.prototype.getJson as jest.Mock).mockResolvedValue({
+      result: mockManifest
+    });
     const manifest = await getManifest();
     expect(manifest).toEqual(mockManifest);
+    expect(tc.getManifestFromRepo as jest.Mock).toHaveBeenCalledTimes(1);
   });
 });
 

dist/setup/index.js

@@ -55685,6 +55685,8 @@ function findRhelRelease(semanticVersionSpec, architecture, manifest, osVersion)
     }
     return undefined;
 }
+const MANIFEST_FETCH_MAX_ATTEMPTS = 3;
+const MANIFEST_FETCH_RETRY_BASE_DELAY_MS = 1000;
 async function findReleaseFromManifest(semanticVersionSpec, architecture, manifest) {
     if (!manifest) {
         manifest = await getManifest();
@@ -55712,15 +55714,54 @@ function isIToolRelease(obj) {
             typeof file.arch === 'string' &&
             typeof file.download_url === 'string'));
 }
+// Rejects empty or truncated manifest responses.
+function isValidManifest(manifest) {
+    return (Array.isArray(manifest) &&
+        manifest.length > 0 &&
+        manifest.every(isIToolRelease));
+}
+function sleep(ms) {
+    return new Promise(resolve => setTimeout(resolve, ms));
+}
+// HTTP 403/429 from http-client (`statusCode`) or tool-cache (`httpStatusCode`).
+function isRateLimitError(err) {
+    const e = err;
+    const status = e?.httpStatusCode ?? e?.statusCode;
+    return status === 403 || status === 429;
+}
+// Fetches and validates a manifest, retrying transient failures with backoff.
+async function fetchValidManifest(source, fetcher) {
+    let lastError;
+    let attempts = 0;
+    for (let attempt = 1; attempt <= MANIFEST_FETCH_MAX_ATTEMPTS; attempt++) {
+        attempts = attempt;
+        try {
+            const manifest = await fetcher();
+            if (isValidManifest(manifest)) {
+                return manifest;
+            }
+            throw new Error(`The manifest fetched from ${source} is empty, truncated, or does not contain any valid tool release entries.`);
+        }
+        catch (err) {
+            lastError = err instanceof Error ? err : new Error(String(err));
+            core.debug(`Attempt ${attempt}/${MANIFEST_FETCH_MAX_ATTEMPTS} to fetch the manifest from ${source} failed: ${lastError.message}`);
+            // Rate limits won't clear within the backoff window; fall back instead.
+            if (isRateLimitError(err)) {
+                core.debug(`${source} is rate-limited; skipping retries for this source.`);
+                break;
+            }
+            if (attempt < MANIFEST_FETCH_MAX_ATTEMPTS) {
+                const delay = MANIFEST_FETCH_RETRY_BASE_DELAY_MS * 2 ** (attempt - 1);
+                core.debug(`Retrying in ${delay}ms...`);
+                await sleep(delay);
+            }
+        }
+    }
+    throw new Error(`Failed to fetch a valid manifest from ${source} after ${attempts} attempt(s): ${lastError?.message}`);
+}
 async function getManifest() {
     try {
-        const repoManifest = await getManifestFromRepo();
+        return await fetchValidManifest('the GitHub API', getManifestFromRepo);
-        if (Array.isArray(repoManifest) &&
-            repoManifest.length &&
-            repoManifest.every(isIToolRelease)) {
-            return repoManifest;
-        }
-        throw new Error('The repository manifest is invalid or does not include any valid tool release (IToolRelease) entries.');
     }
     catch (err) {
         core.debug('Fetching the manifest via the API failed.');
@@ -55728,10 +55769,17 @@ async function getManifest() {
             core.debug(err.message);
         }
         else {
-            core.error('An unexpected error occurred while fetching the manifest.');
+            core.debug('An unexpected error occurred while fetching the manifest.');
         }
     }
-    return await getManifestFromURL();
+    try {
+        return await fetchValidManifest('the raw URL', getManifestFromURL);
+    }
+    catch (err) {
+        const message = err instanceof Error ? err.message : String(err);
+        // Fail loudly so the action doesn't exit 0 without installing Python.
+        throw new Error(`Failed to fetch the Python versions manifest. The response was empty, truncated, or invalid, and all retries were exhausted. ${message}`);
+    }
 }
 function getManifestFromRepo() {
     core.debug(`Getting manifest from ${MANIFEST_REPO_OWNER}/${MANIFEST_REPO_NAME}@${MANIFEST_REPO_BRANCH}`);

src/install-python.ts

@@ -80,6 +80,9 @@ function findRhelRelease(
   return undefined;
 }
 
+const MANIFEST_FETCH_MAX_ATTEMPTS = 3;
+const MANIFEST_FETCH_RETRY_BASE_DELAY_MS = 1000;
+
 export async function findReleaseFromManifest(
   semanticVersionSpec: string,
   architecture: string,
@@ -133,28 +136,95 @@ function isIToolRelease(obj: any): obj is IToolRelease {
   );
 }
 
-export async function getManifest(): Promise<tc.IToolRelease[]> {
+// Rejects empty or truncated manifest responses.
+function isValidManifest(manifest: unknown): manifest is tc.IToolRelease[] {
+  return (
+    Array.isArray(manifest) &&
+    manifest.length > 0 &&
+    manifest.every(isIToolRelease)
+  );
+}
+
+function sleep(ms: number): Promise<void> {
+  return new Promise(resolve => setTimeout(resolve, ms));
+}
+
+// HTTP 403/429 from http-client (`statusCode`) or tool-cache (`httpStatusCode`).
+function isRateLimitError(err: unknown): boolean {
+  const e = err as
+    | {httpStatusCode?: number; statusCode?: number}
+    | null
+    | undefined;
+  const status = e?.httpStatusCode ?? e?.statusCode;
+  return status === 403 || status === 429;
+}
+
+// Fetches and validates a manifest, retrying transient failures with backoff.
+async function fetchValidManifest(
+  source: string,
+  fetcher: () => Promise<tc.IToolRelease[]>
+): Promise<tc.IToolRelease[]> {
+  let lastError: Error | undefined;
+  let attempts = 0;
+
+  for (let attempt = 1; attempt <= MANIFEST_FETCH_MAX_ATTEMPTS; attempt++) {
+    attempts = attempt;
     try {
-    const repoManifest = await getManifestFromRepo();
+      const manifest = await fetcher();
-    if (
+      if (isValidManifest(manifest)) {
-      Array.isArray(repoManifest) &&
+        return manifest;
-      repoManifest.length &&
-      repoManifest.every(isIToolRelease)
-    ) {
-      return repoManifest;
       }
       throw new Error(
-      'The repository manifest is invalid or does not include any valid tool release (IToolRelease) entries.'
+        `The manifest fetched from ${source} is empty, truncated, or does not contain any valid tool release entries.`
       );
+    } catch (err) {
+      lastError = err instanceof Error ? err : new Error(String(err));
+      core.debug(
+        `Attempt ${attempt}/${MANIFEST_FETCH_MAX_ATTEMPTS} to fetch the manifest from ${source} failed: ${lastError.message}`
+      );
+
+      // Rate limits won't clear within the backoff window; fall back instead.
+      if (isRateLimitError(err)) {
+        core.debug(
+          `${source} is rate-limited; skipping retries for this source.`
+        );
+        break;
+      }
+
+      if (attempt < MANIFEST_FETCH_MAX_ATTEMPTS) {
+        const delay = MANIFEST_FETCH_RETRY_BASE_DELAY_MS * 2 ** (attempt - 1);
+        core.debug(`Retrying in ${delay}ms...`);
+        await sleep(delay);
+      }
+    }
+  }
+
+  throw new Error(
+    `Failed to fetch a valid manifest from ${source} after ${attempts} attempt(s): ${lastError?.message}`
+  );
+}
+
+export async function getManifest(): Promise<tc.IToolRelease[]> {
+  try {
+    return await fetchValidManifest('the GitHub API', getManifestFromRepo);
   } catch (err) {
     core.debug('Fetching the manifest via the API failed.');
     if (err instanceof Error) {
       core.debug(err.message);
     } else {
-      core.error('An unexpected error occurred while fetching the manifest.');
+      core.debug('An unexpected error occurred while fetching the manifest.');
     }
   }
-  return await getManifestFromURL();
+
+  try {
+    return await fetchValidManifest('the raw URL', getManifestFromURL);
+  } catch (err) {
+    const message = err instanceof Error ? err.message : String(err);
+    // Fail loudly so the action doesn't exit 0 without installing Python.
+    throw new Error(
+      `Failed to fetch the Python versions manifest. The response was empty, truncated, or invalid, and all retries were exhausted. ${message}`
+    );
+  }
 }
 
 export function getManifestFromRepo(): Promise<tc.IToolRelease[]> {