Skip to content

Commit 54baeea

Browse files
priyagupta108priyagupta108
authored
Validate and retry manifest fetch to prevent silent failures (#1332)
* validate and retry manifest fetch * Refactor error handling in isRateLimitError function for improved clarity
1 parent c709277 commit 54baeea

3 files changed

Lines changed: 200 additions & 22 deletions

File tree

‎__tests__/install-python.test.ts‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,21 +31,81 @@ describe('getManifest', () => {
3131
jest.resetAllMocks();
3232
});
3333

34+
afterEach(() => {
35+
jest.useRealTimers();
36+
});
37+
3438
it('should return manifest from repo', async () => {
3539
(tc.getManifestFromRepo as jest.Mock).mockResolvedValue(mockManifest);
3640
const manifest = await getManifest();
3741
expect(manifest).toEqual(mockManifest);
3842
});
3943

4044
it('should return manifest from URL if repo fetch fails', async () => {
45+
jest.useFakeTimers();
4146
(tc.getManifestFromRepo as jest.Mock).mockRejectedValue(
4247
new Error('Fetch failed')
4348
);
4449
(httpm.HttpClient.prototype.getJson as jest.Mock).mockResolvedValue({
4550
result: mockManifest
4651
});
52+
const promise = getManifest();
53+
await jest.runAllTimersAsync();
54+
const manifest = await promise;
55+
expect(manifest).toEqual(mockManifest);
56+
});
57+
58+
it('should fall back to URL if repo returns a truncated/empty manifest', async () => {
59+
jest.useFakeTimers();
60+
(tc.getManifestFromRepo as jest.Mock).mockResolvedValue([]);
61+
(httpm.HttpClient.prototype.getJson as jest.Mock).mockResolvedValue({
62+
result: mockManifest
63+
});
64+
const promise = getManifest();
65+
await jest.runAllTimersAsync();
66+
const manifest = await promise;
67+
expect(manifest).toEqual(mockManifest);
68+
});
69+
70+
it('should retry on a transient invalid manifest and then succeed', async () => {
71+
jest.useFakeTimers();
72+
(tc.getManifestFromRepo as jest.Mock)
73+
.mockResolvedValueOnce([])
74+
.mockResolvedValueOnce(mockManifest);
75+
const promise = getManifest();
76+
await jest.runAllTimersAsync();
77+
const manifest = await promise;
78+
expect(manifest).toEqual(mockManifest);
79+
expect(tc.getManifestFromRepo as jest.Mock).toHaveBeenCalledTimes(2);
80+
});
81+
82+
it('should fail loudly when the manifest is truncated/empty on every source', async () => {
83+
jest.useFakeTimers();
84+
(tc.getManifestFromRepo as jest.Mock).mockResolvedValue([]);
85+
(httpm.HttpClient.prototype.getJson as jest.Mock).mockResolvedValue({
86+
result: []
87+
});
88+
const promise = getManifest();
89+
// Prevent unhandled rejection before timers advance.
90+
const catchPromise = promise.catch(() => {});
91+
await jest.runAllTimersAsync();
92+
await catchPromise;
93+
await expect(promise).rejects.toThrow(
94+
'Failed to fetch the Python versions manifest'
95+
);
96+
});
97+
98+
it('should not retry the API on a rate-limit error and fall back to URL immediately', async () => {
99+
const rateLimitError = Object.assign(new Error('API rate limit exceeded'), {
100+
httpStatusCode: 403
101+
});
102+
(tc.getManifestFromRepo as jest.Mock).mockRejectedValue(rateLimitError);
103+
(httpm.HttpClient.prototype.getJson as jest.Mock).mockResolvedValue({
104+
result: mockManifest
105+
});
47106
const manifest = await getManifest();
48107
expect(manifest).toEqual(mockManifest);
108+
expect(tc.getManifestFromRepo as jest.Mock).toHaveBeenCalledTimes(1);
49109
});
50110
});
51111

‎dist/setup/index.js‎

