fix: populate correct value on yargs.parsed and stop warning on access (

yargs#1412)

* fix: populate correct type for yargs.parsed
* fix: revert the warning message on yargs.parsed access

index.js

@@ -32,7 +32,6 @@ function singletonify (inst) {
         return inst.$0
       })
       Argv.__defineGetter__('parsed', () => {
-        console.warn('In future major releases of yargs, "parsed" will be a private field. Use the return value of ".parse()" or ".argv" instead')
         return inst.parsed
       })
     }

lib/command.js

@@ -174,7 +174,7 @@ module.exports = function command (yargs, usage, validation, globalMiddleware) {
     let numFiles = currentContext.files.length
     const parentCommands = currentContext.commands.slice()
 
-    // what does yargs look like after the buidler is run?
+    // what does yargs look like after the builder is run?
     let innerArgv = parsed.argv
     let innerYargs = null
     let positionalMap = {}
@@ -230,14 +230,22 @@ module.exports = function command (yargs, usage, validation, globalMiddleware) {
 
       innerArgv = applyMiddleware(innerArgv, yargs, middlewares, false)
 
-      const handlerResult = isPromise(innerArgv)
-        ? innerArgv.then(argv => commandHandler.handler(argv))
-        : commandHandler.handler(innerArgv)
+      let handlerResult
+      if (isPromise(innerArgv)) {
+        handlerResult = innerArgv.then(argv => commandHandler.handler(argv))
+      } else {
+        handlerResult = commandHandler.handler(innerArgv)
+      }
 
       if (isPromise(handlerResult)) {
-        handlerResult.catch(error =>
-          yargs.getUsageInstance().fail(null, error)
-        )
+        yargs.getUsageInstance().cacheHelpMessage()
+        handlerResult.catch(error => {
+          try {
+            yargs.getUsageInstance().fail(null, error)
+          } catch (err) {
+            // fail's throwing would cause an unhandled rejection.
+          }
+        })
       }
     }
 

lib/usage.js

@@ -148,6 +148,7 @@ module.exports = function usage (yargs, y18n) {
 
   const defaultGroup = 'Options:'
   self.help = function help () {
+    if (cachedHelpMessage) return cachedHelpMessage
     normalizeAliases()
 
     // handle old demanded API
@@ -403,6 +404,13 @@ module.exports = function usage (yargs, y18n) {
     })
   }
 
+  // if yargs is executing an async handler, we take a snapshot of the
+  // help message to display on failure:
+  let cachedHelpMessage
+  self.cacheHelpMessage = function () {
+    cachedHelpMessage = this.help()
+  }
+
   // given a set of keys, place any keys that are
   // ungrouped under the 'Options:' grouping.
   function addUngroupedKeys (keys, aliases, groups) {

test/command.js

@@ -1426,36 +1426,69 @@ describe('Command', () => {
     })
   })
 
-  // addresses https://github.com/yargs/yargs/issues/510
-  it('fails when the promise returned by the command handler rejects', (done) => {
-    const error = new Error()
-    yargs('foo')
-      .command('foo', 'foo command', noop, (yargs) => Promise.reject(error))
-      .fail((msg, err) => {
-        expect(msg).to.equal(null)
-        expect(err).to.equal(error)
-        done()
+  describe('async', () => {
+    // addresses https://github.com/yargs/yargs/issues/510
+    it('fails when the promise returned by the command handler rejects', (done) => {
+      const error = new Error()
+      yargs('foo')
+        .command('foo', 'foo command', noop, (yargs) => Promise.reject(error))
+        .fail((msg, err) => {
+          expect(msg).to.equal(null)
+          expect(err).to.equal(error)
+          done()
+        })
+        .parse()
+    })
+
+    it('succeeds when the promise returned by the command handler resolves', (done) => {
+      const handler = new Promise((resolve, reject) => {
+        setTimeout(() => {
+          return resolve(true)
+        }, 5)
       })
-      .parse()
-  })
+      const parsed = yargs('foo hello')
+        .command('foo <pos>', 'foo command', () => {}, (yargs) => handler)
+        .fail((msg, err) => {
+          return done(Error('should not have been called'))
+        })
+        .parse()
 
-  it('succeeds when the promise returned by the command handler resolves', (done) => {
-    const handler = new Promise((resolve, reject) => {
-      setTimeout(() => {
-        return resolve(true)
-      }, 5)
-    })
-    const parsed = yargs('foo hello')
-      .command('foo <pos>', 'foo command', () => {}, (yargs) => handler)
-      .fail((msg, err) => {
-        return done(Error('should not have been called'))
+      handler.then(called => {
+        called.should.equal(true)
+        parsed.pos.should.equal('hello')
+        return done()
       })
-      .parse()
+    })
 
-    handler.then(called => {
-      called.should.equal(true)
-      parsed.pos.should.equal('hello')
-      return done()
+    // see: https://github.com/yargs/yargs/issues/1144
+    it('displays error and appropriate help message when handler fails', (done) => {
+      const y = yargs('foo')
+        .command('foo', 'foo command', (yargs) => {
+          yargs.option('bar', {
+            describe: 'bar option'
+          })
+        }, (argv) => {
+          return Promise.reject(Error('foo error'))
+        })
+        .exitProcess(false)
+      // the bug reported in #1144 only happens when
+      // usage.help() is called, this does not occur when
+      // console output is suppressed. tldr; we capture
+      // the log output:
+      let errorLog = ''
+      y._getLoggerInstance().error = (out) => {
+        if (out) errorLog += `${out}\n`
+      }
+      y.parse()
+      // the promise returned by the handler rejects immediately,
+      // so we can wait for a small number of ms:
+      setTimeout(() => {
+        // ensure the appropriate help is displayed:
+        errorLog.should.include('bar option')
+        // ensure error was displayed:
+        errorLog.should.include('foo error')
+        return done()
+      }, 5)
     })
   })
 

yargs.js

@@ -544,10 +544,12 @@ function Yargs (processArgs, cwd, parentRequire) {
     argsert('[string|array] [function|boolean|object] [function]', [args, shortCircuit, _parseFn], arguments.length)
     freeze()
     if (typeof args === 'undefined') {
-      const parsed = self._parseArgs(processArgs)
+      const argv = self._parseArgs(processArgs)
+      const tmpParsed = self.parsed
       unfreeze()
       // TODO: remove this compatibility hack when we release yargs@15.x:
-      return (this.parsed = parsed)
+      self.parsed = tmpParsed
+      return argv
     }
 
     // a context object can optionally be provided, this allows