fix(api): return 503 to npm/yarn on uplink connection timeout (#1331)

fix  #1328 and #720

Type: bug

The following has been addressed in the PR:

Instead of returning a 404 (Not Found) when npm, yarn, etc requests a package and the package cannot be acquired from an uplink due to a connection timeout, socket timeout, or connection reset problem, a 503 (service unavailable) is returned by Verdaccio instead. In limited testing of a few versions of npm and yarn, both of these clients correctly attempt to retry the request when a 503 is returned.

Added functional tests to verify the behavior (this adds a dev dependency on nock, which provides HTTP request mocking

Description:

This resolves issue #1328 and #720, and ensures npm/yarn install commands don't fail immediately when there is an intermittent network timeout problem with an uplink. Instead Verdaccio will appropriately respond to the client with a 503. A 404 response (current behavior) incorrectly tells the client that the package does not exist (which may or may not be true) and to not try again.

package.json

@@ -73,6 +73,7 @@
     "jest": "24.7.1",
     "jest-environment-node": "24.7.1",
     "lint-staged": "8.1.5",
+    "nock": "10.0.6",
     "prettier": "1.17.0",
     "puppeteer": "1.8.0",
     "rimraf": "2.6.3",

src/lib/storage.js

@@ -411,13 +411,13 @@ class Storage implements IStorageHandler {
    returns callback(err, result, uplink_errors)
    */
   _syncUplinksMetadata(name: string, packageInfo: Package, options: ISyncUplinks, callback: Callback): void {
-    let exists = true;
+    let found = true;
     const self = this;
     const upLinks = [];
     const hasToLookIntoUplinks = _.isNil(options.uplinksLook) || options.uplinksLook;
 
     if (!packageInfo) {
-      exists = false;
+      found = false;
       packageInfo = generatePackageTemplate(name);
     }
 
@@ -489,14 +489,36 @@ class Storage implements IStorageHandler {
 
           // if we got to this point, assume that the correct package exists
           // on the uplink
-          exists = true;
+          found = true;
           cb();
         });
       },
       (err: Error, upLinksErrors: any) => {
         assert(!err && Array.isArray(upLinksErrors));
-        if (!exists) {
+
-          return callback(ErrorCode.getNotFound(API_ERROR.NO_PACKAGE), null, upLinksErrors);
+        // Check for connection timeout or reset errors with uplink(s)
+        // (these should be handled differently from the package not being found)
+        if (!found) {
+          let uplinkTimeoutError;
+          for (let i = 0; i < upLinksErrors.length; i++) {
+            if (upLinksErrors[i]) {
+              for (let j = 0; j < upLinksErrors[i].length; j++) {
+                if (upLinksErrors[i][j]) {
+                  const code = upLinksErrors[i][j].code;
+                  if (code === 'ETIMEDOUT' || code === 'ESOCKETTIMEDOUT' || code === 'ECONNRESET') {
+                    uplinkTimeoutError = true;
+                    break;
+                  }
+                }
+              }
+            }
+          }
+
+          if (uplinkTimeoutError) {
+            return callback(ErrorCode.getServiceUnavailable(), null, upLinksErrors);
+          } else {
+            return callback(ErrorCode.getNotFound(API_ERROR.NO_PACKAGE), null, upLinksErrors);
+          }
         }
 
         if (upLinks.length === 0) {

test/functional/fixtures/plugins/middlewares.uplink.js

@@ -0,0 +1,21 @@
+const nock = require('nock');
+
+function Plugin(config) {
+  const self = Object.create(Plugin.prototype);
+  self._config = config;
+  return self;
+}
+
+Plugin.prototype.register_middlewares = function (app, auth, storage) {
+
+  app.get('/test-uplink-timeout-*', function (req, res, next) {
+    nock('http://localhost:55552')
+      .get(req.path)
+      .socketDelay(31000).reply(200); // 31s is greater than the default 30s connection timeout
+
+    next();
+  });
+
+}
+
+module.exports = Plugin;

test/functional/index.spec.js

@@ -28,6 +28,7 @@ import race from './performance/race';
 import pluginsAuth from './plugins/auth';
 import middleware from './plugins/middleware';
 import upLinkCache from './uplinks/cache';
+import uplinkTimeout from './uplinks/timeout';
 
 describe('functional test verdaccio', function() {
   jest.setTimeout(10000);
@@ -55,6 +56,7 @@ describe('functional test verdaccio', function() {
   addtag(server1);
   pluginsAuth(server2);
   notify(app);  
+  uplinkTimeout(server1, server2, server3);
   // requires packages published to server1/server2
   upLinkCache(server1, server2, server3);
   adduser(server1);

test/functional/store/config-1.yaml

@@ -6,6 +6,10 @@ web:
   enable: true
   title: verdaccio-server-1
 
+middlewares:
+  ../fixtures/plugins/middlewares.uplink:
+    message: provides uplink mocking (e.g. simulates socket timeout)
+
 auth:
   auth-memory:
     users:
@@ -116,6 +120,12 @@ packages:
     publish: $authenticated
     storage: sub_storage
 
+  'test-uplink-timeout-*':
+    access: $all
+    proxy:
+      - server2
+      - server3
+
   '*':
     access: test $anonymous
     publish: test $anonymous

test/functional/store/config-3.yaml

@@ -35,6 +35,10 @@ packages:
     access: $all
     proxy: server2
 
+  'test-uplink-timeout-*':
+    access: $all
+    publish: $all
+
   '*':
     access: $all
 

test/functional/uplinks/timeout.js

@@ -0,0 +1,27 @@
+import {HTTP_STATUS} from "../../../src/lib/constants";
+
+const PKG_SINGLE_UPLINK = 'test-uplink-timeout-single';
+const PKG_MULTIPLE_UPLINKS = 'test-uplink-timeout-multiple';
+
+export default function (server, server2, server3) {
+
+  describe('uplink connection timeouts', () => {
+    beforeAll(async () => {
+      await server2.addPackage(PKG_SINGLE_UPLINK).status(HTTP_STATUS.CREATED);
+      await server2.addPackage(PKG_MULTIPLE_UPLINKS).status(HTTP_STATUS.CREATED);
+      await server3.addPackage(PKG_MULTIPLE_UPLINKS).status(HTTP_STATUS.CREATED);
+    });
+
+    describe('get package', () => {
+      test('503 response when uplink connection ESOCKETTIMEDOUT', () => {
+        return server.getPackage(PKG_SINGLE_UPLINK).status(HTTP_STATUS.SERVICE_UNAVAILABLE);
+      });
+
+      test('200 response even though one uplink timesout', () => {
+        return server.getPackage(PKG_MULTIPLE_UPLINKS).status(HTTP_STATUS.OK)
+      });
+    });
+
+  });
+
+}