Log failed optional platform dependencies as info

lib/install.js

@@ -119,6 +119,7 @@ var unlock = locker.unlock
 var parseJSON = require('./utils/parse-json.js')
 var output = require('./utils/output.js')
 var saveMetrics = require('./utils/metrics.js').save
+var logErrorMessage = require('./utils/log-error-message.js')
 
 // install specific libraries
 var copyTree = require('./install/copy-tree.js')
@@ -342,13 +343,7 @@ Installer.prototype.run = function (_cb) {
     if (installEr) self.failing = true
     chain(postInstallSteps, function (postInstallEr) {
       if (installEr && postInstallEr) {
-        var msg = errorMessage(postInstallEr)
-        msg.summary.forEach(function (logline) {
-          log.warn.apply(log, logline)
-        })
-        msg.detail.forEach(function (logline) {
-          log.verbose.apply(log, logline)
-        })
+        logErrorMessage(postInstallEr, 'warn', 'verbose')
       }
       cb(installEr || postInstallEr, self.getInstalledModules(), self.idealTree)
     })
@@ -736,13 +731,7 @@ Installer.prototype.printWarnings = function (cb) {
     if (warning.code === 'EPACKAGEJSON' && self.global) return
     if (warning.code === 'ENOTDIR') return
     warned = true
-    var msg = errorMessage(warning)
-    msg.summary.forEach(function (logline) {
-      log.warn.apply(log, logline)
-    })
-    msg.detail.forEach(function (logline) {
-      log.verbose.apply(log, logline)
-    })
+    logErrorMessage(warning, 'warn', 'verbose')
   })
   if (warned && log.levels[npm.config.get('loglevel')] <= log.levels.warn) console.error()
   cb()

lib/install/report-optional-failure.js

@@ -1,6 +1,7 @@
 'use strict'
 var path = require('path')
 var moduleName = require('../utils/module-name.js')
+var logErrorMessage = require('../utils/log-error-message.js')
 
 module.exports = reportOptionalFailure
 
@@ -27,5 +28,11 @@ function reportOptionalFailure (tree, what, error) {
 
   error.optional = id
   error.location = location
-  topTree.warnings.push(error)
+  if (error.code === 'EBADPLATFORM') {
+    // Optional dependencies irrelevant to the current OS probably aren't worth
+    // always warning users about, so indicate this issue at a lower log level.
+    logErrorMessage(error, 'info', 'verbose')
+  } else {
+    topTree.warnings.push(error)
+  }
 }

lib/utils/error-handler.js

@@ -12,7 +12,7 @@ var exitCode = 0
 var rollbacks = npm.rollbacks
 var chain = require('slide').chain
 var writeFileAtomic = require('write-file-atomic')
-var errorMessage = require('./error-message.js')
+var logErrorMessage = require('./log-error-message.js')
 var stopMetrics = require('./metrics.js').stop
 var mkdirp = require('mkdirp')
 var fs = require('graceful-fs')
@@ -198,10 +198,7 @@ function errorHandler (er) {
     if (v) log.error(k, v)
   })
 
-  var msg = errorMessage(er)
-  msg.summary.concat(msg.detail).forEach(function (errline) {
-    log.error.apply(log, errline)
-  })
+  var msg = logErrorMessage(er, 'error', 'error')
   if (npm.config && npm.config.get('json')) {
     var error = {
       error: {

lib/utils/log-error-message.js

@@ -0,0 +1,16 @@
+'use strict'
+var log = require('npmlog')
+var errorMessage = require('./error-message.js')
+
+module.exports = logErrorMessage
+
+function logErrorMessage (er, summaryLevel, detailLevel) {
+  var msg = errorMessage(er)
+  msg.summary.forEach(function (logline) {
+    log[summaryLevel].apply(log, logline)
+  })
+  msg.detail.forEach(function (logline) {
+    log[detailLevel].apply(log, logline)
+  })
+  return msg
+}

test/tap/full-warning-messages.js

@@ -70,15 +70,15 @@ function exists (t, filepath, msg) {
     t.pass(msg)
     return true
   } catch (ex) {
-    t.fail(msg, {found: null, wanted: 'exists', compare: 'fs.stat(' + filepath + ')'})
+    t.fail(msg, { found: null, wanted: 'exists', compare: 'fs.stat(' + filepath + ')' })
     return false
   }
 }
 
 function notExists (t, filepath, msg) {
   try {
     fs.statSync(filepath)
-    t.fail(msg, {found: 'exists', wanted: null, compare: 'fs.stat(' + filepath + ')'})
+    t.fail(msg, { found: 'exists', wanted: null, compare: 'fs.stat(' + filepath + ')' })
     return true
   } catch (ex) {
     t.pass(msg)
@@ -87,25 +87,29 @@ function notExists (t, filepath, msg) {
 }
 
 test('tree-style', function (t) {
-  common.npm(['install', '--json', '--loglevel=warn'], {cwd: base}, function (err, code, stdout, stderr) {
+  common.npm(['install', '--json', '--loglevel=info'], { cwd: base }, function (err, code, stdout, stderr) {
     if (err) throw err
     t.is(code, 0, 'result code')
     var result = JSON.parse(stdout)
     t.is(result.added.length, 1, 'only added one module')
     t.is(result.added[0].name, 'modA', 'modA got installed')
-    t.is(result.warnings.length, 1, 'one warning')
-    var stderrlines = stderr.trim().split(/\n/)
-    t.is(stderrlines.length, 2, 'two lines of warnings')
-    t.match(stderr, /SKIPPING OPTIONAL DEPENDENCY/, 'expected optional failure warning in stderr')
+    t.is(result.warnings.length, 0, 'no warnings')
+    t.match(stderr, /SKIPPING OPTIONAL DEPENDENCY/, 'expected optional failure info in stderr')
     t.match(stderr, /Unsupported platform/, 'reason for optional failure in stderr')
-    t.match(result.warnings[0], /SKIPPING OPTIONAL DEPENDENCY/, 'expected optional failure warning in JSON')
-    t.match(result.warnings[0], /Unsupported platform/, 'reason for optional failure in JSON')
     exists(t, modJoin(base, 'modA'), 'module A')
     notExists(t, modJoin(base, 'modB'), 'module B')
     t.done()
   })
 })
 
+test('tree-style', function (t) {
+  common.npm(['install', '--loglevel=warn'], { cwd: base }, function (err, code, stdout, stderr) {
+    if (err) throw err
+    t.is(stderr.length, 0, 'EBADPLATFORM errors for optional deps are excluded from warnings')
+    t.done()
+  })
+})
+
 test('cleanup', function (t) {
   cleanup()
   t.end()