Lines changed: 57 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -55685,6 +55685,8 @@ function findRhelRelease(semanticVersionSpec, architecture, manifest, osVersion)
5568555685
}
5568655686
return undefined;
5568755687
}
55688+
const MANIFEST_FETCH_MAX_ATTEMPTS = 3;
55689+
const MANIFEST_FETCH_RETRY_BASE_DELAY_MS = 1000;
5568855690
async function findReleaseFromManifest(semanticVersionSpec, architecture, manifest) {
5568955691
if (!manifest) {
5569055692
manifest = await getManifest();
@@ -55712,26 +55714,72 @@ function isIToolRelease(obj) {
5571255714
typeof file.arch === 'string' &&
5571355715
typeof file.download_url === 'string'));
5571455716
}
55717+
// Rejects empty or truncated manifest responses.
55718+
function isValidManifest(manifest) {
55719+
return (Array.isArray(manifest) &&
55720+
manifest.length > 0 &&
55721+
manifest.every(isIToolRelease));
55722+
}
55723+
function sleep(ms) {
55724+
return new Promise(resolve => setTimeout(resolve, ms));
55725+
}
55726+
// HTTP 403/429 from http-client (`statusCode`) or tool-cache (`httpStatusCode`).
55727+
function isRateLimitError(err) {
55728+
const e = err;
55729+
const status = e?.httpStatusCode ?? e?.statusCode;
55730+
return status === 403 || status === 429;
55731+
}
55732+
// Fetches and validates a manifest, retrying transient failures with backoff.
55733+
async function fetchValidManifest(source, fetcher) {
55734+
let lastError;
55735+
let attempts = 0;
55736+
for (let attempt = 1; attempt <= MANIFEST_FETCH_MAX_ATTEMPTS; attempt++) {
55737+
attempts = attempt;
55738+
try {
55739+
const manifest = await fetcher();
55740+
if (isValidManifest(manifest)) {
55741+
return manifest;
55742+
}
55743+
throw new Error(`The manifest fetched from ${source} is empty, truncated, or does not contain any valid tool release entries.`);
55744+
}
55745+
catch (err) {
55746+
lastError = err instanceof Error ? err : new Error(String(err));
55747+
core.debug(`Attempt ${attempt}/${MANIFEST_FETCH_MAX_ATTEMPTS} to fetch the manifest from ${source} failed: ${lastError.message}`);
55748+
// Rate limits won't clear within the backoff window; fall back instead.
55749+
if (isRateLimitError(err)) {
55750+
core.debug(`${source} is rate-limited; skipping retries for this source.`);
55751+
break;
55752+
}
55753+
if (attempt < MANIFEST_FETCH_MAX_ATTEMPTS) {
55754+
const delay = MANIFEST_FETCH_RETRY_BASE_DELAY_MS * 2 ** (attempt - 1);
55755+
core.debug(`Retrying in ${delay}ms...`);
55756+
await sleep(delay);
55757+
}
55758+
}
55759+
}
55760+
throw new Error(`Failed to fetch a valid manifest from ${source} after ${attempts} attempt(s): ${lastError?.message}`);
55761+
}
5571555762
async function getManifest() {
5571655763
try {
55717-
const repoManifest = await getManifestFromRepo();
55718-
if (Array.isArray(repoManifest) &&
55719-
repoManifest.length &&
55720-
repoManifest.every(isIToolRelease)) {
55721-
return repoManifest;
55722-
}
55723-
throw new Error('The repository manifest is invalid or does not include any valid tool release (IToolRelease) entries.');
55764+
return await fetchValidManifest('the GitHub API', getManifestFromRepo);
5572455765
}
5572555766
catch (err) {
5572655767
core.debug('Fetching the manifest via the API failed.');
5572755768
if (err instanceof Error) {
5572855769
core.debug(err.message);
5572955770
}
5573055771
else {
55731-
core.error('An unexpected error occurred while fetching the manifest.');
55772+
core.debug('An unexpected error occurred while fetching the manifest.');
5573255773
}
5573355774
}
55734-
return await getManifestFromURL();
55775+
try {
55776+
return await fetchValidManifest('the raw URL', getManifestFromURL);
55777+
}
55778+
catch (err) {
55779+
const message = err instanceof Error ? err.message : String(err);
55780+
// Fail loudly so the action doesn't exit 0 without installing Python.
55781+
throw new Error(`Failed to fetch the Python versions manifest. The response was empty, truncated, or invalid, and all retries were exhausted. ${message}`);
55782+
}
5573555783
}
5573655784
function getManifestFromRepo() {
5573755785
core.debug(`Getting manifest from ${MANIFEST_REPO_OWNER}/${MANIFEST_REPO_NAME}@${MANIFEST_REPO_BRANCH}`);

‎src/install-python.ts‎

Lines changed: 83 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,9 @@ function findRhelRelease(
8080
return undefined;
8181
}
8282

83+
const MANIFEST_FETCH_MAX_ATTEMPTS = 3;
84+
const MANIFEST_FETCH_RETRY_BASE_DELAY_MS = 1000;
85+
8386
export async function findReleaseFromManifest(
8487
semanticVersionSpec: string,
8588
architecture: string,
@@ -133,28 +136,95 @@ function isIToolRelease(obj: any): obj is IToolRelease {
133136
);
134137
}
135138

139+
// Rejects empty or truncated manifest responses.
140+
function isValidManifest(manifest: unknown): manifest is tc.IToolRelease[] {
141+
return (
142+
Array.isArray(manifest) &&
143+
manifest.length > 0 &&
144+
manifest.every(isIToolRelease)
145+
);
146+
}
147+
148+
function sleep(ms: number): Promise<void> {
149+
return new Promise(resolve => setTimeout(resolve, ms));
150+
}
151+
152+
// HTTP 403/429 from http-client (`statusCode`) or tool-cache (`httpStatusCode`).
153+
function isRateLimitError(err: unknown): boolean {
154+
const e = err as
155+
| {httpStatusCode?: number; statusCode?: number}
156+
| null
157+
| undefined;
158+
const status = e?.httpStatusCode ?? e?.statusCode;
159+
return status === 403 || status === 429;
160+
}
161+
162+
// Fetches and validates a manifest, retrying transient failures with backoff.
163+
async function fetchValidManifest(
164+
source: string,
165+
fetcher: () => Promise<tc.IToolRelease[]>
166+
): Promise<tc.IToolRelease[]> {
167+
let lastError: Error | undefined;
168+
let attempts = 0;
169+
170+
for (let attempt = 1; attempt <= MANIFEST_FETCH_MAX_ATTEMPTS; attempt++) {
171+
attempts = attempt;
172+
try {
173+
const manifest = await fetcher();
174+
if (isValidManifest(manifest)) {
175+
return manifest;
176+
}
177+
throw new Error(
178+
`The manifest fetched from ${source} is empty, truncated, or does not contain any valid tool release entries.`
179+
);
180+
} catch (err) {
181+
lastError = err instanceof Error ? err : new Error(String(err));
182+
core.debug(
183+
`Attempt ${attempt}/${MANIFEST_FETCH_MAX_ATTEMPTS} to fetch the manifest from ${source} failed: ${lastError.message}`
184+
);
185+
186+
// Rate limits won't clear within the backoff window; fall back instead.
187+
if (isRateLimitError(err)) {
188+
core.debug(
189+
`${source} is rate-limited; skipping retries for this source.`
190+
);
191+
break;
192+
}
193+
194+
if (attempt < MANIFEST_FETCH_MAX_ATTEMPTS) {
195+
const delay = MANIFEST_FETCH_RETRY_BASE_DELAY_MS * 2 ** (attempt - 1);
196+
core.debug(`Retrying in ${delay}ms...`);
197+
await sleep(delay);
198+
}
199+
}
200+
}
201+
202+
throw new Error(
203+
`Failed to fetch a valid manifest from ${source} after ${attempts} attempt(s): ${lastError?.message}`
204+
);
205+
}
206+
136207
export async function getManifest(): Promise<tc.IToolRelease[]> {
137208
try {
138-
const repoManifest = await getManifestFromRepo();
139-
if (
140-
Array.isArray(repoManifest) &&
141-
repoManifest.length &&
142-
repoManifest.every(isIToolRelease)
143-
) {
144-
return repoManifest;
145-
}
146-
throw new Error(
147-
'The repository manifest is invalid or does not include any valid tool release (IToolRelease) entries.'
148-
);
209+
return await fetchValidManifest('the GitHub API', getManifestFromRepo);
149210
} catch (err) {
150211
core.debug('Fetching the manifest via the API failed.');
151212
if (err instanceof Error) {
152213
core.debug(err.message);
153214
} else {
154-
core.error('An unexpected error occurred while fetching the manifest.');
215+
core.debug('An unexpected error occurred while fetching the manifest.');
155216
}
156217
}
157-
return await getManifestFromURL();
218+
219+
try {
220+
return await fetchValidManifest('the raw URL', getManifestFromURL);
221+
} catch (err) {
222+
const message = err instanceof Error ? err.message : String(err);
223+
// Fail loudly so the action doesn't exit 0 without installing Python.
224+
throw new Error(
225+
`Failed to fetch the Python versions manifest. The response was empty, truncated, or invalid, and all retries were exhausted. ${message}`
226+
);
227+
}
158228
}
159229

160230
export function getManifestFromRepo(): Promise<tc.IToolRelease[]> {

0 commit comments

Comments
 (0)