Skip to content
Snippets Groups Projects
Commit 66230471 authored by Rhys Arkins's avatar Rhys Arkins
Browse files

fix: add npm retries for null versions or parseError

parent eebef765
No related branches found
No related tags found
No related merge requests found
...@@ -110,8 +110,15 @@ async function getDependency(name, retries = 5) { ...@@ -110,8 +110,15 @@ async function getDependency(name, retries = 5) {
headers, headers,
})).body; })).body;
if (!res.versions || !Object.keys(res.versions).length) { if (!res.versions || !Object.keys(res.versions).length) {
// Registry returned a 200 OK but with no versions
if (retries <= 0) {
logger.info({ name }, 'No versions returned');
return null; return null;
} }
logger.info('No versions returned, retrying');
await delay(5000 / retries);
return getDependency(name, 0);
}
// Determine repository URL // Determine repository URL
let repositoryUrl; let repositoryUrl;
if (res.repository) { if (res.repository) {
...@@ -142,19 +149,24 @@ async function getDependency(name, retries = 5) { ...@@ -142,19 +149,24 @@ async function getDependency(name, retries = 5) {
return dep; return dep;
} catch (err) { } catch (err) {
if (err.statusCode === 401) { if (err.statusCode === 401) {
logger.info({ err, name }, `Dependency lookup unauthorized`); logger.info({ err, name }, `Dependency lookup failure: unauthorized`);
return null; return null;
} }
if (err.statusCode === 404) { if (err.statusCode === 404) {
logger.info({ name }, `Dependency not found`); logger.info({ name }, `Dependency lookup failure: not found`);
logger.debug({ err }); logger.debug({ err });
return null; return null;
} }
if (err.name === 'ParseError') { if (err.name === 'ParseError') {
// Registry returned a 200 OK but got failed to parse it // Registry returned a 200 OK but got failed to parse it
if (retries <= 0) {
logger.warn({ err }, 'npm registry failure: ParseError'); logger.warn({ err }, 'npm registry failure: ParseError');
throw new Error('registry-failure'); throw new Error('registry-failure');
} }
logger.info({ err }, 'npm registry failure: ParseError, retrying');
await delay(5000 / retries);
return getDependency(name, retries - 1);
}
if (err.statusCode === 429) { if (err.statusCode === 429) {
// This is bad if it ever happens, so we should error // This is bad if it ever happens, so we should error
logger.error({ err }, 'npm registry failure: too many requests'); logger.error({ err }, 'npm registry failure: too many requests');
......
...@@ -51,6 +51,8 @@ Object { ...@@ -51,6 +51,8 @@ Object {
} }
`; `;
exports[`api/npm should retry when 408 or 5xx 1`] = `null`;
exports[`api/npm should send an authorization header if provided 1`] = ` exports[`api/npm should send an authorization header if provided 1`] = `
Object { Object {
"dist-tags": Object { "dist-tags": Object {
......
...@@ -35,7 +35,10 @@ describe('api/npm', () => { ...@@ -35,7 +35,10 @@ describe('api/npm', () => {
nock('https://registry.npmjs.org') nock('https://registry.npmjs.org')
.get('/foobar') .get('/foobar')
.reply(200, missingVersions); .reply(200, missingVersions);
const res = await npm.getDependency('foobar'); nock('https://registry.npmjs.org')
.get('/foobar')
.reply(200, missingVersions);
const res = await npm.getDependency('foobar', 1);
expect(res).toBe(null); expect(res).toBe(null);
}); });
it('should fetch package info from npm', async () => { it('should fetch package info from npm', async () => {
...@@ -70,12 +73,15 @@ describe('api/npm', () => { ...@@ -70,12 +73,15 @@ describe('api/npm', () => {
expect(res).toBeNull(); expect(res).toBeNull();
}); });
it('should throw error for unparseable', async () => { it('should throw error for unparseable', async () => {
nock('https://registry.npmjs.org')
.get('/foobar')
.reply(200, 'oops');
nock('https://registry.npmjs.org') nock('https://registry.npmjs.org')
.get('/foobar') .get('/foobar')
.reply(200, 'oops'); .reply(200, 'oops');
let e; let e;
try { try {
await npm.getDependency('foobar'); await npm.getDependency('foobar', 1);
} catch (err) { } catch (err) {
e = err; e = err;
} }
...@@ -127,8 +133,8 @@ describe('api/npm', () => { ...@@ -127,8 +133,8 @@ describe('api/npm', () => {
nock('https://registry.npmjs.org') nock('https://registry.npmjs.org')
.get('/foobar') .get('/foobar')
.reply(200); .reply(200);
const res = await npm.getDependency('foobar'); const res = await npm.getDependency('foobar', 2);
expect(res).toBe(null); expect(res).toMatchSnapshot();
}); });
it('should throw error for others', async () => { it('should throw error for others', async () => {
nock('https://registry.npmjs.org') nock('https://registry.npmjs.org')
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment