From d774a14910ed2af13c1e96b2606b33e2532f7666 Mon Sep 17 00:00:00 2001
From: Rhys Arkins <rhys@arkins.net>
Date: Sun, 21 Jan 2018 07:16:28 +0100
Subject: [PATCH] feat: abort repo for most npm registry errors

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
---
 lib/manager/npm/registry.js                   | 16 ++++--
 .../npm/__snapshots__/registry.spec.js.snap   | 19 ++++++-
 test/manager/npm/registry.spec.js             | 52 ++++++++++++++++---
 3 files changed, 74 insertions(+), 13 deletions(-)

diff --git a/lib/manager/npm/registry.js b/lib/manager/npm/registry.js
index 5f8bd8773a..c18f6d1dd9 100644
--- a/lib/manager/npm/registry.js
+++ b/lib/manager/npm/registry.js
@@ -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');
   }
 }
diff --git a/test/manager/npm/__snapshots__/registry.spec.js.snap b/test/manager/npm/__snapshots__/registry.spec.js.snap
index d35564e314..2220264b22 100644
--- a/test/manager/npm/__snapshots__/registry.spec.js.snap
+++ b/test/manager/npm/__snapshots__/registry.spec.js.snap
@@ -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 {
@@ -81,7 +96,7 @@ Object {
   "repositoryUrl": undefined,
   "versions": Object {
     "0.0.1": Object {
-      "time": "",
+      "time": "2017-01-01T12:00:00.000Z",
     },
   },
 }
diff --git a/test/manager/npm/registry.spec.js b/test/manager/npm/registry.spec.js
index a775a16af4..498085728a 100644
--- a/test/manager/npm/registry.spec.js
+++ b/test/manager/npm/registry.spec.js
@@ -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')
@@ -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',
@@ -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')
@@ -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();
+  });
 });
-- 
GitLab