fix(ls/search) --json arg error also sent to stdout

lib/ls.js

@@ -7,6 +7,7 @@ const chalk = require('chalk')
 const Arborist = require('@npmcli/arborist')
 const { breadth } = require('treeverse')
 const npa = require('npm-package-arg')
+const errorMessage = require('./utils/error-message')
 
 const completion = require('./utils/completion/installed-deep.js')
 
@@ -189,13 +190,32 @@ class LS extends ArboristWorkspaceCmd {
  const [rootError] = tree.errors.filter(e =>
  e.code === 'EJSONPARSE' && e.path === resolve(path, 'package.json'))
 
- this.npm.output(
- json
- ? jsonOutput({ path, problems, result, rootError, seenItems })
- : parseable
- ? parseableOutput({ seenNodes, global, long })
- : humanOutput({ color, result, seenItems, unicode })
- )
+ const shouldThrow = problems.size &&
+ ![...problems].every(problem => problem.startsWith('extraneous:'))
+
+ let out
+ if (json) {
+ let err
+ if (shouldThrow) {
+ const code = 'ELSPROBLEMS'
+ const errorMsg = errorMessage(
+ { code: code, message: [...problems].join(EOL) },
+ this.npm
+ )
+ err = {
+ code: code,
+ summary: errorMsg.summary.map(line => line.slice(1).join(' ')).join('\n'),
+ detail: '',
+ }
+ }
+
+ out = jsonOutput({ path, problems, result, rootError, seenItems, err })
+ } else if (parseable)
+ out = parseableOutput({ seenNodes, global, long })
+ else
+ out = humanOutput({ color, result, seenItems, unicode })
+
+ this.npm.output(out)
 
  // if filtering items, should exit with error code on no results
  if (result && !result[_include] && args.length)
@@ -208,9 +228,6 @@ class LS extends ArboristWorkspaceCmd {
  )
  }
 
