Skip to content

Commit

Permalink
chore(refactor): finish passing npm context
Browse files Browse the repository at this point in the history
No more requiring npm as a singleton.  This will now allow us to move
forward with the other refactoring we need to always use the npm object
itself in tests, not a mocked one.
  • Loading branch information
wraithgar committed Jun 15, 2021
1 parent e5abf2a commit b77bff2
Show file tree
Hide file tree
Showing 14 changed files with 137 additions and 294 deletions.
1 change: 1 addition & 0 deletions lib/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ module.exports = (process) => {

const npm = require('../lib/npm.js')
const errorHandler = require('../lib/utils/error-handler.js')
errorHandler.setNpm(npm)

// if npm is called as "npmg" or "npm_g", then
// run in global mode.
Expand Down
66 changes: 0 additions & 66 deletions lib/utils/cache-file.js

This file was deleted.

48 changes: 35 additions & 13 deletions lib/utils/error-handler.js
Original file line number Diff line number Diff line change
@@ -1,28 +1,27 @@
let npm // set by the cli
let cbCalled = false
const log = require('npmlog')
const npm = require('../npm.js')
let itWorked = false
const path = require('path')
const writeFileAtomic = require('write-file-atomic')
const mkdirp = require('mkdirp-infer-owner')
const fs = require('graceful-fs')
let wroteLogFile = false
let exitCode = 0
const errorMessage = require('./error-message.js')
const replaceInfo = require('./replace-info.js')

const cacheFile = require('./cache-file.js')

let logFileName
const getLogFile = () => {
// we call this multiple times, so we need to treat it as a singleton because
// the date is part of the name
if (!logFileName)
logFileName = path.resolve(npm.config.get('cache'), '_logs', (new Date()).toISOString().replace(/[.:]/g, '_') + '-debug.log')

return logFileName
}

const timings = {
version: npm.version,
command: process.argv.slice(2),
logfile: null,
}
const timings = {}
process.on('timing', (name, value) => {
if (timings[name])
timings[name] += value
Expand All @@ -35,9 +34,21 @@ process.on('exit', code => {
log.disableProgress()
if (npm.config && npm.config.loaded && npm.config.get('timing')) {
try {
timings.logfile = getLogFile()
cacheFile.append('_timing.json', JSON.stringify(timings) + '\n')
} catch (_) {
const file = path.resolve(npm.config.get('cache'), '_timing.json')
const dir = path.dirname(npm.config.get('cache'))
mkdirp.sync(dir)

fs.appendFileSync(file, JSON.stringify({
command: process.argv.slice(2),
logfile: getLogFile(),
version: npm.version,
...timings,
}) + '\n')

const st = fs.lstatSync(path.dirname(npm.config.get('cache')))
fs.chownSync(dir, st.uid, st.gid)
fs.chownSync(file, st.uid, st.gid)
} catch (ex) {
// ignore
}
}
Expand Down Expand Up @@ -174,7 +185,7 @@ const errorHandler = (er) => {
log.error(k, v)
}

const msg = errorMessage(er)
const msg = errorMessage(er, npm)
for (const errline of [...msg.summary, ...msg.detail])
log.error(...errline)

Expand Down Expand Up @@ -214,7 +225,15 @@ const writeLogFile = () => {
logOutput += line + os.EOL
})
})
cacheFile.write(getLogFile(), logOutput)

const file = getLogFile()
const dir = path.dirname(file)
mkdirp.sync(dir)
writeFileAtomic.sync(file, logOutput)

const st = fs.lstatSync(path.dirname(npm.config.get('cache')))
fs.chownSync(dir, st.uid, st.gid)
fs.chownSync(file, st.uid, st.gid)

// truncate once it's been written.
log.record.length = 0
Expand All @@ -226,3 +245,6 @@ const writeLogFile = () => {

module.exports = errorHandler
module.exports.exit = exit
module.exports.setNpm = (n) => {
npm = n
}
7 changes: 3 additions & 4 deletions lib/utils/error-message.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
const npm = require('../npm.js')
const { format } = require('util')
const { resolve } = require('path')
const nameValidator = require('validate-npm-package-name')
const npmlog = require('npmlog')
const replaceInfo = require('./replace-info.js')
const { report: explainEresolve } = require('./explain-eresolve.js')
const { report } = require('./explain-eresolve.js')

module.exports = (er) => {
module.exports = (er, npm) => {
const short = []
const detail = []

Expand All @@ -19,7 +18,7 @@ module.exports = (er) => {
case 'ERESOLVE':
short.push(['ERESOLVE', er.message])
detail.push(['', ''])
detail.push(['', explainEresolve(er)])
detail.push(['', report(er, npm.color, resolve(npm.cache, 'eresolve-report.txt'))])
break

case 'ENOLOCK': {
Expand Down
19 changes: 4 additions & 15 deletions lib/utils/explain-eresolve.js
Original file line number Diff line number Diff line change
@@ -1,20 +1,14 @@
// this is called when an ERESOLVE error is caught in the error-handler,
// or when there's a log.warn('eresolve', msg, explanation), to turn it
// into a human-intelligible explanation of what's wrong and how to fix.
//
// TODO: abstract out the explainNode methods into a separate util for
// use by a future `npm explain <path || spec>` command.

const npm = require('../npm.js')
const { writeFileSync } = require('fs')
const { resolve } = require('path')
const { explainEdge, explainNode, printNode } = require('./explain-dep.js')

// expl is an explanation object that comes from Arborist. It looks like:
// Depth is how far we want to want to descend into the object making a report.
// The full report (ie, depth=Infinity) is always written to the cache folder
// at ${cache}/eresolve-report.txt along with full json.
const explainEresolve = (expl, color, depth) => {
const explain = (expl, color, depth) => {
const { edge, current, peerConflict, currentEdge } = expl

const out = []
Expand Down Expand Up @@ -42,9 +36,7 @@ const explainEresolve = (expl, color, depth) => {
}

// generate a full verbose report and tell the user how to fix it
const report = (expl, depth = 4) => {
const fullReport = resolve(npm.cache, 'eresolve-report.txt')

const report = (expl, color, fullReport) => {
const orNoStrict = expl.strictPeerDeps ? '--no-strict-peer-deps, ' : ''
const fix = `Fix the upstream dependency conflict, or retry
this command with ${orNoStrict}--force, or --legacy-peer-deps
Expand All @@ -54,7 +46,7 @@ to accept an incorrect (and potentially broken) dependency resolution.`
${new Date().toISOString()}
${explainEresolve(expl, false, Infinity)}
${explain(expl, false, Infinity)}
${fix}
Expand All @@ -63,13 +55,10 @@ Raw JSON explanation object:
${JSON.stringify(expl, null, 2)}
`, 'utf8')

return explainEresolve(expl, npm.color, depth) +
return explain(expl, color, 4) +
`\n\n${fix}\n\nSee ${fullReport} for a full report.`
}

// the terser explain method for the warning when using --force
const explain = (expl, depth = 2) => explainEresolve(expl, npm.color, depth)

module.exports = {
explain,
report,
Expand Down
27 changes: 13 additions & 14 deletions lib/utils/setup-log.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,22 @@ module.exports = (config) => {

const { warn } = log

const stdoutTTY = process.stdout.isTTY
const stderrTTY = process.stderr.isTTY
const dumbTerm = process.env.TERM === 'dumb'
const stderrNotDumb = stderrTTY && !dumbTerm
const enableColorStderr = color === 'always' ? true
: color === false ? false
: stderrTTY

const enableColorStdout = color === 'always' ? true
: color === false ? false
: stdoutTTY

log.warn = (heading, ...args) => {
if (heading === 'ERESOLVE' && args[1] && typeof args[1] === 'object') {
warn(heading, args[0])
return warn('', explain(args[1]))
return warn('', explain(args[1], enableColorStdout, 2))
}
return warn(heading, ...args)
}
Expand All @@ -29,19 +41,6 @@ module.exports = (config) => {

log.heading = config.get('heading') || 'npm'

const stdoutTTY = process.stdout.isTTY
const stderrTTY = process.stderr.isTTY
const dumbTerm = process.env.TERM === 'dumb'
const stderrNotDumb = stderrTTY && !dumbTerm

const enableColorStderr = color === 'always' ? true
: color === false ? false
: stderrTTY

const enableColorStdout = color === 'always' ? true
: color === false ? false
: stdoutTTY

if (enableColorStderr)
log.enableColor()
else
Expand Down
2 changes: 1 addition & 1 deletion tap-snapshots/test/lib/utils/error-handler.js.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
exports[`test/lib/utils/error-handler.js TAP handles unknown error > should have expected log contents for unknown error 1`] = `
0 verbose code 1
1 error foo A complete log of this run can be found in:
1 error foo {CWD}/cachefolder/_logs/expecteddate-debug.log
1 error foo {CWD}/test/lib/utils/tap-testdir-error-handler/_logs/expecteddate-debug.log
2 verbose stack Error: ERROR
3 verbose cwd {CWD}
4 verbose Foo 1.0.0
Expand Down
Loading

0 comments on commit b77bff2

Please sign in to comment.