feat: New input parameter job fetch retries with a linear backoff str…

…ategy (#267) * feat: make WORKFLOW_JOB_STEPS_RETRY_MS as an action input parameter and linear backoff strategy for retries --------- Co-authored-by: Alex Miller <[email protected]>
Codex- · Jan 9, 2025 · 73e46aa · 73e46aa
1 parent 1482f37
commit 73e46aa
Show file tree

Hide file tree

Showing 12 changed files with 97 additions and 31 deletions.
diff --git a/.github/workflows/action.yml b/.github/workflows/action.yml
@@ -26,6 +26,7 @@ jobs:
           workflow: dispatch.yml
           workflow_inputs: '{"cake":"delicious"}'
           workflow_timeout_seconds: 30
+          workflow_job_steps_retry_seconds: 10
       - name: Evaluate that the Run ID output has been set
         run: |
           if [ "${{ steps.return_dispatch.outputs.run_id }}" == "" ]; then

diff --git a/README.md b/README.md
@@ -25,6 +25,10 @@ steps:
       workflow: automation-test.yml
       workflow_inputs: '{ "some_input": "value" }' # Optional
       workflow_timeout_seconds: 120 # Default: 300
+      workflow_job_steps_retry_seconds:
+        # Lineal backoff retry attempts are made where the attempt count is
+        # the magnitude and the scaling value is `workflow_job_steps_retry_seconds`
+        10 # Default: 5
       distinct_id: someDistinctId # Optional
 
   - name: Use the output run ID and URL

diff --git a/action.yml b/action.yml
@@ -30,6 +30,11 @@ inputs:
   workflow_timeout_seconds:
     description: Time until giving up waiting for the start of the workflow run.
     default: 300
+  workflow_job_steps_retry_seconds:
+    description: |
+      The interval (in seconds) to wait between retries. A linear backoff strategy is used, where the wait time
+      increases by this value with each attempt (e.g., 1st retry = this value, 2nd retry = 2x this value, etc.).
+    default: 5
   distinct_id:
     description: Specify a static string to use instead of a random distinct ID.
 

diff --git a/src/__snapshots__/return-dispatch.spec.ts.snap b/src/__snapshots__/return-dispatch.spec.ts.snap
@@ -6,22 +6,30 @@ exports[`return-dispatch > getRunIdAndUrl > should call retryOrTimeout with the
 
 exports[`return-dispatch > getRunIdAndUrl > should retry until an ID is found 1`] = `"No Run IDs found for workflow, attempt 1..."`;
 
-exports[`return-dispatch > getRunIdAndUrl > should retry until an ID is found 2`] = `"No Run IDs found for workflow, attempt 2..."`;
+exports[`return-dispatch > getRunIdAndUrl > should retry until an ID is found 2`] = `"Waiting for 5000ms before the next attempt..."`;
 
-exports[`return-dispatch > getRunIdAndUrl > should retry until an ID is found 3`] = `"Attempting to get step names for Run IDs: [0]"`;
+exports[`return-dispatch > getRunIdAndUrl > should retry until an ID is found 3`] = `"No Run IDs found for workflow, attempt 2..."`;
+
+exports[`return-dispatch > getRunIdAndUrl > should retry until an ID is found 4`] = `"Waiting for 10000ms before the next attempt..."`;
+
+exports[`return-dispatch > getRunIdAndUrl > should retry until an ID is found 5`] = `"Attempting to get step names for Run IDs: [0]"`;
 
 exports[`return-dispatch > getRunIdAndUrl > should return the ID when found 1`] = `"Attempting to get step names for Run IDs: [0]"`;
 
 exports[`return-dispatch > getRunIdAndUrl > should return the ID when found 2`] = `undefined`;
 
 exports[`return-dispatch > getRunIdAndUrl > should timeout when unable to find over time 1`] = `"Exhausted searching IDs in known runs, attempt 1..."`;
 
-exports[`return-dispatch > getRunIdAndUrl > should timeout when unable to find over time 2`] = `"Attempting to get step names for Run IDs: [0]"`;
+exports[`return-dispatch > getRunIdAndUrl > should timeout when unable to find over time 2`] = `"Waiting for 3000ms before the next attempt..."`;
 
-exports[`return-dispatch > getRunIdAndUrl > should timeout when unable to find over time 3`] = `"Exhausted searching IDs in known runs, attempt 2..."`;
+exports[`return-dispatch > getRunIdAndUrl > should timeout when unable to find over time 3`] = `"Attempting to get step names for Run IDs: [0]"`;
 
-exports[`return-dispatch > getRunIdAndUrl > should timeout when unable to find over time 4`] = `"Attempting to get step names for Run IDs: [0]"`;
+exports[`return-dispatch > getRunIdAndUrl > should timeout when unable to find over time 4`] = `"Exhausted searching IDs in known runs, attempt 2..."`;
 
-exports[`return-dispatch > getRunIdAndUrl > should timeout when unable to find over time 5`] = `"Exhausted searching IDs in known runs, attempt 3..."`;
+exports[`return-dispatch > getRunIdAndUrl > should timeout when unable to find over time 5`] = `"Waiting for 6000ms before the next attempt..."`;
 
 exports[`return-dispatch > getRunIdAndUrl > should timeout when unable to find over time 6`] = `"Attempting to get step names for Run IDs: [0]"`;
+
+exports[`return-dispatch > getRunIdAndUrl > should timeout when unable to find over time 7`] = `"Exhausted searching IDs in known runs, attempt 3..."`;
+
+exports[`return-dispatch > getRunIdAndUrl > should timeout when unable to find over time 8`] = `"Attempting to get step names for Run IDs: [0]"`;
diff --git a/src/action.spec.ts b/src/action.spec.ts
@@ -28,6 +28,7 @@ describe("Action", () => {
         workflow: "workflow_name",
         workflow_inputs: JSON.stringify(workflowInputs),
         workflow_timeout_seconds: "60",
+        workflow_job_steps_retry_seconds: "3",
         distinct_id: "distinct_id",
       };
 
@@ -48,6 +49,8 @@ describe("Action", () => {
             return mockEnvConfig.workflow_inputs;
           case "workflow_timeout_seconds":
             return mockEnvConfig.workflow_timeout_seconds;
+          case "workflow_job_steps_retry_seconds":
+            return mockEnvConfig.workflow_job_steps_retry_seconds;
           case "distinct_id":
             return mockEnvConfig.distinct_id;
           default:
@@ -72,6 +75,7 @@ describe("Action", () => {
       expect(config.workflow).toStrictEqual("workflow_name");
       expect(config.workflowInputs).toStrictEqual(workflowInputs);
       expect(config.workflowTimeoutSeconds).toStrictEqual(60);
+      expect(config.workflowJobStepsRetrySeconds).toStrictEqual(3);
       expect(config.distinctId).toStrictEqual("distinct_id");
     });
 
@@ -89,6 +93,13 @@ describe("Action", () => {
       expect(config.workflowTimeoutSeconds).toStrictEqual(300);
     });
 
+    it("should provide a default workflow job step retry if none is supplied", () => {
+      mockEnvConfig.workflow_job_steps_retry_seconds = "";
+      const config: ActionConfig = getConfig();
+
+      expect(config.workflowJobStepsRetrySeconds).toStrictEqual(5);
+    });
+
     it("should handle no inputs being provided", () => {
       mockEnvConfig.workflow_inputs = "";
       const config: ActionConfig = getConfig();

diff --git a/src/action.ts b/src/action.ts
@@ -3,6 +3,7 @@ import { randomUUID } from "node:crypto";
 import * as core from "@actions/core";
 
 const WORKFLOW_TIMEOUT_SECONDS = 5 * 60;
+const WORKFLOW_JOB_STEPS_RETRY_SECONDS = 5;
 
 /**
  * action.yaml definition.
@@ -43,6 +44,11 @@ export interface ActionConfig {
    */
   workflowTimeoutSeconds: number;
 
+  /**
+   * Time in retries for identifying the Run ID.
+   */
+  workflowJobStepsRetrySeconds: number;
+
   /**
    * Specify a static ID to use instead of a distinct ID.
    */
@@ -69,6 +75,9 @@ export function getConfig(): ActionConfig {
     workflowTimeoutSeconds:
       getNumberFromValue(core.getInput("workflow_timeout_seconds")) ??
       WORKFLOW_TIMEOUT_SECONDS,
+    workflowJobStepsRetrySeconds:
+      getNumberFromValue(core.getInput("workflow_job_steps_retry_seconds")) ??
+      WORKFLOW_JOB_STEPS_RETRY_SECONDS,
     distinctId:
       getOptionalWorkflowValue(core.getInput("distinct_id")) ?? randomUUID(),
   };

diff --git a/src/api.spec.ts b/src/api.spec.ts
@@ -95,6 +95,8 @@ describe("API", () => {
           return JSON.stringify({ testInput: "test" });
         case "workflow_timeout_seconds":
           return "30";
+        case "workflow_job_steps_retry_seconds":
+          return "5";
         default:
           return "";
       }
@@ -332,6 +334,7 @@ describe("API", () => {
       workflow: "workflow_name",
       workflowInputs: { testInput: "test" },
       workflowTimeoutSeconds: 60,
+      workflowJobStepsRetrySeconds: 3,
       distinctId: "test-uuid",
     };
 

diff --git a/src/constants.ts b/src/constants.ts
@@ -1,6 +1,5 @@
 /* eslint-disable @typescript-eslint/no-inferrable-types */
 
 export const WORKFLOW_FETCH_TIMEOUT_MS: number = 60 * 1000;
-export const WORKFLOW_JOB_STEPS_RETRY_MS: number = 5000;
 export const WORKFLOW_JOB_STEPS_SERVER_ERROR_RETRY_MAX: number = 3;
 export const WORKFLOW_JOB_STEPS_SERVER_ERROR_RETRY_MS: number = 500;
diff --git a/src/main.spec.ts b/src/main.spec.ts
@@ -35,6 +35,7 @@ describe("main", () => {
     ref: "test-ref",
     workflow: "test-workflow",
     workflowTimeoutSeconds: 0,
+    workflowJobStepsRetrySeconds: 0,
   } satisfies Partial<action.ActionConfig> as action.ActionConfig;
   const testBranch: utils.BranchNameResult = {
     branchName: "test-branch",
@@ -173,6 +174,7 @@ describe("main", () => {
       distinctIdRegex: distinctIdRegex,
       workflowId: 0,
       workflowTimeoutMs: testCfg.workflowTimeoutSeconds * 1000,
+      workflowJobStepsRetryMs: testCfg.workflowJobStepsRetrySeconds * 1000,
     });
 
     // Result

diff --git a/src/main.ts b/src/main.ts
@@ -44,6 +44,7 @@ export async function main(): Promise<void> {
       distinctIdRegex,
       workflowId,
       workflowTimeoutMs: config.workflowTimeoutSeconds * 1000,
+      workflowJobStepsRetryMs: config.workflowJobStepsRetrySeconds * 1000,
     });
     if (result.success) {
       handleActionSuccess(result.value.id, result.value.url);

diff --git a/src/return-dispatch.spec.ts b/src/return-dispatch.spec.ts
@@ -480,6 +480,7 @@ describe("return-dispatch", () => {
       distinctIdRegex: distinctIdRegex,
       workflowId: workflowId,
       workflowTimeoutMs: 100,
+      workflowJobStepsRetryMs: 5,
     };
 
     let apiFetchWorkflowRunIdsMock: MockInstance<
@@ -662,13 +663,14 @@ describe("return-dispatch", () => {
         .mockResolvedValueOnce({ success: true, value: [] });
       apiFetchWorkflowRunJobStepsMock.mockResolvedValue([distinctId]);
       apiFetchWorkflowRunUrlMock.mockResolvedValue(runUrl);
-      vi.spyOn(constants, "WORKFLOW_JOB_STEPS_RETRY_MS", "get").mockReturnValue(
-        5000,
-      );
+
+      const retryMs = 5000;
+      const timeoutMs = 60 * 60 * 100;
 
       const getRunIdAndUrlPromise = getRunIdAndUrl({
         ...defaultOpts,
-        workflowTimeoutMs: 60 * 60 * 1000,
+        workflowTimeoutMs: timeoutMs,
+        workflowJobStepsRetryMs: retryMs,
       });
 
       // First attempt
@@ -677,28 +679,30 @@ describe("return-dispatch", () => {
 
       assertOnlyCalled(coreInfoLogMock);
 
-      expect(coreInfoLogMock).toHaveBeenCalledOnce();
+      expect(coreInfoLogMock).toHaveBeenCalledTimes(2);
       expect(coreInfoLogMock.mock.calls[0]?.[0]).toMatchSnapshot();
+      expect(coreInfoLogMock.mock.calls[1]?.[0]).toMatchSnapshot();
 
       expect(utilSleepMock).toHaveBeenCalledOnce();
-      expect(utilSleepMock).toHaveBeenCalledWith(5000);
+      expect(utilSleepMock).toHaveBeenCalledWith(retryMs);
 
       resetLogMocks();
-      await vi.advanceTimersByTimeAsync(5000);
+      await vi.advanceTimersByTimeAsync(retryMs);
 
       // Second attempt
       expect(apiRetryOrTimeoutMock).toHaveBeenCalledTimes(2);
 
       assertOnlyCalled(coreInfoLogMock);
 
-      expect(coreInfoLogMock).toHaveBeenCalledOnce();
+      expect(coreInfoLogMock).toHaveBeenCalledTimes(2);
       expect(coreInfoLogMock.mock.calls[0]?.[0]).toMatchSnapshot();
+      expect(coreInfoLogMock.mock.calls[1]?.[0]).toMatchSnapshot();
 
       expect(utilSleepMock).toHaveBeenCalledTimes(2);
-      expect(utilSleepMock).toHaveBeenCalledWith(5000);
+      expect(utilSleepMock).toHaveBeenCalledWith(retryMs * 2);
 
       resetLogMocks();
-      await vi.advanceTimersByTimeAsync(5000);
+      await vi.advanceTimersByTimeAsync(retryMs * 2);
 
       // Third attempt
       expect(apiRetryOrTimeoutMock).toHaveBeenCalledTimes(3);
@@ -771,13 +775,14 @@ describe("return-dispatch", () => {
       });
       apiFetchWorkflowRunJobStepsMock.mockResolvedValue([]);
       apiFetchWorkflowRunUrlMock.mockResolvedValue(runUrl);
-      vi.spyOn(constants, "WORKFLOW_JOB_STEPS_RETRY_MS", "get").mockReturnValue(
-        5000,
-      );
+
+      const retryMs = 3000;
+      const timeoutMs = 15 * 1000;
 
       const getRunIdAndUrlPromise = getRunIdAndUrl({
         ...defaultOpts,
-        workflowTimeoutMs: 15 * 1000,
+        workflowTimeoutMs: timeoutMs,
+        workflowJobStepsRetryMs: retryMs,
       });
 
       // First attempt
@@ -786,51 +791,60 @@ describe("return-dispatch", () => {
       expect(apiFetchWorkflowRunJobStepsMock).toHaveBeenCalledOnce();
       assertOnlyCalled(coreDebugLogMock, coreInfoLogMock);
 
-      expect(coreInfoLogMock).toHaveBeenCalledOnce();
+      expect(coreInfoLogMock).toHaveBeenCalledTimes(2);
       expect(coreInfoLogMock.mock.calls[0]?.[0]).toMatchSnapshot();
+      expect(coreInfoLogMock.mock.calls[1]?.[0]).toMatchSnapshot();
 
       expect(coreDebugLogMock).toHaveBeenCalledOnce();
       expect(coreDebugLogMock.mock.calls[0]?.[0]).toMatchSnapshot();
 
       expect(utilSleepMock).toHaveBeenCalledOnce();
-      expect(utilSleepMock).toHaveBeenCalledWith(5000);
+      expect(utilSleepMock).toHaveBeenCalledWith(retryMs);
 
       resetLogMocks();
-      await vi.advanceTimersByTimeAsync(5000);
+      await vi.advanceTimersByTimeAsync(retryMs);
 
       // Second attempt
       expect(apiRetryOrTimeoutMock).toHaveBeenCalledTimes(2);
       expect(apiFetchWorkflowRunJobStepsMock).toHaveBeenCalledTimes(2);
       assertOnlyCalled(coreDebugLogMock, coreInfoLogMock);
 
-      expect(coreInfoLogMock).toHaveBeenCalledOnce();
+      expect(coreInfoLogMock).toHaveBeenCalledTimes(2);
       expect(coreInfoLogMock.mock.calls[0]?.[0]).toMatchSnapshot();
+      expect(coreInfoLogMock.mock.calls[1]?.[0]).toMatchSnapshot();
 
       expect(coreDebugLogMock).toHaveBeenCalledOnce();
       expect(coreDebugLogMock.mock.calls[0]?.[0]).toMatchSnapshot();
 
       expect(utilSleepMock).toHaveBeenCalledTimes(2);
-      expect(utilSleepMock).toHaveBeenCalledWith(5000);
+      expect(utilSleepMock).toHaveBeenCalledWith(retryMs * 2);
 
       resetLogMocks();
-      await vi.advanceTimersByTimeAsync(5000);
+      await vi.advanceTimersByTimeAsync(retryMs * 2);
 
       // Timeout attempt
       expect(apiRetryOrTimeoutMock).toHaveBeenCalledTimes(3);
       expect(apiFetchWorkflowRunJobStepsMock).toHaveBeenCalledTimes(3);
       assertOnlyCalled(coreDebugLogMock, coreInfoLogMock);
 
-      expect(coreInfoLogMock).toHaveBeenCalledOnce();
+      expect(coreInfoLogMock).toHaveBeenCalledTimes(2);
       expect(coreInfoLogMock.mock.calls[0]?.[0]).toMatchSnapshot();
+      expect(coreInfoLogMock.mock.calls[1]?.[0]).toMatch(
+        /Waiting for \d{4,5}ms before the next attempt\.\.\./,
+      );
 
       expect(coreDebugLogMock).toHaveBeenCalledOnce();
       expect(coreDebugLogMock.mock.calls[0]?.[0]).toMatchSnapshot();
 
       expect(utilSleepMock).toHaveBeenCalledTimes(3);
-      expect(utilSleepMock).toHaveBeenCalledWith(5000);
+      const elapsedTime = Date.now() - defaultOpts.startTime; // `waitTime` should be using `workflowTimeoutMs` at this point
+      expect(utilSleepMock.mock.lastCall?.[0]).approximately(
+        timeoutMs - elapsedTime,
+        5,
+      );
 
       resetLogMocks();
-      await vi.advanceTimersByTimeAsync(5000);
+      await vi.advanceTimersByTimeAsync(retryMs * 3);
 
       // Result
       const run = await getRunIdAndUrlPromise;

diff --git a/src/return-dispatch.ts b/src/return-dispatch.ts
@@ -129,13 +129,15 @@ export interface GetRunIdAndUrlOpts {
   distinctIdRegex: RegExp;
   workflowId: number;
   workflowTimeoutMs: number;
+  workflowJobStepsRetryMs: number;
 }
 export async function getRunIdAndUrl({
   startTime,
   branch,
   distinctIdRegex,
   workflowId,
   workflowTimeoutMs,
+  workflowJobStepsRetryMs,
 }: GetRunIdAndUrlOpts): Promise<Result<{ id: number; url: string }>> {
   const retryTimeout = Math.max(
     constants.WORKFLOW_FETCH_TIMEOUT_MS,
@@ -178,7 +180,14 @@ export async function getRunIdAndUrl({
       core.info(`No Run IDs found for workflow, attempt ${attemptNo}...`);
     }
 
-    await sleep(constants.WORKFLOW_JOB_STEPS_RETRY_MS);
+    const waitTime = Math.min(
+      workflowJobStepsRetryMs * attemptNo, // Lineal backoff
+      workflowTimeoutMs - elapsedTime, // Ensure we don't exceed the timeout
+    );
+
+    core.info(`Waiting for ${waitTime}ms before the next attempt...`);
+    await sleep(waitTime);
+
     elapsedTime = Date.now() - startTime;
   }