Skip to content

Commit cceb97d

Browse files
ruyadornowraithgar
andcommitted
fix: empty newline printed to stderr
Starting in v7.7.0 running `npm` (no args) is printing an empty newline to stderr. This fixes that by correctly exiting via errorHandler and avoiding hitting the cb() never called error and adds a test to make sure we avoid that regression moving forward. Fixes: nodejs/node#37678 (comment) Co-authored-by: Gar <gar+gh@danger.computer>
1 parent c55ea98 commit cceb97d

File tree

4 files changed

+52
-4
lines changed

4 files changed

+52
-4
lines changed

lib/cli.js

+1-3
Original file line numberDiff line numberDiff line change
@@ -61,16 +61,14 @@ module.exports = (process) => {
6161
impl(npm.argv, errorHandler)
6262
else {
6363
try {
64-
// I don't know why this is needed but we get a cb() not called if we
65-
// omit it
66-
npm.log.level = 'silent'
6764
if (cmd) {
6865
const didYouMean = require('./utils/did-you-mean.js')
6966
const suggestions = await didYouMean(npm, npm.localPrefix, cmd)
7067
npm.output(`Unknown command: "${cmd}"${suggestions}\n\nTo see a list of supported npm commands, run:\n npm help`)
7168
} else
7269
npm.output(npm.usage)
7370
process.exitCode = 1
71+
return errorHandler()
7472
} catch (err) {
7573
errorHandler(err)
7674
}

smoke-tests/index.js

+13
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,19 @@ t.test('npm init', async t => {
5555
t.equal(pkg.version, '1.0.0', 'should have expected generated version')
5656
})
5757

58+
t.test('npm (no args)', async t => {
59+
const cmd = `"${process.execPath}" "${npmLocation}" --no-audit --no-update-notifier`
60+
const cmdRes = await execAsync(cmd, { cwd: localPrefix, env })
61+
.catch(err => {
62+
t.equal(err.code, 1, 'should exit with error code')
63+
return err
64+
})
65+
66+
t.equal(cmdRes.stderr, '', 'should have no stderr output')
67+
t.matchSnapshot(String(cmdRes.stdout),
68+
'should have expected no args output')
69+
})
70+
5871
t.test('npm install prodDep@version', async t => {
5972
const cmd = `${npmBin} install abbrev@1.0.4`
6073
const cmdRes = await exec(cmd)

tap-snapshots/smoke-tests-index.js-TAP.test.js

+37
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,43 @@
55
* Make sure to inspect the output below. Do not ignore changes!
66
*/
77
'use strict'
8+
exports[`smoke-tests/index.js TAP npm (no args) > should have expected no args output 1`] = `
9+
npm <command>
10+
11+
Usage:
12+
13+
npm install install all the dependencies in your project
14+
npm install <foo> add the <foo> dependency to your project
15+
npm test run this project's tests
16+
npm run <foo> run the script named <foo>
17+
npm <command> -h quick help on <command>
18+
npm -l display usage info for all commands
19+
npm help <term> search for help on <term>
20+
npm help npm more involved overview
21+
22+
All commands:
23+
24+
access, adduser, audit, bin, bugs, cache, ci, completion,
25+
config, dedupe, deprecate, diff, dist-tag, docs, doctor,
26+
edit, exec, explain, explore, find-dupes, fund, get, help,
27+
hook, init, install, install-ci-test, install-test, link,
28+
ll, login, logout, ls, org, outdated, owner, pack, ping,
29+
prefix, profile, prune, publish, rebuild, repo, restart,
30+
root, run-script, search, set, set-script, shrinkwrap, star,
31+
stars, start, stop, team, test, token, uninstall, unpublish,
32+
unstar, update, version, view, whoami
33+
34+
Specify configs in the ini-formatted file:
35+
{CWD}/smoke-tests/index/.npmrc
36+
or on the command line via: npm <command> --key=value
37+
38+
More configuration info: npm help config
39+
Configuration fields: npm help 7 config
40+
41+
npm@7.7.5 {CWD}
42+
43+
`
44+
845
exports[`smoke-tests/index.js TAP npm diff > should have expected diff output 1`] = `
946
diff --git a/package.json b/package.json
1047
index v1.0.4..v1.1.1 100644

test/lib/cli.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ t.test('gracefully handles error printing usage', t => {
182182
throw new Error('test exception')
183183
}
184184
errorHandlerCb = () => {
185-
t.match(errorHandlerCalled, /test exception/)
185+
t.match(errorHandlerCalled, [], 'should call errorHandler with no args')
186186
t.end()
187187
}
188188
cli(proc)

0 commit comments

Comments
 (0)