8 files changed, 25 insertions, 18 deletions
diff --git a/deps/npm/lib/exec.js b/deps/npm/lib/exec.js
index 959fab6663..8c64c2f240 100644
--- a/deps/npm/lib/exec.js
+++ b/deps/npm/lib/exec.js
@@ -68,7 +68,6 @@ class Exec extends BaseCommand {
   async _exec (_args, { locationMsg, path, runPath }) {
     const args = [..._args]
     const call = this.npm.config.get('call')
-    const color = this.npm.config.get('color')
     const {
       flatOptions,
       localBin,
@@ -87,7 +86,6 @@ class Exec extends BaseCommand {
       ...flatOptions,
       args,
       call,
-      color,
       localBin,
       locationMsg,
       log,
@@ -103,7 +101,7 @@ class Exec extends BaseCommand {
 
   async _execWorkspaces (args, filters) {
     await this.setWorkspaces(filters)
-    const color = this.npm.config.get('color')
+    const color = this.npm.color
 
     for (const path of this.workspacePaths) {
       const locationMsg = await getLocationMsg({ color, path })
diff --git a/deps/npm/lib/fund.js b/deps/npm/lib/fund.js
index 92580a756e..1e0fa1ecb9 100644
--- a/deps/npm/lib/fund.js
+++ b/deps/npm/lib/fund.js
@@ -109,7 +109,7 @@ class Fund extends ArboristWorkspaceCmd {
   }
 
   printHuman (fundingInfo) {
-    const color = !!this.npm.color
+    const color = this.npm.color
     const unicode = this.npm.config.get('unicode')
     const seenUrls = new Map()
 
diff --git a/deps/npm/lib/ls.js b/deps/npm/lib/ls.js
index 91e9a9dd3d..7e41892c53 100644
--- a/deps/npm/lib/ls.js
+++ b/deps/npm/lib/ls.js
@@ -67,7 +67,7 @@ class LS extends ArboristWorkspaceCmd {
 
   async ls (args) {
     const all = this.npm.config.get('all')
-    const color = !!this.npm.color
+    const color = this.npm.color
     const depth = this.npm.config.get('depth')
     const dev = this.npm.config.get('dev')
     const development = this.npm.config.get('development')
diff --git a/deps/npm/lib/npm.js b/deps/npm/lib/npm.js
index db3559a384..966d11210c 100644
--- a/deps/npm/lib/npm.js
+++ b/deps/npm/lib/npm.js
@@ -232,7 +232,7 @@ const npm = module.exports = new class extends EventEmitter {
     process.emit('timeEnd', 'npm:load:setTitle')
 
     process.emit('time', 'npm:load:setupLog')
-    this.color = setupLog(this.config)
+    setupLog(this.config)
     process.emit('timeEnd', 'npm:load:setupLog')
     process.env.COLOR = this.color ? '1' : '0'
 
@@ -261,6 +261,12 @@ const npm = module.exports = new class extends EventEmitter {
     return flat
   }
 
+  get color () {
+    // This is a special derived value that takes into consideration not only
+    // the config, but whether or not we are operating in a tty.
+    return this.flatOptions.color
+  }
+
   get lockfileVersion () {
     return 2
   }
diff --git a/deps/npm/lib/run-script.js b/deps/npm/lib/run-script.js
index b94d2fce07..1daaeb9900 100644
--- a/deps/npm/lib/run-script.js
+++ b/deps/npm/lib/run-script.js
@@ -137,7 +137,7 @@ class RunScript extends BaseCommand {
     path = path || this.npm.localPrefix
     const { scripts, name, _id } = await rpj(`${path}/package.json`)
     const pkgid = _id || name
-    const color = !!this.npm.color
+    const color = this.npm.color
 
     if (!scripts)
       return []
diff --git a/deps/npm/lib/utils/config/definitions.js b/deps/npm/lib/utils/config/definitions.js
index abe6bda70d..36b8a84a61 100644
--- a/deps/npm/lib/utils/config/definitions.js
+++ b/deps/npm/lib/utils/config/definitions.js
@@ -438,6 +438,9 @@ define('cidr', {
   flatten,
 })
 
+// This should never be directly used, the flattened value is the derived value
+// and is sent to other modules, and is also exposed as `npm.color` for use
+// inside npm itself.
 define('color', {
   default: !process.env.NO_COLOR || process.env.NO_COLOR === '0',
   usage: '--color|--no-color|--color always',
diff --git a/deps/npm/lib/utils/exit-handler.js b/deps/npm/lib/utils/exit-handler.js
index 95c9655a71..7be138d2c3 100644
--- a/deps/npm/lib/utils/exit-handler.js
+++ b/deps/npm/lib/utils/exit-handler.js
@@ -52,16 +52,17 @@ process.on('exit', code => {
 
   if (!code)
     npm.log.info('ok')
-  else {
+  else
     npm.log.verbose('code', code)
-    if (!exitHandlerCalled) {
-      npm.log.error('', 'Exit handler never called!')
-      console.error('')
-      npm.log.error('', 'This is an error with npm itself. Please report this error at:')
-      npm.log.error('', '    <https://github.com/npm/cli/issues>')
-      // TODO this doesn't have an npm.config.loaded guard
-      writeLogFile()
-    }
+
+  if (!exitHandlerCalled) {
+    process.exitCode = code || 1
+    npm.log.error('', 'Exit handler never called!')
+    console.error('')
+    npm.log.error('', 'This is an error with npm itself. Please report this error at:')
+    npm.log.error('', '    <https://github.com/npm/cli/issues>')
+    // TODO this doesn't have an npm.config.loaded guard
+    writeLogFile()
   }
   // In timing mode we always write the log file
   if (npm.config.loaded && npm.config.get('timing') && !wroteLogFile)
diff --git a/deps/npm/lib/utils/setup-log.js b/deps/npm/lib/utils/setup-log.js
index 9ee79d192d..aaf7fa47e2 100644
--- a/deps/npm/lib/utils/setup-log.js
+++ b/deps/npm/lib/utils/setup-log.js
@@ -18,6 +18,7 @@ module.exports = (config) => {
   const stderrTTY = process.stderr.isTTY
   const dumbTerm = process.env.TERM === 'dumb'
   const stderrNotDumb = stderrTTY && !dumbTerm
+  // this logic is duplicated in the config 'color' flattener
   const enableColorStderr = color === 'always' ? true
     : color === false ? false
     : stderrTTY
@@ -58,6 +59,4 @@ module.exports = (config) => {
     log.enableProgress()
   else
     log.disableProgress()
-
-  return enableColorStdout
 }