Skip to content

Commit

Permalink
fix: internal retry should not fail action before the input timeout i…
Browse files Browse the repository at this point in the history
…s exceeded
  • Loading branch information
Codex- committed Sep 26, 2024
1 parent 3003423 commit d8c15b5
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 20 deletions.
46 changes: 31 additions & 15 deletions src/api.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
getWorkflowRunJobSteps,
getWorkflowRunUrl,
init,
retryOrDie,
retryOrTimeout,
} from "./api.ts";

vi.mock("@actions/core");
Expand Down Expand Up @@ -427,7 +427,7 @@ describe("API", () => {
});
});

describe("retryOrDie", () => {
describe("retryOrTimeout", () => {
beforeEach(() => {
vi.useFakeTimers();
});
Expand All @@ -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);
});
});
Expand Down
19 changes: 15 additions & 4 deletions src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -235,25 +235,36 @@ export async function getWorkflowRunJobSteps(runId: number): Promise<string[]> {
}
}

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 };
}
10 changes: 9 additions & 1 deletion src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,21 @@ 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,
);
if (fetchWorkflowRunIds.timeout) {
core.debug("Timed out while attempting to fetch Workflow Run IDs");
await new Promise((resolve) =>
setTimeout(resolve, WORKFLOW_JOB_STEPS_RETRY_MS),
);
continue;
}

const workflowRunIds = fetchWorkflowRunIds.value;
core.debug(
`Attempting to get step names for Run IDs: [${workflowRunIds.join(", ")}]`,
);
Expand Down

0 comments on commit d8c15b5

Please sign in to comment.