From 04a674188d63371932f16fe3f7eea8c22a52ede6 Mon Sep 17 00:00:00 2001 From: Omer Mishania Date: Tue, 18 Nov 2025 19:53:54 +0200 Subject: [PATCH] refactor: enhance release retrieval with pagination safeguards - Introduced a new function `findTagByPagination` to handle release retrieval with pagination, limiting the number of pages checked to avoid hitting GitHub's result limit. - Updated `findTagFromReleases` to first attempt a direct lookup by tag, falling back to pagination if necessary. - Improved error handling for 404 and other errors during release lookups. - Updated test cases to reflect changes in release retrieval logic and error handling. --- __tests__/github.test.ts | 238 +++++++++++++++++++++------------------ src/github.ts | 87 ++++++++++++-- 2 files changed, 204 insertions(+), 121 deletions(-) diff --git a/__tests__/github.test.ts b/__tests__/github.test.ts index 24bd9da..dc880b0 100644 --- a/__tests__/github.test.ts +++ b/__tests__/github.test.ts @@ -57,26 +57,15 @@ describe('github', () => { describe('when the tag_name is not an empty string', () => { const targetTag = 'v1.0.0'; - it('finds a matching release in first batch of results', async () => { + it('finds a release using getReleaseByTag directly', async () => { const targetRelease = { ...mockRelease, - owner, - repo, tag_name: targetTag, }; - const otherRelease = { - ...mockRelease, - owner, - repo, - tag_name: 'v1.0.1', - }; const releaser = { ...mockReleaser, - allReleases: async function* () { - yield { data: [targetRelease] }; - yield { data: [otherRelease] }; - }, + getReleaseByTag: async () => ({ data: targetRelease }), }; const result = await findTagFromReleases(releaser, owner, repo, targetTag); @@ -84,44 +73,13 @@ describe('github', () => { assert.deepStrictEqual(result, targetRelease); }); - it('finds a matching release in second batch of results', async () => { - const targetRelease = { - ...mockRelease, - owner, - repo, - tag_name: targetTag, - }; - const otherRelease = { - ...mockRelease, - owner, - repo, - tag_name: 'v1.0.1', - }; - + it('returns undefined when getReleaseByTag returns 404', async () => { const releaser = { ...mockReleaser, - allReleases: async function* () { - yield { data: [otherRelease] }; - yield { data: [targetRelease] }; - }, - }; - - const result = await findTagFromReleases(releaser, owner, repo, targetTag); - assert.deepStrictEqual(result, targetRelease); - }); - - it('returns undefined when a release is not found in any batch', async () => { - const otherRelease = { - ...mockRelease, - owner, - repo, - tag_name: 'v1.0.1', - }; - const releaser = { - ...mockReleaser, - allReleases: async function* () { - yield { data: [otherRelease] }; - yield { data: [otherRelease] }; + getReleaseByTag: async () => { + const error: any = new Error('Not found'); + error.status = 404; + throw error; }, }; @@ -130,11 +88,86 @@ describe('github', () => { assert.strictEqual(result, undefined); }); - it('returns undefined when no releases are returned', async () => { + it('falls back to pagination when getReleaseByTag fails with non-404 error', async () => { + const targetRelease = { + ...mockRelease, + owner, + repo, + tag_name: targetTag, + }; + const otherRelease = { + ...mockRelease, + owner, + repo, + tag_name: 'v1.0.1', + }; + const releaser = { ...mockReleaser, + getReleaseByTag: async () => { + const error: any = new Error('Server error'); + error.status = 500; + throw error; + }, allReleases: async function* () { - yield { data: [] }; + yield { data: [targetRelease] }; + yield { data: [otherRelease] }; + }, + }; + + const result = await findTagFromReleases(releaser, owner, repo, targetTag); + + assert.deepStrictEqual(result, targetRelease); + }); + + it('finds a matching release in second batch of results when falling back to pagination', async () => { + const targetRelease = { + ...mockRelease, + owner, + repo, + tag_name: targetTag, + }; + const otherRelease = { + ...mockRelease, + owner, + repo, + tag_name: 'v1.0.1', + }; + + const releaser = { + ...mockReleaser, + getReleaseByTag: async () => { + const error: any = new Error('Server error'); + error.status = 500; + throw error; + }, + allReleases: async function* () { + yield { data: [otherRelease] }; + yield { data: [targetRelease] }; + }, + }; + + const result = await findTagFromReleases(releaser, owner, repo, targetTag); + assert.deepStrictEqual(result, targetRelease); + }); + + it('returns undefined when a release is not found in any batch during pagination fallback', async () => { + const otherRelease = { + ...mockRelease, + owner, + repo, + tag_name: 'v1.0.1', + }; + const releaser = { + ...mockReleaser, + getReleaseByTag: async () => { + const error: any = new Error('Server error'); + error.status = 500; + throw error; + }, + allReleases: async function* () { + yield { data: [otherRelease] }; + yield { data: [otherRelease] }; }, }; @@ -147,7 +180,38 @@ describe('github', () => { describe('when the tag_name is an empty string', () => { const emptyTag = ''; - it('finds a matching release in first batch of results', async () => { + it('finds a release using getReleaseByTag directly', async () => { + const targetRelease = { + ...mockRelease, + tag_name: emptyTag, + }; + + const releaser = { + ...mockReleaser, + getReleaseByTag: async () => ({ data: targetRelease }), + }; + + const result = await findTagFromReleases(releaser, owner, repo, emptyTag); + + assert.deepStrictEqual(result, targetRelease); + }); + + it('returns undefined when getReleaseByTag returns 404', async () => { + const releaser = { + ...mockReleaser, + getReleaseByTag: async () => { + const error: any = new Error('Not found'); + error.status = 404; + throw error; + }, + }; + + const result = await findTagFromReleases(releaser, owner, repo, emptyTag); + + assert.strictEqual(result, undefined); + }); + + it('falls back to pagination when getReleaseByTag fails with non-404 error', async () => { const targetRelease = { ...mockRelease, owner, @@ -163,6 +227,11 @@ describe('github', () => { const releaser = { ...mockReleaser, + getReleaseByTag: async () => { + const error: any = new Error('Server error'); + error.status = 500; + throw error; + }, allReleases: async function* () { yield { data: [targetRelease] }; yield { data: [otherRelease] }; @@ -173,72 +242,17 @@ describe('github', () => { assert.deepStrictEqual(result, targetRelease); }); - - it('finds a matching release in second batch of results', async () => { - const targetRelease = { - ...mockRelease, - owner, - repo, - tag_name: emptyTag, - }; - const otherRelease = { - ...mockRelease, - owner, - repo, - tag_name: 'v1.0.1', - }; - - const releaser = { - ...mockReleaser, - allReleases: async function* () { - yield { data: [otherRelease] }; - yield { data: [targetRelease] }; - }, - }; - - const result = await findTagFromReleases(releaser, owner, repo, emptyTag); - assert.deepStrictEqual(result, targetRelease); - }); - - it('returns undefined when a release is not found in any batch', async () => { - const otherRelease = { - ...mockRelease, - owner, - repo, - tag_name: 'v1.0.1', - }; - const releaser = { - ...mockReleaser, - allReleases: async function* () { - yield { data: [otherRelease] }; - yield { data: [otherRelease] }; - }, - }; - - const result = await findTagFromReleases(releaser, owner, repo, emptyTag); - - assert.strictEqual(result, undefined); - }); - - it('returns undefined when no releases are returned', async () => { - const releaser = { - ...mockReleaser, - allReleases: async function* () { - yield { data: [] }; - }, - }; - - const result = await findTagFromReleases(releaser, owner, repo, emptyTag); - - assert.strictEqual(result, undefined); - }); }); }); describe('error handling', () => { it('handles 422 already_exists error gracefully', async () => { const mockReleaser: Releaser = { - getReleaseByTag: () => Promise.reject('Not implemented'), + getReleaseByTag: async () => { + const error: any = new Error('Not found'); + error.status = 404; + throw error; + }, createRelease: () => Promise.reject({ status: 422, diff --git a/src/github.ts b/src/github.ts index 9d59ed8..033f284 100644 --- a/src/github.ts +++ b/src/github.ts @@ -345,6 +345,63 @@ export const release = async ( } }; +/** + * Paginates through releases with safeguards to avoid hitting GitHub's 10,000 result limit. + * Stops early if encountering too many consecutive empty pages. + * + * @param releaser - The GitHub API wrapper for release operations + * @param owner - The owner of the repository + * @param repo - The name of the repository + * @param tag - The tag name to search for + * @returns The release with the given tag name, or undefined if no release with that tag name is found + */ +async function findTagByPagination( + releaser: Releaser, + owner: string, + repo: string, + tag: string, +): Promise { + // Limit pagination to avoid hitting GitHub's 10,000 result limit + // Stop aggressively on empty pages to prevent CI blocking + let pageCount = 0; + const maxPages = 30; // Stop after 30 pages (3000 releases max) to avoid hitting limits + const minPagesBeforeEmptyPageStop = 5; // After checking at least 5 pages, stop immediately on first empty page + + for await (const { data: releases } of releaser.allReleases({ + owner, + repo, + })) { + pageCount++; + + // Stop if we've checked too many pages + if (pageCount > maxPages) { + console.warn( + `⚠️ Stopped pagination after ${maxPages} pages to avoid hitting GitHub's result limit`, + ); + break; + } + + // If we get an empty page, stop immediately if we've already checked enough pages + // This prevents getting stuck on empty pages (like pages 300-1000) which blocks CI + if (releases.length === 0) { + if (pageCount >= minPagesBeforeEmptyPageStop) { + console.log( + `Stopped pagination after encountering empty page at page ${pageCount} (to avoid hitting GitHub's result limit)`, + ); + break; + } + // If we haven't checked many pages yet, continue (might be at the very end) + continue; + } + + const release = releases.find((release) => release.tag_name === tag); + if (release) { + return release; + } + } + return undefined; +} + /** * Finds a release by tag name from all a repository's releases. * @@ -360,16 +417,28 @@ export async function findTagFromReleases( repo: string, tag: string, ): Promise { - for await (const { data: releases } of releaser.allReleases({ - owner, - repo, - })) { - const release = releases.find((release) => release.tag_name === tag); - if (release) { - return release; - } + // If tag is empty, skip direct lookup and go straight to pagination + // (some releases may not have tags) + if (!tag) { + return await findTagByPagination(releaser, owner, repo, tag); + } + + // First try to get the release directly by tag (much more efficient than paginating) + try { + const { data } = await releaser.getReleaseByTag({ owner, repo, tag }); + return data; + } catch (error: any) { + // If the release doesn't exist (404), return undefined + // For other errors, fall back to pagination as a safety measure + if (error.status === 404) { + return undefined; + } + // For non-404 errors, fall back to pagination (though this should rarely happen) + console.warn( + `⚠️ Direct tag lookup failed (status: ${error.status}), falling back to pagination...`, + ); + return await findTagByPagination(releaser, owner, repo, tag); } - return undefined; } async function createRelease(