Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Give release archive workflows more time #170

Merged
merged 5 commits into from
Oct 11, 2024
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
3 changes: 3 additions & 0 deletions e2e/e2e.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ import { FakeSecrets } from "./stubs/fake-secrets";

jest.setTimeout(30000);

// Speed up e2e tests by retrying failed requests with 100ms delay factor.
process.env.BACKOFF_DELAY_FACTOR = "100";

describe("e2e tests", () => {
let cloudFunctions: CloudFunctions;
let fakeGitHub: FakeGitHub;
Expand Down
15 changes: 15 additions & 0 deletions jest.setup.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { MatcherFunction } from 'expect'

declare global {
namespace jest {
interface Matchers<R> {
Expand All @@ -6,6 +8,12 @@ declare global {
message: string
): R;
}
interface Expect {
matchesPredicate(predicate: (actual: any) => boolean): any;
}
interface ExpectExtendMap {
matchesPredicate: MatcherFunction<[predicate: (actual: any) => boolean]>;
}
}
}

Expand Down Expand Up @@ -59,6 +67,13 @@ But instead it was:
message: () => "Expected function to throw but it did not",
};
},

matchesPredicate(actual: any, predicate: (actual: any) => boolean) {
return {
pass: predicate(actual),
message: () => "Expected object that passes predicate",
};
},
});

export default undefined;
22 changes: 21 additions & 1 deletion src/domain/release-archive.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import fs, { WriteStream } from "node:fs";
import os from "node:os";
import path from "node:path";
import tar from "tar";
import "../../jest.setup";
import { fakeModuleFile } from "../test/mock-template-files";
import { expectThrownError } from "../test/util";
import {
Expand Down Expand Up @@ -66,11 +67,30 @@ describe("fetch", () => {
});

test("retries the request if it fails", async () => {
// Restore the original behavior of exponentialDelay.
mocked(axiosRetry.exponentialDelay).mockImplementation(
jest.requireActual("axios-retry").exponentialDelay
);

await ReleaseArchive.fetch(RELEASE_ARCHIVE_URL, STRIP_PREFIX);

expect(axiosRetry).toHaveBeenCalledWith(axios, {
retries: 3,
retryDelay: axiosRetry.exponentialDelay,
retryDelay: expect.matchesPredicate((retryDelayFn: Function) => {
// Make sure the retry delays follow exponential backoff
// and the final retry happens after at least 1 minute total
// (in this case, at least 70 seconds).
// Axios randomly adds an extra 0-20% of jitter to each delay.
// Test upper bounds as well to ensure the workflow completes reasonably quickly
// (in this case, no more than 84 seconds total).
let firstRetryDelay = retryDelayFn.call(this, 0);
let secondRetryDelay = retryDelayFn.call(this, 1);
let thirdRetryDelay = retryDelayFn.call(this, 2);
return 10000 <= firstRetryDelay && firstRetryDelay <= 12000
&& 20000 <= secondRetryDelay && secondRetryDelay <= 24000
&& 40000 <= thirdRetryDelay && thirdRetryDelay <= 48000;
}),
shouldResetTimeout: true,
});
});

Expand Down
18 changes: 15 additions & 3 deletions src/domain/release-archive.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import axios, { AxiosResponse } from "axios";
import axios, { AxiosError, AxiosResponse } from "axios";
import axiosRetry from "axios-retry";
import extractZip from "extract-zip";
import fs from "node:fs";
Expand Down Expand Up @@ -108,6 +108,15 @@ export class ReleaseArchive {
}
}

function exponentialDelay(
retryCount: number,
error: AxiosError | undefined
): number {
// Default delay factor is 10 seconds, but can be overridden for testing.
const delayFactor = Number(process.env.BACKOFF_DELAY_FACTOR) || 10_000;
return axiosRetry.exponentialDelay(retryCount, error, delayFactor);
}

async function download(url: string, dest: string): Promise<void> {
if (process.env.INTEGRATION_TESTING) {
// Point downloads to the standin github server
Expand All @@ -124,10 +133,13 @@ async function download(url: string, dest: string): Promise<void> {

const writer = fs.createWriteStream(dest, { flags: "w" });

// Retry the request in case the artifact is still being uploaded
// Retry the request in case the artifact is still being uploaded.
// Exponential backoff with 3 retries and a delay factor of 10 seconds
// gives you at least 70 seconds to upload a release archive.
axiosRetry(axios, {
retries: 3,
retryDelay: axiosRetry.exponentialDelay,
retryDelay: exponentialDelay,
shouldResetTimeout: true,
});

let response: AxiosResponse;
Expand Down