fix: add first param titles to logs where missing (#7533)

The first argument to all `log.method()` calls gets formatted differently with a color. So calls to these should always be a short descriptive title or an empty string.
npm · May 15, 2024 · 12f103c · 12f103c
1 parent badeac2
commit 12f103c
Show file tree

Hide file tree

Showing 7 changed files with 36 additions and 30 deletions.
diff --git a/lib/commands/audit.js b/lib/commands/audit.js
@@ -90,7 +90,7 @@ class Audit extends ArboristWorkspaceCmd {
       )
     }
 
-    log.verbose('loading installed dependencies')
+    log.verbose('audit', 'loading installed dependencies')
     const Arborist = require('@npmcli/arborist')
     const opts = {
       ...this.npm.flatOptions,

diff --git a/lib/commands/cache.js b/lib/commands/cache.js
@@ -132,7 +132,7 @@ class Cache extends BaseCommand {
       try {
         entry = await cacache.get(cachePath, key)
       } catch (err) {
-        log.warn(`Not Found: ${key}`)
+        log.warn('cache', `Not Found: ${key}`)
         break
       }
       output.standard(`Deleted: ${key}`)

diff --git a/tap-snapshots/test/lib/cli/exit-handler.js.test.cjs b/tap-snapshots/test/lib/cli/exit-handler.js.test.cjs
@@ -7,10 +7,10 @@
 'use strict'
 exports[`test/lib/cli/exit-handler.js TAP handles unknown error with logs and debug file > debug file contents 1`] = `
 XX timing npm:load:whichnode Completed in {TIME}ms
-XX silly config:load:file:{CWD}/npmrc
-XX silly config:load:file:{CWD}/prefix/.npmrc
-XX silly config:load:file:{CWD}/home/.npmrc
-XX silly config:load:file:{CWD}/global/etc/npmrc
+XX silly config load:file:{CWD}/npmrc
+XX silly config load:file:{CWD}/prefix/.npmrc
+XX silly config load:file:{CWD}/home/.npmrc
+XX silly config load:file:{CWD}/global/etc/npmrc
 XX timing npm:load:configload Completed in {TIME}ms
 XX timing npm:load:mkdirpcache Completed in {TIME}ms
 XX timing npm:load:mkdirplogs Completed in {TIME}ms
@@ -37,10 +37,10 @@ XX error A complete log of this run can be found in: {CWD}/cache/_logs/{DATE}-de
 
 exports[`test/lib/cli/exit-handler.js TAP handles unknown error with logs and debug file > logs 1`] = `
 timing npm:load:whichnode Completed in {TIME}ms
-silly config:load:file:{CWD}/npmrc
-silly config:load:file:{CWD}/prefix/.npmrc
-silly config:load:file:{CWD}/home/.npmrc
-silly config:load:file:{CWD}/global/etc/npmrc
+silly config load:file:{CWD}/npmrc
+silly config load:file:{CWD}/prefix/.npmrc
+silly config load:file:{CWD}/home/.npmrc
+silly config load:file:{CWD}/global/etc/npmrc
 timing npm:load:configload Completed in {TIME}ms
 timing npm:load:mkdirpcache Completed in {TIME}ms
 timing npm:load:mkdirplogs Completed in {TIME}ms

diff --git a/tap-snapshots/test/lib/commands/shrinkwrap.js.test.cjs b/tap-snapshots/test/lib/commands/shrinkwrap.js.test.cjs
@@ -49,7 +49,7 @@ exports[`test/lib/commands/shrinkwrap.js TAP with hidden lockfile ancient upgrad
     "created a lockfile as npm-shrinkwrap.json with version 3"
   ],
   "warn": [
-    "Converting lock file (npm-shrinkwrap.json) from v1 -> v3"
+    "shrinkwrap Converting lock file (npm-shrinkwrap.json) from v1 -> v3"
   ]
 }
 `
@@ -98,7 +98,7 @@ exports[`test/lib/commands/shrinkwrap.js TAP with hidden lockfile existing downg
     "created a lockfile as npm-shrinkwrap.json with version 1"
   ],
   "warn": [
-    "Converting lock file (npm-shrinkwrap.json) from v2 -> v1"
+    "shrinkwrap Converting lock file (npm-shrinkwrap.json) from v2 -> v1"
   ]
 }
 `
@@ -125,7 +125,7 @@ exports[`test/lib/commands/shrinkwrap.js TAP with hidden lockfile existing upgra
     "created a lockfile as npm-shrinkwrap.json with version 3"
   ],
   "warn": [
-    "Converting lock file (npm-shrinkwrap.json) from v2 -> v3"
+    "shrinkwrap Converting lock file (npm-shrinkwrap.json) from v2 -> v3"
   ]
 }
 `
@@ -188,7 +188,7 @@ exports[`test/lib/commands/shrinkwrap.js TAP with npm-shrinkwrap.json ancient >
     "npm-shrinkwrap.json updated to version 3"
   ],
   "warn": [
-    "Converting lock file (npm-shrinkwrap.json) from v1 -> v3"
+    "shrinkwrap Converting lock file (npm-shrinkwrap.json) from v1 -> v3"
   ]
 }
 `
@@ -217,7 +217,7 @@ exports[`test/lib/commands/shrinkwrap.js TAP with npm-shrinkwrap.json ancient up
     "npm-shrinkwrap.json updated to version 3"
   ],
   "warn": [
-    "Converting lock file (npm-shrinkwrap.json) from v1 -> v3"
+    "shrinkwrap Converting lock file (npm-shrinkwrap.json) from v1 -> v3"
   ]
 }
 `
@@ -266,7 +266,7 @@ exports[`test/lib/commands/shrinkwrap.js TAP with npm-shrinkwrap.json existing d
     "npm-shrinkwrap.json updated to version 1"
   ],
   "warn": [
-    "Converting lock file (npm-shrinkwrap.json) from v2 -> v1"
+    "shrinkwrap Converting lock file (npm-shrinkwrap.json) from v2 -> v1"
   ]
 }
 `
@@ -295,7 +295,7 @@ exports[`test/lib/commands/shrinkwrap.js TAP with npm-shrinkwrap.json existing u
     "npm-shrinkwrap.json updated to version 3"
   ],
   "warn": [
-    "Converting lock file (npm-shrinkwrap.json) from v2 -> v3"
+    "shrinkwrap Converting lock file (npm-shrinkwrap.json) from v2 -> v3"
   ]
 }
 `
@@ -322,7 +322,7 @@ exports[`test/lib/commands/shrinkwrap.js TAP with package-lock.json ancient > mu
     "package-lock.json has been renamed to npm-shrinkwrap.json and updated to version 3"
   ],
   "warn": [
-    "Converting lock file (npm-shrinkwrap.json) from v1 -> v3"
+    "shrinkwrap Converting lock file (npm-shrinkwrap.json) from v1 -> v3"
   ]
 }
 `
@@ -351,7 +351,7 @@ exports[`test/lib/commands/shrinkwrap.js TAP with package-lock.json ancient upgr
     "package-lock.json has been renamed to npm-shrinkwrap.json and updated to version 3"
   ],
   "warn": [
-    "Converting lock file (npm-shrinkwrap.json) from v1 -> v3"
+    "shrinkwrap Converting lock file (npm-shrinkwrap.json) from v1 -> v3"
   ]
 }
 `
@@ -400,7 +400,7 @@ exports[`test/lib/commands/shrinkwrap.js TAP with package-lock.json existing dow
     "package-lock.json has been renamed to npm-shrinkwrap.json and updated to version 1"
   ],
   "warn": [
-    "Converting lock file (npm-shrinkwrap.json) from v2 -> v1"
+    "shrinkwrap Converting lock file (npm-shrinkwrap.json) from v2 -> v1"
   ]
 }
 `
@@ -429,7 +429,7 @@ exports[`test/lib/commands/shrinkwrap.js TAP with package-lock.json existing upg
     "package-lock.json has been renamed to npm-shrinkwrap.json and updated to version 3"
   ],
   "warn": [
-    "Converting lock file (npm-shrinkwrap.json) from v2 -> v3"
+    "shrinkwrap Converting lock file (npm-shrinkwrap.json) from v2 -> v3"
   ]
 }
 `
diff --git a/workspaces/arborist/lib/shrinkwrap.js b/workspaces/arborist/lib/shrinkwrap.js
@@ -1153,7 +1153,8 @@ class Shrinkwrap {
       && this.originalLockfileVersion !== this.lockfileVersion
     ) {
       log.warn(
-      `Converting lock file (${relative(process.cwd(), this.filename)}) from v${this.originalLockfileVersion} -> v${this.lockfileVersion}`
+        'shrinkwrap',
+        `Converting lock file (${relative(process.cwd(), this.filename)}) from v${this.originalLockfileVersion} -> v${this.lockfileVersion}`
       )
     }
 

diff --git a/workspaces/config/lib/index.js b/workspaces/config/lib/index.js
@@ -585,7 +585,7 @@ class Config {
 
   async #loadFile (file, type) {
     // only catch the error from readFile, not from the loadObject call
-    log.silly(`config:load:file:${file}`)
+    log.silly('config', `load:file:${file}`)
     await readFile(file, 'utf8').then(
       data => {
         const parsedConfig = ini.parse(data)
@@ -684,15 +684,15 @@ class Config {
           if (w === this.localPrefix) {
             // see if there's a .npmrc file in the workspace, if so log a warning
             if (await fileExists(this.localPrefix, '.npmrc')) {
-              log.warn(`ignoring workspace config at ${this.localPrefix}/.npmrc`)
+              log.warn('config', `ignoring workspace config at ${this.localPrefix}/.npmrc`)
             }
 
             // set the workspace in the default layer, which allows it to be overridden easily
             const { data } = this.data.get('default')
             data.workspace = [this.localPrefix]
             this.localPrefix = p
             this.localPackage = hasPackageJson
-            log.info(`found workspace root at ${this.localPrefix}`)
+            log.info('config', `found workspace root at ${this.localPrefix}`)
             // we found a root, so we return now
             return
           }

diff --git a/workspaces/config/test/index.js b/workspaces/config/test/index.js
@@ -1185,7 +1185,8 @@ t.test('workspaces', async (t) => {
     t.same(config.get('workspace'), [join(path, 'workspaces', 'one')], 'set the workspace')
     const info = logs.filter(l => l[0] === 'info')
     t.equal(info.length, 1, 'got one log message')
-    t.match(info[0], ['info', /^found workspace root at/], 'logged info about workspace root')
+    t.match(info[0],
+      ['info', 'config', /^found workspace root at/], 'logged info about workspace root')
   })
 
   t.test('finds other workspace parent', async (t) => {
@@ -1207,7 +1208,8 @@ t.test('workspaces', async (t) => {
     t.same(config.get('workspace'), ['../two'], 'kept the specified workspace')
     const info = logs.filter(l => l[0] === 'info')
     t.equal(info.length, 1, 'got one log message')
-    t.match(info[0], ['info', /^found workspace root at/], 'logged info about workspace root')
+    t.match(info[0],
+      ['info', 'config', /^found workspace root at/], 'logged info about workspace root')
   })
 
   t.test('warns when workspace has .npmrc', async (t) => {
@@ -1229,8 +1231,10 @@ t.test('workspaces', async (t) => {
     t.same(config.get('workspace'), [join(path, 'workspaces', 'three')], 'kept the workspace')
     const filtered = logs.filter(l => l[0] === 'info' || l[0] === 'warn')
     t.equal(filtered.length, 2, 'got two log messages')
-    t.match(filtered[0], ['warn', /^ignoring workspace config/], 'warned about ignored config')
-    t.match(filtered[1], ['info', /^found workspace root at/], 'logged info about workspace root')
+    t.match(filtered[0],
+      ['warn', 'config', /^ignoring workspace config/], 'warned about ignored config')
+    t.match(filtered[1],
+      ['info', 'config', /^found workspace root at/], 'logged info about workspace root')
   })
 
   t.test('prefix skips auto detect', async (t) => {
@@ -1364,7 +1368,8 @@ t.test('workspaces', async (t) => {
     t.same(config.get('workspace'), [join(path, 'workspaces', 'one')], 'set the workspace')
     const filtered = logs.filter(l => l[0] !== 'silly')
     t.equal(filtered.length, 1, 'got one log message')
-    t.match(filtered[0], ['info', /^found workspace root at/], 'logged info about workspace root')
+    t.match(filtered[0],
+      ['info', 'config', /^found workspace root at/], 'logged info about workspace root')
   })
 })