refactor: better usage of http error functions

src/api/endpoint/api/package.js

@@ -37,7 +37,7 @@ export default function(route: Router, auth: IAuth, storage: IStorageHandler, co
           }
         }
       }
-      return next(ErrorCode.get404(`version not found: ${req.params.version}`));
+      return next(ErrorCode.getNotFound(`version not found: ${req.params.version}`));
     };
 
     storage.getPackage({

src/api/endpoint/api/publish.js

@@ -67,7 +67,7 @@ export default function(router: Router, auth: IAuth, storage: IStorageHandler, c
         || Object.keys(metadata.versions).length !== 1) {
         // npm is doing something strange again
         // if this happens in normal circumstances, report it as a bug
-        return next(ErrorCode.get400('unsupported registry call'));
+        return next(ErrorCode.getBadRequest('unsupported registry call'));
       }
 
       if (err && err.status != 409) {
@@ -108,13 +108,13 @@ export default function(router: Router, auth: IAuth, storage: IStorageHandler, c
 
     if (Object.keys(req.body).length === 1 && isObject(req.body.users)) {
       // 501 status is more meaningful, but npm doesn't show error message for 5xx
-      return next(ErrorCode.get404('npm star|unstar calls are not implemented'));
+      return next(ErrorCode.getNotFound('npm star|unstar calls are not implemented'));
     }
 
     try {
       metadata = validate_metadata(req.body, name);
     } catch (err) {
-      return next(ErrorCode.get422('bad incoming package data'));
+      return next(ErrorCode.getBadData('bad incoming package data'));
     }
 
     if (req.params._rev) {

src/api/endpoint/api/user.js

@@ -32,7 +32,7 @@ export default function(route: Router, auth: IAuth) {
             // With npm registering is the same as logging in,
             // and npm accepts only an 409 error.
             // So, changing status code here.
-            return next( ErrorCode.getCode(err.status, err.message) || ErrorCode.get409(err.message));
+            return next( ErrorCode.getCode(err.status, err.message) || ErrorCode.getConflict(err.message));
           }
           return next(err);
         }

src/api/middleware.js

@@ -38,7 +38,7 @@ export function validateName(req: $RequestExtend, res: $ResponseExtend, next: $N
   } else if (utilValidateName(value)) {
     next();
   } else {
-    next( ErrorCode.get403('invalid ' + name));
+    next( ErrorCode.getForbidden('invalid ' + name));
   }
 }
 
@@ -51,7 +51,7 @@ export function validatePackage(req: $RequestExtend, res: $ResponseExtend, next:
   } else if (utilValidatePackage(value)) {
     next();
   } else {
-    next( ErrorCode.get403('invalid ' + name) );
+    next( ErrorCode.getForbidden('invalid ' + name) );
   }
 }
 
@@ -76,7 +76,7 @@ export function encodeScopePackage(req: $RequestExtend, res: $ResponseExtend, ne
 
 export function expectJson(req: $RequestExtend, res: $ResponseExtend, next: $NextFunctionVer) {
   if (!isObject(req.body)) {
-    return next( ErrorCode.get400('can\'t parse incoming json') );
+    return next( ErrorCode.getBadRequest('can\'t parse incoming json') );
   }
   next();
 }
@@ -115,7 +115,7 @@ export function allow(auth: IAuth) {
         } else {
           // last plugin (that's our built-in one) returns either
           // cb(err) or cb(null, true), so this should never happen
-          throw ErrorCode.get500('bug in the auth plugin system');
+          throw ErrorCode.getInternalError('bug in the auth plugin system');
         }
       });
     };

src/lib/auth.js

