Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Log failed optional platform dependencies as info #169

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 3 additions & 14 deletions lib/install.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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)
})
Expand Down Expand Up @@ -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()
Expand Down
9 changes: 8 additions & 1 deletion lib/install/report-optional-failure.js
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -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)
}
}
7 changes: 2 additions & 5 deletions lib/utils/error-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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: {
Expand Down
16 changes: 16 additions & 0 deletions lib/utils/log-error-message.js
Original file line number Diff line number Diff line change
@@ -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
}
22 changes: 13 additions & 9 deletions test/tap/full-warning-messages.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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()
Expand Down