Merge pull request #247 from Codex-/backport_retry_fix

Prevent internal API retries for timing out action run before input timeout is exceeded

Author: Codex-

dist/index.mjs

@@ -10,7 +10,7 @@ import {
   getWorkflowRunJobSteps,
   getWorkflowRunUrl,
   init,
-  retryOrDie,
+  retryOrTimeout,
 } from "./api.ts";
 
 vi.mock("@actions/core");
@@ -427,7 +427,7 @@ describe("API", () => {
     });
   });
 
-  describe("retryOrDie", () => {
+  describe("retryOrTimeout", () => {
     beforeEach(() => {
       vi.useFakeTimers();
     });
@@ -436,38 +436,54 @@ describe("API", () => {
       vi.useRealTimers();
     });
 
-    it("should return a populated array", async () => {
-      const attempt = () => Promise.resolve([0]);
-      expect(await retryOrDie(attempt, 1000)).toHaveLength(1);
+    it("should return a result", async () => {
+      const attemptResult = [0];
+      const attempt = () => Promise.resolve(attemptResult);
+
+      const result = await retryOrTimeout(attempt, 1000);
+      if (result.timeout) {
+        expect.fail("expected retryOrTimeout not to timeout");
+      }
+
+      expect(result.timeout).toStrictEqual(false);
+      expect(result.value).toStrictEqual(attemptResult);
     });
 
-    it("should throw if the given timeout is exceeded", async () => {
+    it("should return a timeout result if the given timeout is exceeded", async () => {
       // Never return data.
       const attempt = () => Promise.resolve([]);
 
-      const retryOrDiePromise = retryOrDie(attempt, 1000);
-      vi.advanceTimersByTime(2000);
+      const retryOrTimeoutPromise = retryOrTimeout(attempt, 1000);
       // eslint-disable-next-line @typescript-eslint/no-floating-promises
       vi.advanceTimersByTimeAsync(2000);
 
-      await expect(retryOrDiePromise).rejects.toThrow(
-        "Timed out while attempting to fetch data",
-      );
+      const result = await retryOrTimeoutPromise;
+      if (!result.timeout) {
+        expect.fail("expected retryOrTimeout to timeout");
+      }
+
+      expect(result.timeout).toStrictEqual(true);
     });
 
     it("should retry to get a populated array", async () => {
+      const attemptResult = [0];
       const attempt = vi
         .fn()
-        .mockResolvedValue([0])
+        .mockResolvedValue(attemptResult)
         .mockResolvedValueOnce([])
         .mockResolvedValueOnce([]);
 
-      const retryOrDiePromise = retryOrDie(attempt, 5000);
-      vi.advanceTimersByTime(3000);
+      const retryOrDiePromise = retryOrTimeout(attempt, 5000);
       // eslint-disable-next-line @typescript-eslint/no-floating-promises
       vi.advanceTimersByTimeAsync(3000);
 
-      expect(await retryOrDiePromise).toHaveLength(1);
+      const result = await retryOrDiePromise;
+      if (result.timeout) {
+        expect.fail("expected retryOrTimeout not to timeout");
+      }
+
+      expect(result.timeout).toStrictEqual(false);
+      expect(result.value).toStrictEqual(attemptResult);
       expect(attempt).toHaveBeenCalledTimes(3);
     });
   });

src/api.spec.ts

@@ -219,41 +219,52 @@ export async function getWorkflowRunJobSteps(runId: number): Promise<string[]> {
       "Fetched Workflow Run Job Steps:\n" +
         `  Repository: ${config.owner}/${config.repo}\n` +
         `  Workflow Run ID: ${runId}\n` +
-        `  Jobs Fetched: [${jobs.map((job) => job.id).join(", ")}]` +
+        `  Jobs Fetched: [${jobs.map((job) => job.id).join(", ")}]\n` +
         `  Steps Fetched: [${steps.join(", ")}]`,
     );
 
     return steps;
   } catch (error) {
     if (error instanceof Error) {
       core.error(
-        `getWorkflowRunJobs: An unexpected error has occurred: ${error.message}`,
+        `getWorkflowRunJobSteps: An unexpected error has occurred: ${error.message}`,
       );
       core.debug(error.stack ?? "");
     }
     throw error;
   }
 }
 
+type RetryOrTimeoutResult<T> = ResultFound<T> | ResultTimeout;
+
+interface ResultFound<T> {
+  timeout: false;
+  value: T;
+}
+
+interface ResultTimeout {
+  timeout: true;
+}
+
 /**
  * Attempt to get a non-empty array from the API.
  */
-export async function retryOrDie<T>(
+export async function retryOrTimeout<T>(
   retryFunc: () => Promise<T[]>,
   timeoutMs: number,
-): Promise<T[]> {
+): Promise<RetryOrTimeoutResult<T[]>> {
   const startTime = Date.now();
   let elapsedTime = 0;
   while (elapsedTime < timeoutMs) {
     elapsedTime = Date.now() - startTime;
 
     const response = await retryFunc();
     if (response.length > 0) {
-      return response;
+      return { timeout: false, value: response };
     }
 
     await new Promise<void>((resolve) => setTimeout(resolve, 1000));
   }
 
-  throw new Error("Timed out while attempting to fetch data");
+  return { timeout: true };
 }

src/api.ts

@@ -37,13 +37,18 @@ async function run(): Promise<void> {
       core.debug(`Attempting to fetch Run IDs for Workflow ID ${workflowId}`);
 
       // Get all runs for a given workflow ID
-      const workflowRunIds = await api.retryOrDie(
+      const fetchWorkflowRunIds = await api.retryOrTimeout(
         () => api.getWorkflowRunIds(workflowId),
-        WORKFLOW_FETCH_TIMEOUT_MS > timeoutMs
-          ? timeoutMs
-          : WORKFLOW_FETCH_TIMEOUT_MS,
+        Math.max(WORKFLOW_FETCH_TIMEOUT_MS, timeoutMs),
       );
+      if (fetchWorkflowRunIds.timeout) {
+        core.debug(
+          `Timed out while attempting to fetch Workflow Run IDs, waited ${Date.now() - startTime}ms`,
+        );
+        break;
+      }
 
+      const workflowRunIds = fetchWorkflowRunIds.value;
       core.debug(
         `Attempting to get step names for Run IDs: [${workflowRunIds.join(", ")}]`,
       );
@@ -88,10 +93,11 @@ async function run(): Promise<void> {
       );
     }
 
-    throw new Error("Timeout exceeded while attempting to get Run ID");
+    core.error("Failed: Timeout exceeded while attempting to get Run ID");
+    core.setFailed("Timeout exceeded while attempting to get Run ID");
   } catch (error) {
     if (error instanceof Error) {
-      core.error(`Failed to complete: ${error.message}`);
+      core.error(`Failed: ${error.message}`);
       core.warning("Does the token have the correct permissions?");
       core.debug(error.stack ?? "");
       core.setFailed(error.message);