From 4d4fa45266eab9fb2f8d7e13df21dbb10a6c1920 Mon Sep 17 00:00:00 2001 From: Wes Mason Date: Thu, 5 May 2016 22:56:36 +0100 Subject: [PATCH 1/3] Fix underlying file access error for 404s when in offline mode --- app.js | 43 +++++++++++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/app.js b/app.js index 3885808..49524d9 100644 --- a/app.js +++ b/app.js @@ -35,23 +35,30 @@ app.get( '/:package', function( req, res, next ){ var cacheFile = [ NPM_PATH, REGISTRY_NAME, packageName, '.cache.json' ].join( '/' ); return fileExists( cacheFile ) - .tap( function( isExists ){ + .then( function( isExists ){ if( !isExists ){ - if ( ENABLE_NPM_FAILOVER ) { - res._log.cacheHit = '---'; - return fetchAndCacheMetadata( packageName, cacheFile ); - } else { - return res.status( 404 ).json( {} ); + if ( !ENABLE_NPM_FAILOVER ) { + return false; } + res._log.cacheHit = '---'; + return fetchAndCacheMetadata( packageName, cacheFile ); } }) - .then( function( ){ + .then( function( isExists ){ + if ( !isExists ) { + return false; + } res._log.cacheFile = cacheFile; return readFile( cacheFile, 'utf-8' ); }) .then( function( cacheData ){ cacheData = JSON.parse( cacheData ); - patchData( cacheData ); + if ( cacheData ) { + patchData( cacheData ); + } else { + res.status( 404 ); + cacheData = {}; + } return res.send( cacheData ); }) .catch( next ); @@ -63,19 +70,23 @@ app.get( '/:package/-/:tarball', function( req, res, next ){ var packagePath = [ NPM_PATH , packageName, version, 'package.tgz'].join( '/' ); fileExists( packagePath ) - .tap( function( isExists ){ + .then( function( isExists ){ if( !isExists ){ - if ( ENABLE_NPM_FAILOVER ) { - res._log.cacheHit = '---'; - return fetchAndCacheTarball( packageName, version, packagePath ); - } else { - return res.status( 404 ).json( {} ); + if ( !ENABLE_NPM_FAILOVER ) { + return false; } + res._log.cacheHit = '---'; + return fetchAndCacheTarball( packageName, version, packagePath ); } }) - .then( function(){ + .then( function( isExists ){ res._log.cacheFile = packagePath; - return res.sendFile( packagePath ); + if ( isExists ) { + return res.sendFile( packagePath ); + } else { + res.status( 404 ); + return res.end(); + } }) .catch( next ); }); From 9ccb6063f8617bad1fb2b5cff16b66c3f5ee4da0 Mon Sep 17 00:00:00 2001 From: Wes Mason Date: Fri, 6 May 2016 09:28:22 +0100 Subject: [PATCH 2/3] Rename second isState flags in both chains to cacheState, so it's clear what they represent in the next callback --- app.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app.js b/app.js index 49524d9..dc5f6d7 100644 --- a/app.js +++ b/app.js @@ -44,8 +44,8 @@ app.get( '/:package', function( req, res, next ){ return fetchAndCacheMetadata( packageName, cacheFile ); } }) - .then( function( isExists ){ - if ( !isExists ) { + .then( function( cacheState ){ + if ( !cacheState ) { return false; } res._log.cacheFile = cacheFile; @@ -79,9 +79,9 @@ app.get( '/:package/-/:tarball', function( req, res, next ){ return fetchAndCacheTarball( packageName, version, packagePath ); } }) - .then( function( isExists ){ + .then( function( cacheState ){ res._log.cacheFile = packagePath; - if ( isExists ) { + if ( cacheState ) { return res.sendFile( packagePath ); } else { res.status( 404 ); From 86ae839e9bd9bbddd1aeb11a90640d3bf7084c2f Mon Sep 17 00:00:00 2001 From: "Harish.K" Date: Fri, 6 May 2016 17:13:25 +0530 Subject: [PATCH 3/3] Small cleanup --- app.js | 41 +++++++++++++---------------------------- 1 file changed, 13 insertions(+), 28 deletions(-) diff --git a/app.js b/app.js index dc5f6d7..fd6da09 100644 --- a/app.js +++ b/app.js @@ -1,4 +1,5 @@ var express = require('express'); +var Promise = require('bluebird'); var config = require( __dirname + '/./config' ); var utils = require( __dirname + '/./utils'); @@ -35,31 +36,23 @@ app.get( '/:package', function( req, res, next ){ var cacheFile = [ NPM_PATH, REGISTRY_NAME, packageName, '.cache.json' ].join( '/' ); return fileExists( cacheFile ) - .then( function( isExists ){ + .tap( function( isExists ){ if( !isExists ){ if ( !ENABLE_NPM_FAILOVER ) { - return false; + return Promise.reject( { status:404, message: 'Package not found' }); } res._log.cacheHit = '---'; return fetchAndCacheMetadata( packageName, cacheFile ); } }) - .then( function( cacheState ){ - if ( !cacheState ) { - return false; - } + .then( function( ){ res._log.cacheFile = cacheFile; return readFile( cacheFile, 'utf-8' ); }) - .then( function( cacheData ){ - cacheData = JSON.parse( cacheData ); - if ( cacheData ) { - patchData( cacheData ); - } else { - res.status( 404 ); - cacheData = {}; - } - return res.send( cacheData ); + .then( function( cachedData ){ + cachedData = JSON.parse( cachedData ); + patchData( cachedData ); + return res.send( cachedData ); }) .catch( next ); }); @@ -70,23 +63,18 @@ app.get( '/:package/-/:tarball', function( req, res, next ){ var packagePath = [ NPM_PATH , packageName, version, 'package.tgz'].join( '/' ); fileExists( packagePath ) - .then( function( isExists ){ + .tap( function( isExists ){ if( !isExists ){ if ( !ENABLE_NPM_FAILOVER ) { - return false; + return Promise.reject( { status: 404, message: '' }); } res._log.cacheHit = '---'; return fetchAndCacheTarball( packageName, version, packagePath ); } }) - .then( function( cacheState ){ + .then( function( ){ res._log.cacheFile = packagePath; - if ( cacheState ) { - return res.sendFile( packagePath ); - } else { - res.status( 404 ); - return res.end(); - } + return res.sendFile( packagePath ); }) .catch( next ); }); @@ -104,10 +92,7 @@ app.use(function(req, res, next) { app.use(function(err, req, res, next) { console.log( err.stack ); res.status(err.status || 500); - res.send({ - message: err.message, - error: err - }); + res.send( err.message || err ); if( next ) { next(); } });