doctor: do not crash w/ registries that do not support ping

If registry doesn't support ping, npm doctor should display the information instead of stopping its investigation. Credit: @watilde PR-URL: npm/npm#16021 Reviewed-By: @iarna
degouville · Apr 20, 2017 · 9c860f2 · 9c860f2
1 parent f08c663
commit 9c860f2
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 14 deletions.
diff --git a/lib/doctor.js b/lib/doctor.js
@@ -72,14 +72,14 @@ function doctor (args, silent, cb) {
 }
 
 function makePretty (p) {
-  var ping = p[0] ? 'ok' : 'notOk'
-  var npmLTS = p[1]
-  var nodeLTS = p[2].replace('v', '')
-  var whichGit = p[3] || 'not installed'
-  var readbleCaches = p[4] ? 'ok' : 'notOk'
-  var executableGlobalModules = p[5] ? 'ok' : 'notOk'
-  var executableLocalModules = p[6] ? 'ok' : 'notOk'
-  var checksumCachedFiles = p[7] ? 'ok' : 'notOk'
+  var ping = p[1]
+  var npmLTS = p[2]
+  var nodeLTS = p[3].replace('v', '')
+  var whichGit = p[4] || 'not installed'
+  var readbleCaches = p[5] ? 'ok' : 'notOk'
+  var executableGlobalModules = p[6] ? 'ok' : 'notOk'
+  var executableLocalModules = p[7] ? 'ok' : 'notOk'
+  var checksumCachedFiles = p[8] ? 'ok' : 'notOk'
   var npmV = npm.version
   var nodeV = process.version.replace('v', '')
   var registry = npm.config.get('registry')
@@ -95,7 +95,7 @@ function makePretty (p) {
     ['Checksum cached files', checksumCachedFiles]
   ]
 
-  if (ping !== 'ok') list[0][2] = 'Check your internet connection'
+  if (p[0] !== 200) list[0][2] = 'Check your internet connection'
   if (!semver.satisfies(npmV, '>=' + npmLTS)) list[1][2] = 'Use npm v' + npmLTS
   if (!semver.satisfies(nodeV, '>=' + nodeLTS)) list[2][2] = 'Use node v' + nodeLTS
   if (registry !== defaultRegistry) list[3][2] = 'Try `npm config set registry ' + defaultRegistry + '`'

diff --git a/lib/doctor/check-ping.js b/lib/doctor/check-ping.js
@@ -4,9 +4,9 @@ var ping = require('../ping.js')
 function checkPing (cb) {
   var tracker = log.newItem('checkPing', 1)
   tracker.info('checkPing', 'Pinging registry')
-  ping({}, true, function (err, pong) {
+  ping({}, true, function (err, pong, data, res) {
     tracker.finish()
-    cb(err, pong)
+    cb(null, [res.statusCode, res.statusMessage])
   })
 }
 

diff --git a/lib/ping.js b/lib/ping.js
@@ -14,8 +14,8 @@ function ping (args, silent, cb) {
   if (!registry) return cb(new Error('no default registry set'))
   var auth = npm.config.getCredentialsByURI(registry)
 
-  npm.registry.ping(registry, {auth: auth}, function (er, pong) {
+  npm.registry.ping(registry, {auth: auth}, function (er, pong, data, res) {
     if (!silent) output(JSON.stringify(pong))
-    cb(er, er ? null : pong)
+    cb(er, er ? null : pong, data, res)
   })
 }
diff --git a/test/tap/doctor.js b/test/tap/doctor.js
@@ -51,7 +51,7 @@ test('npm doctor', function (t) {
   npm.commands.doctor({'node-url': node_url}, true, function (e, list) {
     t.ifError(e, 'npm loaded successfully')
     t.same(list.length, 9, 'list should have 9 prop')
-    t.same(list[0][1], 'ok', 'npm ping')
+    t.same(list[0][1], 'OK', 'npm ping')
     t.same(list[1][1], 'v' + npm.version, 'npm -v')
     t.same(list[2][1], process.version, 'node -v')
     t.same(list[3][1], common.registry + '/', 'npm config get registry')