Skip to content

Commit

Permalink
fix: respond to 404 errors when checking releases, and fall back to t…
Browse files Browse the repository at this point in the history
…ags if any errors (#171)
  • Loading branch information
maribethb authored Jul 6, 2022
1 parent 71d10be commit 89746a5
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 34 deletions.
30 changes: 22 additions & 8 deletions packages/server/src/lib/github.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,16 @@ export const getRelease = async (
token: string,
tags: string[]
): Promise<string | undefined> => {
const matchingRelease = await getReleaseForTags(name, token, tags);
if (matchingRelease) {
return matchingRelease;
try {
const matchingRelease = await getReleaseForTags(name, token, tags);
if (matchingRelease) {
return matchingRelease;
}
} catch (err) {
// Fall back to checking for matching tags instead of throwing the error.
console.error(
`Error while checking releases: ${err}. Checking tags instead.`
);
}
return getMatchingTags(name, token, tags);
};
Expand All @@ -93,13 +100,20 @@ const getReleaseForTags = async (
client.get(
`/repos/${name}/releases/tags/${tag}`,
{},
(err: Error, code: number, resp: {name: string}) => {
(err: {statusCode: number}, code: number, resp: {name: string}) => {
if (err) {
return reject(
Error(`getReleaseForTags: tag = ${tag}, err = ${err}`)
);
if (err.statusCode === 404) {
// A release matching this tag wasn't found. This isn't an error, just try the next tag in the list.
return resolve(undefined);
} else {
return reject(
Error(
`getReleaseForTags: tag = ${tag}, statusCode = ${err.statusCode}, err = ${err}`
)
);
}
}
resolve(resp.name || undefined);
resolve(resp?.name || undefined);
}
);
});
Expand Down
15 changes: 8 additions & 7 deletions packages/server/src/lib/write-package.ts
Original file line number Diff line number Diff line change
Expand Up @@ -298,13 +298,14 @@ async function enforceMatchingRelease(
const msg = `matching release v${newVersion} not found for ${repoName}. Did not find any tags matching: ${tags.join()}`;
throw new WombatServerError(msg, 400);
}
} catch (_err) {
// TODO(#158): This can swallow some errors that do have messages
const err = _err as {statusMessage: string; statusCode: number};
if (err.statusCode && err.statusMessage) throw err;
err.statusCode = 500;
err.statusMessage = 'unknown error';
throw err;
} catch (err) {
if (err instanceof WombatServerError) {
throw err;
}
if (err instanceof Error) {
throw new WombatServerError(err.message || 'unknown error', 500);
}
throw new WombatServerError('unknown error', 500);
}
}

