From 6623047177cc259042f96412db3d2d57991557a2 Mon Sep 17 00:00:00 2001 From: Rhys Arkins <rhys@arkins.net> Date: Tue, 27 Mar 2018 20:57:11 +0200 Subject: [PATCH] fix: add npm retries for null versions or parseError --- lib/datasource/npm.js | 22 ++++++++++++++----- .../npm/__snapshots__/registry.spec.js.snap | 2 ++ test/manager/npm/registry.spec.js | 14 ++++++++---- 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/lib/datasource/npm.js b/lib/datasource/npm.js index 81486f3f48..f5d724e3aa 100644 --- a/lib/datasource/npm.js +++ b/lib/datasource/npm.js @@ -110,7 +110,14 @@ async function getDependency(name, retries = 5) { headers, })).body; if (!res.versions || !Object.keys(res.versions).length) { - return null; + // Registry returned a 200 OK but with no versions + if (retries <= 0) { + logger.info({ name }, 'No versions returned'); + return null; + } + logger.info('No versions returned, retrying'); + await delay(5000 / retries); + return getDependency(name, 0); } // Determine repository URL let repositoryUrl; @@ -142,18 +149,23 @@ async function getDependency(name, retries = 5) { return dep; } catch (err) { if (err.statusCode === 401) { - logger.info({ err, name }, `Dependency lookup unauthorized`); + logger.info({ err, name }, `Dependency lookup failure: unauthorized`); return null; } if (err.statusCode === 404) { - logger.info({ name }, `Dependency not found`); + logger.info({ name }, `Dependency lookup failure: not found`); logger.debug({ err }); return null; } if (err.name === 'ParseError') { // Registry returned a 200 OK but got failed to parse it - logger.warn({ err }, 'npm registry failure: ParseError'); - throw new Error('registry-failure'); + if (retries <= 0) { + logger.warn({ err }, 'npm registry failure: ParseError'); + 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) { // This is bad if it ever happens, so we should error diff --git a/test/manager/npm/__snapshots__/registry.spec.js.snap b/test/manager/npm/__snapshots__/registry.spec.js.snap index 943cf847f4..aeba6372bf 100644 --- a/test/manager/npm/__snapshots__/registry.spec.js.snap +++ b/test/manager/npm/__snapshots__/registry.spec.js.snap @@ -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`] = ` Object { "dist-tags": Object { diff --git a/test/manager/npm/registry.spec.js b/test/manager/npm/registry.spec.js index 857cce9c38..6a4089c2a4 100644 --- a/test/manager/npm/registry.spec.js +++ b/test/manager/npm/registry.spec.js @@ -35,7 +35,10 @@ describe('api/npm', () => { nock('https://registry.npmjs.org') .get('/foobar') .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); }); it('should fetch package info from npm', async () => { @@ -70,12 +73,15 @@ describe('api/npm', () => { expect(res).toBeNull(); }); it('should throw error for unparseable', async () => { + nock('https://registry.npmjs.org') + .get('/foobar') + .reply(200, 'oops'); nock('https://registry.npmjs.org') .get('/foobar') .reply(200, 'oops'); let e; try { - await npm.getDependency('foobar'); + await npm.getDependency('foobar', 1); } catch (err) { e = err; } @@ -127,8 +133,8 @@ describe('api/npm', () => { nock('https://registry.npmjs.org') .get('/foobar') .reply(200); - const res = await npm.getDependency('foobar'); - expect(res).toBe(null); + const res = await npm.getDependency('foobar', 2); + expect(res).toMatchSnapshot(); }); it('should throw error for others', async () => { nock('https://registry.npmjs.org') -- GitLab