From 9c8b77cf65bfbcff3e014756322d06930a451ea1 Mon Sep 17 00:00:00 2001 From: German Suarez Alonso Date: Fri, 20 Dec 2024 11:58:44 +0100 Subject: [PATCH 1/5] make WORKFLOW_JOB_STEPS_RETRY_MS as an action input parameter and linear backoff strategy for retries --- .github/workflows/action.yml | 1 + README.md | 1 + action.yml | 3 +++ src/action.spec.ts | 11 ++++++++++ src/action.ts | 9 +++++++++ src/api.spec.ts | 3 +++ src/constants.ts | 1 - src/main.spec.ts | 2 ++ src/main.ts | 1 + src/return-dispatch.spec.ts | 39 +++++++++++++++++++----------------- src/return-dispatch.ts | 15 ++++++++++++-- 11 files changed, 65 insertions(+), 21 deletions(-) diff --git a/.github/workflows/action.yml b/.github/workflows/action.yml index b9f909c2..64ad3874 100644 --- 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 index dec03ca1..1674c3b0 100644 --- a/README.md +++ b/README.md @@ -25,6 +25,7 @@ steps: workflow: automation-test.yml workflow_inputs: '{ "some_input": "value" }' # Optional workflow_timeout_seconds: 120 # Default: 300 + 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 index e1a9cbb4..5a6fa8e5 100644 --- a/action.yml +++ b/action.yml @@ -30,6 +30,9 @@ 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: Duration to wait between retries while monitoring for the workflow run to start. + default: 5 distinct_id: description: Specify a static string to use instead of a random distinct ID. diff --git a/src/action.spec.ts b/src/action.spec.ts index c378389d..48b9a83b 100644 --- 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 index a521e1fc..b9f23394 100644 --- 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 index 25c4b352..6723b322 100644 --- 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 index 693c62a2..db31c7f1 100644 --- 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 index 179ccb30..e4751ef5 100644 --- 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 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 index 17e56a8c..4b3412e6 100644 --- a/src/main.ts +++ b/src/main.ts @@ -44,6 +44,7 @@ export async function main(): Promise { 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 index 5ec69e31..33d73347 100644 --- 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 @@ -681,10 +683,10 @@ describe("return-dispatch", () => { expect(coreInfoLogMock.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); @@ -695,10 +697,10 @@ describe("return-dispatch", () => { expect(coreInfoLogMock.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); // Third attempt expect(apiRetryOrTimeoutMock).toHaveBeenCalledTimes(3); @@ -771,13 +773,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 @@ -793,10 +796,10 @@ describe("return-dispatch", () => { 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); @@ -810,10 +813,10 @@ describe("return-dispatch", () => { 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); @@ -827,10 +830,10 @@ describe("return-dispatch", () => { expect(coreDebugLogMock.mock.calls[0]?.[0]).toMatchSnapshot(); expect(utilSleepMock).toHaveBeenCalledTimes(3); - expect(utilSleepMock).toHaveBeenCalledWith(5000); + expect(utilSleepMock).toHaveBeenCalledWith(retryMs * 3); 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 index 64010fd7..8f5f4a41 100644 --- a/src/return-dispatch.ts +++ b/src/return-dispatch.ts @@ -129,6 +129,7 @@ export interface GetRunIdAndUrlOpts { distinctIdRegex: RegExp; workflowId: number; workflowTimeoutMs: number; + workflowJobStepsRetryMs: number; } export async function getRunIdAndUrl({ startTime, @@ -136,14 +137,16 @@ export async function getRunIdAndUrl({ distinctIdRegex, workflowId, workflowTimeoutMs, + workflowJobStepsRetryMs, }: GetRunIdAndUrlOpts): Promise> { const retryTimeout = Math.max( - constants.WORKFLOW_FETCH_TIMEOUT_MS, + workflowJobStepsRetryMs, workflowTimeoutMs, ); let attemptNo = 0; let elapsedTime = Date.now() - startTime; + while (elapsedTime < workflowTimeoutMs) { attemptNo++; @@ -178,9 +181,17 @@ 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; } return { success: false, reason: "timeout" }; } + From 87b877d5c99c36573ce327310d07912220fd64f9 Mon Sep 17 00:00:00 2001 From: German Suarez Alonso Date: Mon, 23 Dec 2024 17:07:01 +0100 Subject: [PATCH 2/5] fix tests and formatting --- src/action.ts | 2 +- src/main.spec.ts | 2 +- src/return-dispatch.spec.ts | 8 +++++--- src/return-dispatch.ts | 3 +-- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/action.ts b/src/action.ts index b9f23394..7ac5254a 100644 --- a/src/action.ts +++ b/src/action.ts @@ -3,7 +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 +const WORKFLOW_JOB_STEPS_RETRY_SECONDS = 5; /** * action.yaml definition. diff --git a/src/main.spec.ts b/src/main.spec.ts index e4751ef5..9ad5a8c6 100644 --- a/src/main.spec.ts +++ b/src/main.spec.ts @@ -174,7 +174,7 @@ describe("main", () => { distinctIdRegex: distinctIdRegex, workflowId: 0, workflowTimeoutMs: testCfg.workflowTimeoutSeconds * 1000, - workflowJobStepsRetryMs :testCfg.workflowJobStepsRetrySeconds * 1000, + workflowJobStepsRetryMs: testCfg.workflowJobStepsRetrySeconds * 1000, }); // Result diff --git a/src/return-dispatch.spec.ts b/src/return-dispatch.spec.ts index 33d73347..f5256101 100644 --- a/src/return-dispatch.spec.ts +++ b/src/return-dispatch.spec.ts @@ -666,7 +666,7 @@ describe("return-dispatch", () => { const retryMs = 5000; const timeoutMs = 60 * 60 * 100; - + const getRunIdAndUrlPromise = getRunIdAndUrl({ ...defaultOpts, workflowTimeoutMs: timeoutMs, @@ -679,8 +679,9 @@ 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(retryMs); @@ -789,8 +790,9 @@ 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(); diff --git a/src/return-dispatch.ts b/src/return-dispatch.ts index 8f5f4a41..49241a00 100644 --- a/src/return-dispatch.ts +++ b/src/return-dispatch.ts @@ -140,7 +140,7 @@ export async function getRunIdAndUrl({ workflowJobStepsRetryMs, }: GetRunIdAndUrlOpts): Promise> { const retryTimeout = Math.max( - workflowJobStepsRetryMs, + constants.WORKFLOW_FETCH_TIMEOUT_MS, workflowTimeoutMs, ); @@ -194,4 +194,3 @@ export async function getRunIdAndUrl({ return { success: false, reason: "timeout" }; } - From 1674d6ac41dec3b5ecd0203404e308c456c458e4 Mon Sep 17 00:00:00 2001 From: German Suarez Alonso Date: Thu, 26 Dec 2024 13:12:56 +0100 Subject: [PATCH 3/5] update snapshot and fix tests --- .../return-dispatch.spec.ts.snap | 20 +++++++++++++------ src/return-dispatch.spec.ts | 13 ++++++++---- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/src/__snapshots__/return-dispatch.spec.ts.snap b/src/__snapshots__/return-dispatch.spec.ts.snap index e85d65b7..acb6fee3 100644 --- a/src/__snapshots__/return-dispatch.spec.ts.snap +++ b/src/__snapshots__/return-dispatch.spec.ts.snap @@ -6,9 +6,13 @@ 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]"`; @@ -16,12 +20,16 @@ exports[`return-dispatch > getRunIdAndUrl > should return the ID when found 2`] 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/return-dispatch.spec.ts b/src/return-dispatch.spec.ts index f5256101..3f844de1 100644 --- a/src/return-dispatch.spec.ts +++ b/src/return-dispatch.spec.ts @@ -694,8 +694,9 @@ 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).toHaveBeenCalledTimes(2); expect(utilSleepMock).toHaveBeenCalledWith(retryMs * 2); @@ -808,8 +809,9 @@ describe("return-dispatch", () => { 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(); @@ -825,14 +827,17 @@ describe("return-dispatch", () => { 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(retryMs * 3); + expect(utilSleepMock.mock.calls[2]?.[0]).toBeLessThanOrEqual(retryMs * 3); resetLogMocks(); await vi.advanceTimersByTimeAsync(retryMs * 3); From ae2037583377c2ad1012d24158bd7e47a8954b37 Mon Sep 17 00:00:00 2001 From: German Suarez Alonso Date: Thu, 9 Jan 2025 12:43:12 +0100 Subject: [PATCH 4/5] solve comments --- README.md | 2 ++ action.yml | 4 +++- src/return-dispatch.spec.ts | 6 +++++- src/return-dispatch.ts | 1 - 4 files changed, 10 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 1674c3b0..1069347f 100644 --- a/README.md +++ b/README.md @@ -25,6 +25,8 @@ steps: workflow: automation-test.yml workflow_inputs: '{ "some_input": "value" }' # Optional workflow_timeout_seconds: 120 # Default: 300 + # Lineal backoff retry attempts are made where the attempt count is + # the magnitude and the scaling value is `workflow_job_steps_retry_seconds` workflow_job_steps_retry_seconds: 10 # Default: 5 distinct_id: someDistinctId # Optional diff --git a/action.yml b/action.yml index 5a6fa8e5..ac8bc725 100644 --- a/action.yml +++ b/action.yml @@ -31,7 +31,9 @@ inputs: description: Time until giving up waiting for the start of the workflow run. default: 300 workflow_job_steps_retry_seconds: - description: Duration to wait between retries while monitoring for the workflow run to start. + description: | + The interval (in seconds) to wait between retries, each retry uses a linear backoff strategy. + 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/return-dispatch.spec.ts b/src/return-dispatch.spec.ts index 3f844de1..5aff81eb 100644 --- a/src/return-dispatch.spec.ts +++ b/src/return-dispatch.spec.ts @@ -837,7 +837,11 @@ describe("return-dispatch", () => { expect(coreDebugLogMock.mock.calls[0]?.[0]).toMatchSnapshot(); expect(utilSleepMock).toHaveBeenCalledTimes(3); - expect(utilSleepMock.mock.calls[2]?.[0]).toBeLessThanOrEqual(retryMs * 3); + 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(retryMs * 3); diff --git a/src/return-dispatch.ts b/src/return-dispatch.ts index 49241a00..a7d17836 100644 --- a/src/return-dispatch.ts +++ b/src/return-dispatch.ts @@ -146,7 +146,6 @@ export async function getRunIdAndUrl({ let attemptNo = 0; let elapsedTime = Date.now() - startTime; - while (elapsedTime < workflowTimeoutMs) { attemptNo++; From 7d72531daac72bde7e656ab0c9b71bbc8ca37815 Mon Sep 17 00:00:00 2001 From: Alex Miller Date: Fri, 10 Jan 2025 10:29:43 +1300 Subject: [PATCH 5/5] docs: reword a little --- README.md | 7 ++++--- action.yml | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 1069347f..2a4a89f9 100644 --- a/README.md +++ b/README.md @@ -25,9 +25,10 @@ steps: workflow: automation-test.yml workflow_inputs: '{ "some_input": "value" }' # Optional workflow_timeout_seconds: 120 # Default: 300 - # Lineal backoff retry attempts are made where the attempt count is - # the magnitude and the scaling value is `workflow_job_steps_retry_seconds` - workflow_job_steps_retry_seconds: 10 # Default: 5 + 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 index ac8bc725..08cef724 100644 --- a/action.yml +++ b/action.yml @@ -32,8 +32,8 @@ inputs: default: 300 workflow_job_steps_retry_seconds: description: | - The interval (in seconds) to wait between retries, each retry uses a linear backoff strategy. - Where the wait time increases by this value with each attempt (e.g., 1st retry = this value, 2nd retry = 2x this value, etc.). + 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.