- const shouldThrow = problems.size &&
- ![...problems].every(problem => problem.startsWith('extraneous:'))
-
  if (shouldThrow) {
  throw Object.assign(
  new Error([...problems].join(EOL)),
@@ -518,7 +535,7 @@ const humanOutput = ({ color, result, seenItems, unicode }) => {
  return color ? chalk.reset(archyOutput) : archyOutput
 }
 
-const jsonOutput = ({ path, problems, result, rootError, seenItems }) => {
+const jsonOutput = ({ path, problems, result, rootError, seenItems, err }) => {
  if (problems.size)
  result.problems = [...problems]
 
@@ -530,6 +547,9 @@ const jsonOutput = ({ path, problems, result, rootError, seenItems }) => {
  result.invalid = true
  }
 
+ if (err !== undefined)
+ result.error = err
+
  // we need to traverse the entire tree in order to determine which items
  // should be included (since a nested transitive included dep will make it
  // so that all its ancestors should be displayed)

lib/search.js

@@ -2,6 +2,7 @@ const Minipass = require('minipass')
 const Pipeline = require('minipass-pipeline')
 const libSearch = require('libnpmsearch')
 const log = require('npmlog')
+const errorMessage = require('./utils/error-message')
 
 const formatPackageStream = require('./search/format-package-stream.js')
 const packageFilter = require('./search/package-filter.js')
@@ -70,8 +71,22 @@ class Search extends BaseCommand {
  exclude: prepareExcludes(this.npm.flatOptions.search.exclude),
  }
 
- if (opts.include.length === 0)
+ if (opts.include.length === 0) {
+ if (this.npm.config.get('json')) {
+ const errorMsg = errorMessage(
+ { code: null, message: 'search must be called with arguments' },
+ this.npm
+ )
+ const error = {
+ code: null,
+ summary: errorMsg.summary.map(line => line.slice(1).join(' ')).join('\n'),
+ detail: '',
+ }
+ this.npm.output(JSON.stringify({ error: error }, null, 2))
+ }
+
  throw new Error('search must be called with arguments')
+ }
 
  // Used later to figure out whether we had any packages go out
  let anyOutput = false

test/lib/ls.js

@@ -3057,6 +3057,11 @@ t.test('ls --json', (t) => {
  'invalid: [email protected] {CWD}/tap-testdir-ls-ls---json-missing-invalid-extraneous/node_modules/foo',
  'missing: ipsum@^1.0.0, required by [email protected]',
  ],
+ error: {
+ code: 'ELSPROBLEMS',
+ summary: 'extraneous: [email protected] {CWD}/tap-testdir-ls-ls---json-missing-invalid-extraneous/node_modules/chai/ninvalid: [email protected] {CWD}/tap-testdir-ls-ls---json-missing-invalid-extraneous/node_modules/foo/nmissing: ipsum@^1.0.0, required by [email protected]',
+ detail: '',
+ },
  dependencies: {
  foo: {
  version: '1.0.0',
@@ -3753,6 +3758,11 @@ t.test('ls --json', (t) => {
  problems: [
  'invalid: [email protected] {CWD}/tap-testdir-ls-ls---json-unmet-peer-dep/node_modules/peer-dep',
  ],
+ error: {
+ code: 'ELSPROBLEMS',
+ summary: 'invalid: [email protected] {CWD}/tap-testdir-ls-ls---json-unmet-peer-dep/node_modules/peer-dep',
+ detail: '',
+ },
  dependencies: {
  'peer-dep': {
  version: '1.0.0',
@@ -3814,6 +3824,11 @@ t.test('ls --json', (t) => {
  problems: [
  'invalid: [email protected] {CWD}/tap-testdir-ls-ls---json-unmet-optional-dep/node_modules/optional-dep', // mismatching optional deps get flagged in problems
  ],
+ error: {
+ code: 'ELSPROBLEMS',
+ summary: 'invalid: [email protected] {CWD}/tap-testdir-ls-ls---json-unmet-optional-dep/node_modules/optional-dep',
+ detail: '',
+ },
  dependencies: {
  'optional-dep': {
  version: '1.0.0',
@@ -4748,6 +4763,11 @@ t.test('ls --package-lock-only', (t) => {
  'invalid: [email protected] {CWD}/tap-testdir-ls-ls---package-lock-only-ls---package-lock-only---json-missing-invalid-extraneous/node_modules/foo',
  'missing: ipsum@^1.0.0, required by [email protected]',
  ],
+ error: {
+ code: 'ELSPROBLEMS',
+ summary: 'invalid: [email protected] {CWD}/tap-testdir-ls-ls---package-lock-only-ls---package-lock-only---json-missing-invalid-extraneous/node_modules/foo/nmissing: ipsum@^1.0.0, required by [email protected]',
+ detail: '',
+ },
  dependencies: {
  foo: {
  version: '1.0.0',

test/lib/search.js

@@ -57,6 +57,50 @@ t.test('no args', t => {
  })
 })
 
+t.test('no args --json', t => {
+ const src = new Minipass()
+ src.objectMode = true
+
+ npm.flatOptions.json = true
+ config.json = true
+ const libnpmsearch = {
+ stream () {
+ return src
+ },
+ }
+
+ const Search = t.mock('../../lib/search.js', {
+ ...mocks,
+ libnpmsearch,
+ })
+ const search = new Search(npm)
+
+ search.exec([], err => {
+ const parsedResult = JSON.parse(result)
+
+ t.match(
+ err,
+ /search must be called with arguments/,
+ 'should throw usage instructions'
+ )
+
+ t.same(
+ parsedResult,
+ {
+ error: {
+ code: null,
+ detail: '',
+ summary: 'search must be called with arguments',
+ },
+ },
+ 'should output error to STDOUT'
+ )
+
+ config.json = false
+ t.end()
+ })
+})
+
 t.test('search <name>', t => {
  const src = new Minipass()
  src.objectMode = true