Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
20 changes: 14 additions & 6 deletions src/__snapshots__/return-dispatch.spec.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -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]"`;
11 changes: 11 additions & 0 deletions src/action.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
};

Expand All @@ -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:
Expand All @@ -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");
});

Expand All @@ -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();
Expand Down
9 changes: 9 additions & 0 deletions src/action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
*/
Expand All @@ -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(),
};
Expand Down
3 changes: 3 additions & 0 deletions src/api.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 "";
}
Expand Down Expand Up @@ -332,6 +334,7 @@ describe("API", () => {
workflow: "workflow_name",
workflowInputs: { testInput: "test" },
workflowTimeoutSeconds: 60,
workflowJobStepsRetrySeconds: 3,
distinctId: "test-uuid",
};

Expand Down
1 change: 0 additions & 1 deletion src/constants.ts
Original file line number Diff line number Diff line change
@@ -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;
2 changes: 2 additions & 0 deletions src/main.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -173,6 +174,7 @@ describe("main", () => {
distinctIdRegex: distinctIdRegex,
workflowId: 0,
workflowTimeoutMs: testCfg.workflowTimeoutSeconds * 1000,
workflowJobStepsRetryMs: testCfg.workflowJobStepsRetrySeconds * 1000,
});

// Result
Expand Down
1 change: 1 addition & 0 deletions src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
60 changes: 37 additions & 23 deletions src/return-dispatch.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -480,6 +480,7 @@ describe("return-dispatch", () => {
distinctIdRegex: distinctIdRegex,
workflowId: workflowId,
workflowTimeoutMs: 100,
workflowJobStepsRetryMs: 5,
};

let apiFetchWorkflowRunIdsMock: MockInstance<
Expand Down Expand Up @@ -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
Expand All @@ -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);
Expand Down Expand Up @@ -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
Expand All @@ -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;
Expand Down
11 changes: 10 additions & 1 deletion src/return-dispatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lineal backoff makes sense to me, but does change the behaviour people expect when setting workflow_job_steps_retry_seconds. I'm generally inclined to try follow the principle of least surprise.

If you, as a user, specify workflow_job_steps_retry_seconds to be 10 seconds and then find your workflow is taking 60 seconds for 3 attempts, would this surprise you?

You may expect this to behave such that:

  • Attempt 1: 10s
  • Attempt 2: 10s
  • Attempt 3: 10s

But implicitly using lineal backoff with the attempt count as the magnitude

  • Attempt 1: 10s
  • Attempt 2: 20s
  • Attempt 3: 30s

Perhaps for now just noting this behaviour in the readme will be enough:

...
      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
...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed in the readme and in the action description too

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;
}

Expand Down
Loading