Skip to content

Commit

Permalink
feat: abort repo for most npm registry errors
Browse files Browse the repository at this point in the history
Renovate now aborts processing of repositories if for any 4xx responses except 401 and 404, and also for 200 OK responses which are unparseable.

Closes #1341
  • Loading branch information
rarkins committed Jan 21, 2018
1 parent f3d18e1 commit d774a14
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 13 deletions.
16 changes: 13 additions & 3 deletions lib/manager/npm/registry.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,11 +108,21 @@ async function getDependency(name) {
logger.info({ err, name }, `Dependency not found`);
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 (err.statusCode === 429) {
// This is bad if it ever happens, so we should error
logger.error({ err }, 'npm registry failure: too many requests');
throw new Error('registry-failure');
}
if (err.statusCode >= 500 && err.statusCode < 600) {
logger.info({ err }, 'npm registry failure');
logger.warn({ err }, 'npm registry failure: internal error');
throw new Error('registry-failure');
}
logger.warn({ err, name }, 'Unknown npm lookup error');
return null;
logger.warn({ err, name }, 'npm registry failures: Unknown error');
throw new Error('registry-failure');
}
}
19 changes: 17 additions & 2 deletions test/manager/npm/__snapshots__/registry.spec.js.snap
Original file line number Diff line number Diff line change
@@ -1,6 +1,21 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`api/npm should fetch package info from custom registry 1`] = `null`;
exports[`api/npm should fetch package info from custom registry 1`] = `
Object {
"dist-tags": Object {
"latest": "0.0.1",
},
"homepage": undefined,
"name": undefined,
"renovate-config": undefined,
"repositoryUrl": undefined,
"versions": Object {
"0.0.1": Object {
"time": "",
},
},
}
`;

exports[`api/npm should fetch package info from npm 1`] = `
Object {
Expand Down Expand Up @@ -81,7 +96,7 @@ Object {
"repositoryUrl": undefined,
"versions": Object {
"0.0.1": Object {
"time": "",
"time": "2017-01-01T12:00:00.000Z",
},
},
}
Expand Down
52 changes: 44 additions & 8 deletions test/manager/npm/registry.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,30 @@ describe('api/npm', () => {
const res = await npm.getDependency('foobar');
expect(res).toBeNull();
});
it('should throw error for unparseable', async () => {
nock('https://registry.npmjs.org')
.get('/foobar')
.reply(200, 'oops');
let e;
try {
await npm.getDependency('foobar');
} catch (err) {
e = err;
}
expect(e).toBeDefined();
});
it('should throw error for 429', async () => {
nock('https://registry.npmjs.org')
.get('/foobar')
.reply(429);
let e;
try {
await npm.getDependency('foobar');
} catch (err) {
e = err;
}
expect(e).toBeDefined();
});
it('should throw error for 5xx', async () => {
nock('https://registry.npmjs.org')
.get('/foobar')
Expand All @@ -71,6 +95,18 @@ describe('api/npm', () => {
}
expect(e).toBeDefined();
});
it('should throw error for others', async () => {
nock('https://registry.npmjs.org')
.get('/foobar')
.reply(451);
let e;
try {
await npm.getDependency('foobar');
} catch (err) {
e = err;
}
expect(e).toBeDefined();
});
it('should send an authorization header if provided', async () => {
registryAuthToken.mockImplementation(() => ({
type: 'Basic',
Expand All @@ -92,14 +128,6 @@ describe('api/npm', () => {
process.env.NPM_TOKEN = oldToken;
expect(res).toMatchSnapshot();
});
it('should fetch package info from custom registry', async () => {
nock('https://registry.npmjs.org')
.get('/foobar')
.reply(200, npmResponse);
npm.setNpmrc('registry=https://npm.mycustomregistry.com/');
const res = await npm.getDependency('foobar');
expect(res).toMatchSnapshot();
});
it('should use default registry if missing from npmrc', async () => {
nock('https://registry.npmjs.org')
.get('/foobar')
Expand All @@ -126,4 +154,12 @@ describe('api/npm', () => {
expect(res1).not.toBe(null);
expect(res1).toEqual(res2);
});
it('should fetch package info from custom registry', async () => {
nock('https://npm.mycustomregistry.com')
.get('/foobar')
.reply(200, npmResponse);
npm.setNpmrc('registry=https://npm.mycustomregistry.com/');
const res = await npm.getDependency('foobar');
expect(res).toMatchSnapshot();
});
});

0 comments on commit d774a14

Please sign in to comment.