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..2a4a89f9 100644 --- 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 index e1a9cbb4..08cef724 100644 --- 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 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/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..7ac5254a 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..9ad5a8c6 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..5aff81eb 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 @@ -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 index 64010fd7..a7d17836 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,6 +137,7 @@ export async function getRunIdAndUrl({ distinctIdRegex, workflowId, workflowTimeoutMs, + workflowJobStepsRetryMs, }: GetRunIdAndUrlOpts): Promise> { 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; }