@@ -53,20 +53,20 @@ class Auth {
         }
 
         if (user.name) {
-          cb(ErrorCode.get403('user ' + user.name + ' is not allowed to ' + action + ' package ' + pkg.name));
+          cb(ErrorCode.getForbidden('user ' + user.name + ' is not allowed to ' + action + ' package ' + pkg.name));
         } else {
-          cb(ErrorCode.get403('unregistered users are not allowed to ' + action + ' package ' + pkg.name));
+          cb(ErrorCode.getForbidden('unregistered users are not allowed to ' + action + ' package ' + pkg.name));
         }
       };
     };
 
     this.plugins.push({
       authenticate: function(user, password, cb) {
-        cb(ErrorCode.get403('bad username/password, access denied'));
+        cb(ErrorCode.getForbidden('bad username/password, access denied'));
       },
 
       add_user: function(user, password, cb) {
-        return cb(ErrorCode.get409('bad username/password, access denied'));
+        return cb(ErrorCode.getConflict('bad username/password, access denied'));
       },
 
       allow_access: allow_action('access'),
@@ -224,7 +224,7 @@ class Auth {
 
       const parts = authorization.split(' ');
       if (parts.length !== 2) {
-        return next( ErrorCode.get400('bad authorization header') );
+        return next( ErrorCode.getBadRequest('bad authorization header') );
       }
 
       const credentials = this._parseCredentials(parts);

src/lib/constants.js

@@ -31,6 +31,7 @@ export const HTTP_STATUS = {
   FORBIDDEN: 403,
   NOT_FOUND: 404,
   CONFLICT: 409,
+  BAD_DATA: 422,
 };
 
 export const PORT_SERVER_1 = '55551';

src/lib/local-storage.js

@@ -53,12 +53,12 @@ class LocalStorage implements IStorage {
     const storage: any = this._getLocalStorage(name);
 
     if (_.isNil(storage)) {
-      return callback( ErrorCode.get404('this package cannot be added'));
+      return callback( ErrorCode.getNotFound('this package cannot be added'));
     }
 
     storage.createPackage(name, generatePackageTemplate(name), (err) => {
       if (_.isNull(err) === false && err.code === fileExist) {
-        return callback( ErrorCode.get409());
+        return callback( ErrorCode.getConflict());
       }
 
       const latest = getLatestVersion(pkg);
@@ -80,13 +80,13 @@ class LocalStorage implements IStorage {
     let storage: any = this._getLocalStorage(name);
 
     if (_.isNil(storage)) {
-      return callback( ErrorCode.get404());
+      return callback( ErrorCode.getNotFound());
     }
 
     storage.readPackage(name, (err, data) => {
       if (_.isNil(err) === false) {
         if (err.code === noSuchFile) {
-          return callback( ErrorCode.get404());
+          return callback( ErrorCode.getNotFound());
         } else {
           return callback(err);
         }
@@ -97,7 +97,7 @@ class LocalStorage implements IStorage {
       this.localData.remove(name, (removeFailed) => {
         if (removeFailed) {
           // This will happen when database is locked
-          return callback(ErrorCode.get422(removeFailed.message));
+          return callback(ErrorCode.getBadData(removeFailed.message));
         }
 
         storage.deletePackage(pkgFileName, (err) => {
@@ -220,7 +220,7 @@ class LocalStorage implements IStorage {
       metadata = cleanUpReadme(metadata);
 
       if (data.versions[version] != null) {
-        return cb( ErrorCode.get409() );
+        return cb( ErrorCode.getConflict() );
       }
 
       // if uploaded tarball has a different shasum, it's very likely that we have some kind of error
@@ -231,7 +231,7 @@ class LocalStorage implements IStorage {
           if (_.isNil(data._attachments[tarball].shasum) === false && _.isNil(metadata.dist.shasum) === false) {
             if (data._attachments[tarball].shasum != metadata.dist.shasum) {
               const errorMessage = `shasum error, ${data._attachments[tarball].shasum} != ${metadata.dist.shasum}`;
-              return cb( ErrorCode.get400(errorMessage) );
+              return cb( ErrorCode.getBadRequest(errorMessage) );
             }
           }
 
@@ -258,7 +258,7 @@ class LocalStorage implements IStorage {
 
       this.localData.add(name, (addFailed) => {
         if (addFailed) {
-          return cb(ErrorCode.get422(addFailed.message));
+          return cb(ErrorCode.getBadData(addFailed.message));
         }
 
         cb();
@@ -298,7 +298,7 @@ class LocalStorage implements IStorage {
    * @private
    */
   _getVersionNotFound() {
-    return ErrorCode.get404('this version doesn\'t exist');
+    return ErrorCode.getNotFound('this version doesn\'t exist');
   }
 
   /**
@@ -307,7 +307,7 @@ class LocalStorage implements IStorage {
    * @private
    */
   _getFileNotAvailable() {
-    return ErrorCode.get404('no such file available');
+    return ErrorCode.getNotFound('no such file available');
   }
 
   /**
@@ -323,7 +323,7 @@ class LocalStorage implements IStorage {
                 pkg: Package,
                 revision?: string, callback: Callback) {
     if (!isObject(pkg.versions) || !isObject(pkg[DIST_TAGS])) {
-      return callback( ErrorCode.get422());
+      return callback( ErrorCode.getBadData());
     }
 
     this._updatePackage(name, (jsonData, cb) => {
@@ -407,7 +407,7 @@ class LocalStorage implements IStorage {
 
     if (name === pkgFileName || name === '__proto__') {
       process.nextTick(() => {
-        uploadStream.emit('error', ErrorCode.get403());
+        uploadStream.emit('error', ErrorCode.getForbidden());
       });
       return uploadStream;
     }
@@ -423,7 +423,7 @@ class LocalStorage implements IStorage {
 
     writeStream.on('error', (err) => {
       if (err.code === fileExist) {
-        uploadStream.emit('error', ErrorCode.get409());
+        uploadStream.emit('error', ErrorCode.getConflict());
       } else if (err.code === noSuchFile) {
         // check if package exists to throw an appropriate message
         this.getPackageMetadata(name, function(_err, res) {
@@ -464,7 +464,7 @@ class LocalStorage implements IStorage {
 
     uploadStream.done = function() {
       if (!length) {
-        uploadStream.emit('error', ErrorCode.get422('refusing to accept zero-length file'));
+        uploadStream.emit('error', ErrorCode.getBadData('refusing to accept zero-length file'));
         writeStream.abort();
       } else {
         writeStream.done();
@@ -518,7 +518,7 @@ class LocalStorage implements IStorage {
   _streamSuccessReadTarBall(storage: any, filename: string): IReadTarball {
     const stream: IReadTarball = new ReadTarball();
     const readTarballStream = storage.readTarball(filename);
-    const e404 = ErrorCode.get404;
+    const e404 = ErrorCode.getNotFound;
 
     stream.abort = function() {
       if (_.isNil(readTarballStream) === false) {
@@ -556,7 +556,7 @@ class LocalStorage implements IStorage {
   getPackageMetadata(name: string, callback?: Callback = () => {}): void {
     const storage: IPackageStorage = this._getLocalStorage(name);
     if (_.isNil(storage)) {
-      return callback( ErrorCode.get404() );
+      return callback( ErrorCode.getNotFound() );
     }
 
     this._readPackage(name, storage, callback);
@@ -615,7 +615,7 @@ class LocalStorage implements IStorage {
     storage.readPackage(name, (err, result) => {
       if (err) {
         if (err.code === noSuchFile) {
-          return callback( ErrorCode.get404() );
+          return callback( ErrorCode.getNotFound() );
         } else {
           return callback(this._internalError(err, pkgFileName, 'error reading'));
         }
@@ -703,7 +703,7 @@ class LocalStorage implements IStorage {
   _internalError(err: string, file: string, message: string) {
     this.logger.error( {err: err, file: file}, `${message}  @{file}: @{!err.message}` );
 
-    return ErrorCode.get500();
+    return ErrorCode.getInternalError();
   }
 
   /**
@@ -716,7 +716,7 @@ class LocalStorage implements IStorage {
     const storage: IPackageStorage = this._getLocalStorage(name);
 
     if (!storage) {
-      return callback( ErrorCode.get404() );
+      return callback( ErrorCode.getNotFound() );
     }
 
     storage.updatePackage(name, updateHandler, this._writePackage.bind(this), normalizePackage,

src/lib/storage-utils.js

@@ -127,7 +127,7 @@ export function checkPackageLocal(name: string, localStorage: IStorage): Promise
         return reject(err);
       }
       if (results) {
-        return reject(ErrorCode.get409('this package is already present'));
+        return reject(ErrorCode.getConflict('this package is already present'));
       }
       return resolve();
     });
@@ -158,7 +158,7 @@ export function checkPackageRemote(name: string, isAllowPublishOffline: boolean,
 
       // checking package exist already
       if (_.isNil(packageJsonLocal) === false) {
-        return reject(ErrorCode.get409('this package is already present'));
+        return reject(ErrorCode.getConflict('this package is already present'));
       }
 
       for (let errorItem = 0; errorItem < upLinksErrors.length; errorItem++) {
@@ -170,7 +170,7 @@ export function checkPackageRemote(name: string, isAllowPublishOffline: boolean,
               return resolve();
             }
 
-            return reject(ErrorCode.get503('one of the uplinks is down, refuse to publish'));
+            return reject(ErrorCode.getServiceUnavailable('one of the uplinks is down, refuse to publish'));
           }
         }
       }

src/lib/storage.js

@@ -438,7 +438,7 @@ class Storage implements IStorageHandler {
 
         if (err || !upLinkResponse) {
           // $FlowFixMe
-          return cb(null, [err || ErrorCode.get500('no data')]);
+          return cb(null, [err || ErrorCode.getInternalError('no data')]);
         }
 
         try {
@@ -478,7 +478,7 @@ class Storage implements IStorageHandler {
     }, (err: Error, upLinksErrors: any) => {
       assert(!err && Array.isArray(upLinksErrors));
       if (!exists) {
-        return callback( ErrorCode.get404('no such package available')
+        return callback( ErrorCode.getNotFound('no such package available')
           , null
           , upLinksErrors );
       }

src/lib/up-storage.js

@@ -105,10 +105,10 @@ class ProxyStorage implements IProxy {
 
       process.nextTick(function() {
         if (cb) {
-          cb(ErrorCode.get500('uplink is offline'));
+          cb(ErrorCode.getInternalError('uplink is offline'));
         }
         // $FlowFixMe
-        streamRead.emit('error', ErrorCode.get500('uplink is offline'));
+        streamRead.emit('error', ErrorCode.getInternalError('uplink is offline'));
       });
       // $FlowFixMe
       streamRead._read = function() {};
@@ -408,10 +408,10 @@ class ProxyStorage implements IProxy {
         return callback(err);
       }
       if (res.statusCode === 404) {
-        return callback( ErrorCode.get404('package doesn\'t exist on uplink'));
+        return callback( ErrorCode.getNotFound('package doesn\'t exist on uplink'));
       }
       if (!(res.statusCode >= 200 && res.statusCode < 300)) {
-        const error = ErrorCode.get500(`bad status code: ${res.statusCode}`);
+        const error = ErrorCode.getInternalError(`bad status code: ${res.statusCode}`);
         // $FlowFixMe
         error.remoteStatus = res.statusCode;
         return callback(error);
@@ -441,10 +441,10 @@ class ProxyStorage implements IProxy {
 
     readStream.on('response', function(res: any) {
       if (res.statusCode === 404) {
-        return stream.emit('error', ErrorCode.get404('file doesn\'t exist on uplink'));
+        return stream.emit('error', ErrorCode.getNotFound('file doesn\'t exist on uplink'));
       }
       if (!(res.statusCode >= 200 && res.statusCode < 300)) {
-        return stream.emit('error', ErrorCode.get500(`bad uplink status code: ${res.statusCode}`));
+        return stream.emit('error', ErrorCode.getInternalError(`bad uplink status code: ${res.statusCode}`));
       }
       if (res.headers['content-length']) {
         expected_length = res.headers['content-length'];
@@ -465,7 +465,7 @@ class ProxyStorage implements IProxy {
         current_length += data.length;
       }
       if (expected_length && current_length != expected_length) {
-        stream.emit('error', ErrorCode.get500('content length mismatch'));
+        stream.emit('error', ErrorCode.getInternalError('content length mismatch'));
       }
     });
     return stream;
@@ -494,7 +494,7 @@ class ProxyStorage implements IProxy {
 
     requestStream.on('response', (res) => {
       if (!String(res.statusCode).match(/^2\d\d$/)) {
-        return transformStream.emit('error', ErrorCode.get500(`bad status code ${res.statusCode} from uplink`));
+        return transformStream.emit('error', ErrorCode.getInternalError(`bad status code ${res.statusCode} from uplink`));
       }
 
       // See https://github.com/request/request#requestoptions-callback

src/lib/utils.js

@@ -7,11 +7,13 @@ import YAML from 'js-yaml';
 import URL from 'url';
 import fs from 'fs';
 import _ from 'lodash';
+import asciidoctor from 'asciidoctor.js';
 import createError from 'http-errors';
+import marked from 'marked';
+import {HTTP_STATUS} from './constants';
+
 import type {Package} from '@verdaccio/types';
 import type {$Request} from 'express';
-import marked from 'marked';
-import asciidoctor from 'asciidoctor.js';
 import type {StringValue} from '../../types';
 
 const Logger = require('./logger');
@@ -337,26 +339,27 @@ const getLatestVersion = function(pkgInfo: Package) {
 };
 
 const ErrorCode = {
-  get409: (message: string = 'this package is already present') => {
+  getConflict: (message: string = 'this package is already present') => {
-    return createError(409, message);
+    return createError(HTTP_STATUS.CONFLICT, message);
   },
-  get422: (customMessage?: string) => {
+  getBadData: (customMessage?: string) => {
-    return createError(422, customMessage || 'bad data');
+    return createError(HTTP_STATUS.BAD_DATA, customMessage || 'bad data');
   },
-  get400: (customMessage?: string) => {
+  getBadRequest: (customMessage?: string) => {
-    return createError(400, customMessage);
+    return createError(HTTP_STATUS.BAD_REQUEST, customMessage);
   },
-  get500: (customMessage?: string) => {
+  getInternalError: (customMessage?: string) => {
-    return customMessage ? createError(500, customMessage) : createError(500);
+    return customMessage ? createError(HTTP_STATUS.INTERNAL_ERROR, customMessage)
+      : createError(HTTP_STATUS.INTERNAL_ERROR);
   },
-  get403: (message: string = 'can\'t use this filename') => {
+  getForbidden: (message: string = 'can\'t use this filename') => {
-    return createError(403, message);
+    return createError(HTTP_STATUS.FORBIDDEN, message);
   },
-  get503: (message: string = 'resource temporarily unavailable') => {
+  getServiceUnavailable: (message: string = 'resource temporarily unavailable') => {
-    return createError(503, message);
+    return createError(HTTP_STATUS.SERVICE_UNAVAILABLE, message);
   },
-  get404: (customMessage?: string) => {
+  getNotFound: (customMessage?: string) => {
-    return createError(404, customMessage || 'no such package available');
+    return createError(HTTP_STATUS.NOT_FOUND, customMessage || 'no such package available');
   },
   getCode: (statusCode: number, customMessage: string) => {
     return createError(statusCode, customMessage);