Expand Down
26 changes: 19 additions & 7 deletions packages/server/test/lib/github.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ describe('github', () => {
it('returns release matching lerna-style tag', async () => {
const request = nock('https://api.github.com')
.get('/repos/bcoe/test/releases/tags/test-v1.0.2')
.reply(404, {})
.replyWithError({statusCode: 404})
.get('/repos/bcoe/test/releases/tags/@bcoe/test@1.0.2')
.reply(200, {name: '@bcoe/test@1.0.2'});

Expand All @@ -64,7 +64,19 @@ describe('github', () => {
it('returns matching tag from GitHub if no release found', async () => {
const request = nock('https://api.github.com')
.get('/repos/bcoe/test/releases/tags/v1.0.2')
.reply(404, {})
.replyWithError({statusCode: 404})
.get('/repos/bcoe/test/tags?per_page=100&page=1')
.reply(200, [{name: 'v1.0.2'}]);

const latest = await github.getRelease('bcoe/test', 'abc123', ['v1.0.2']);
expect(latest).to.equal('v1.0.2');
request.done();
});

it('checks tags if there is an error while checking releases', async () => {
const request = nock('https://api.github.com')
.get('/repos/bcoe/test/releases/tags/v1.0.2')
.replyWithError({statusCode: 500})
.get('/repos/bcoe/test/tags?per_page=100&page=1')
.reply(200, [{name: 'v1.0.2'}]);

Expand All @@ -76,7 +88,7 @@ describe('github', () => {
it('bubbles error appropriately', async () => {
const request = nock('https://api.github.com')
.get('/repos/bcoe/test/releases/tags/v1.0.2')
.reply(404, {})
.replyWithError({statusCode: 404})
.get('/repos/bcoe/test/tags?per_page=100&page=1')
.reply(404);
let err: Error | undefined = undefined;
Expand All @@ -95,9 +107,9 @@ describe('github', () => {
it('does not return latest tag without prefix, when monorepo-style used', async () => {
const request = nock('https://api.github.com')
.get('/repos/bcoe/test/releases/tags/foo-v1.0.2')
.reply(404, {})
.replyWithError({statusCode: 404})
.get('/repos/bcoe/test/releases/tags/@scope/foo@1.0.2')
.reply(404, {})
.replyWithError({statusCode: 404})
.get('/repos/bcoe/test/tags?per_page=100&page=1')
.reply(200, [{name: 'v1.0.2'}])
.get('/repos/bcoe/test/tags?per_page=100&page=2')
Expand Down Expand Up @@ -133,7 +145,7 @@ describe('github', () => {
it('returns latest tag matching monorepo style tag', async () => {
const request = nock('https://api.github.com')
.get('/repos/bcoe/test/releases/tags/foo-v1.0.2')
.reply(404, {})
.replyWithError({statusCode: 404})
.get('/repos/bcoe/test/tags?per_page=100&page=1')
.reply(200, [{name: 'v1.0.3'}])
.get('/repos/bcoe/test/tags?per_page=100&page=2')
Expand All @@ -151,7 +163,7 @@ describe('github', () => {
it('returns latest tag matching lerna style tag', async () => {
const request = nock('https://api.github.com')
.get('/repos/bcoe/test/releases/tags/@scope/foo@1.0.2')
.reply(404, {})
.replyWithError({statusCode: 404})
.get('/repos/bcoe/test/tags?per_page=100&page=1')
.reply(200, [{name: 'v1.0.3'}])
.get('/repos/bcoe/test/tags?per_page=100&page=2')
Expand Down
24 changes: 12 additions & 12 deletions packages/server/test/lib/write-package.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ describe('writePackage', () => {
.reply(200, {permissions: {push: true}})
// No releases on GitHub, only tags.
.get('/repos/foo/bar/releases/tags/v1.0.0')
.reply(404, {})
.replyWithError({statusCode: 404})
// most recent release tag on GitHub is v1.0.0
.get('/repos/foo/bar/tags?per_page=100&page=1')
.reply(200, [{name: 'v1.0.0'}]);
Expand Down Expand Up @@ -251,7 +251,7 @@ describe('writePackage', () => {
.reply(200, {permissions: {push: true}})
// No release on GitHub, only tags.
.get('/repos/foo/bar/releases/tags/v1.0.0')
.reply(404, {})
.replyWithError({statusCode: 404})
// most recent tag on GitHub is v1.0.0
.get('/repos/foo/bar/tags?per_page=100&page=1')
.reply(200, [{name: 'v1.0.0'}]);
Expand Down Expand Up @@ -365,7 +365,7 @@ describe('writePackage', () => {
.reply(200, {permissions: {push: true}})
// No matching release.
.get('/repos/foo/bar/releases/tags/v1.0.0')
.reply(404, {})
.replyWithError({statusCode: 404})
// most recent release tag on GitHub is v0.1.0
.get('/repos/foo/bar/tags?per_page=100&page=1')
.reply(200, [{name: 'v0.1.0'}])
Expand Down Expand Up @@ -445,15 +445,15 @@ describe('writePackage', () => {
.reply(200, {permissions: {push: true}})
// No matching release
.get('/repos/foo/bar/releases/tags/v1.0.0')
.reply(404, {})
.replyWithError({statusCode: 404})
// Error while fetching tags
.get('/repos/foo/bar/tags?per_page=100&page=1')
.reply(500);

const ret = await writePackage('@soldair/foo', req, res);
npmRequest.done();
githubRequest.done();
expect(ret.error).to.match(/unknown error/);
expect(ret.error).to.match(/Error 500/);
expect(ret.statusCode).to.equal(500);
});

Expand Down Expand Up @@ -564,7 +564,7 @@ describe('writePackage', () => {
.reply(200, {permissions: {push: true}})
// Matching release for lerna-style tag. No need to check additional tags.
.get('/repos/foo/bar/releases/tags/foo-v1.0.0')
.reply(404, {})
.replyWithError({statusCode: 404})
.get('/repos/foo/bar/releases/tags/@soldair/foo@1.0.0')
.reply(200, {name: '@soldair/foo@1.0.0'});

Expand Down Expand Up @@ -624,9 +624,9 @@ describe('writePackage', () => {
.reply(200, {permissions: {push: true}})
// No matching releases for either tag type.
.get('/repos/foo/bar/releases/tags/foo-v1.0.0')
.reply(404, {})
.replyWithError({statusCode: 404})
.get('/repos/foo/bar/releases/tags/@soldair/foo@1.0.0')
.reply(404, {})
.replyWithError({statusCode: 404})
// But there is a matching tag for v1.0.0
.get('/repos/foo/bar/tags?per_page=100&page=1')
.reply(200, [{name: 'foo-v1.0.0'}]);
Expand Down Expand Up @@ -687,9 +687,9 @@ describe('writePackage', () => {
.reply(200, {permissions: {push: true}})
// No matching releases matching either style.
.get('/repos/foo/bar/releases/tags/foo-v1.0.0')
.reply(404, {})
.replyWithError({statusCode: 404})
.get('/repos/foo/bar/releases/tags/@soldair/foo@1.0.0')
.reply(404, {})
.replyWithError({statusCode: 404})
// most recent release tag on GitHub is v1.0.0
.get('/repos/foo/bar/tags?per_page=100&page=1')
.reply(200, [{name: '@soldair/foo@1.0.0'}]);
Expand Down Expand Up @@ -750,9 +750,9 @@ describe('writePackage', () => {
.reply(200, {permissions: {push: true}})
// No matching releases for either tag style.
.get('/repos/foo/bar/releases/tags/foo-v1.0.0')
.reply(404, {})
.replyWithError({statusCode: 404})
.get('/repos/foo/bar/releases/tags/@soldair/foo@1.0.0')
.reply(404, {})
.replyWithError({statusCode: 404})
// This is monorepo-style token but the tags on GH are not monorepo-style
.get('/repos/foo/bar/tags?per_page=100&page=1')
.reply(200, [{name: 'v1.0.0'}])
Expand Down

0 comments on commit 89746a5

Please sign in to comment.