Crash on second call to help command

[scriptdaemon]

EDIT: The same error occurs doing anything a second time when using help().

Version:

6.6.0

Error:

/mnt/c/Users/kwilliams/Documents/Projects/cheevobot/node_modules/yargs/yargs.js:899
          return k.length > 1
                  ^

TypeError: Cannot read property 'length' of undefined
    at /mnt/c/Users/kwilliams/Documents/Projects/cheevobot/node_modules/yargs/yargs.js:899:19
    at Array.filter (native)
    at parseArgs (/mnt/c/Users/kwilliams/Documents/Projects/cheevobot/node_modules/yargs/yargs.js:898:42)
    at Object.Yargs.self.parse (/mnt/c/Users/kwilliams/Documents/Projects/cheevobot/node_modules/yargs/yargs.js:499:18)
    at SteamBot.exec (/mnt/c/Users/kwilliams/Documents/Projects/cheevobot/lib/steam-bot.js:81:18)
    at SteamUser.SteamBot._client.on (/mnt/c/Users/kwilliams/Documents/Projects/cheevobot/lib/steam-bot.js:35:12)
    at emitThree (events.js:116:13)
    at SteamUser.emit (events.js:195:7)
    at SteamUser._emitIdEvent (/mnt/c/Users/kwilliams/Documents/Projects/cheevobot/node_modules/steam-user/components/utility.js:29:12)
    at SteamUser._handlers.(anonymous function) (/mnt/c/Users/kwilliams/Documents/Projects/cheevobot/node_modules/steam-user/components/chat.js:282:9)

Code:

'use strict'

// -- Dependencies -------------------------------------------------------------

// Node.js API
const path = require('path')

// Node packaged modules
const confit = require('confit')
const SteamUser = require('steam-user')
const yargs = require('yargs/yargs')

// Local modules
const dice = require('./plugins/cmds/dice')
const uptime = require('./plugins/cmds/uptime')

// -- Public Interface ---------------------------------------------------------

class SteamBot {
  constructor () {
    this._client = new SteamUser()
    this._client.on('friendOrChatMessage', (sender, msg, chat) => {
      this.exec(msg, { sender, chat })
    })
  }

  start (cb) {
    confit(path.resolve('config/')).create((err, config) => {
      if (err) return cb(err)

      // Build command parser
      this._parser = yargs()
      this._parser.command({
        'command': '!ch <cmd>', // config.get('cmd'),
        'description': 'test desc', // config.get('description'),
        'builder': yargs => {
          yargs.command(dice(this))
          yargs.command(uptime(this))
          yargs.help()
          yargs.version()
        }
      })

      this._client.logOn(config.get('login'))
      this._client.on('loggedOn', response => {
        this._client.setPersona(SteamUser.EPersonaState.Online)
        cb()
      })
    })
  }

  exec (cmd, context) {
    this._parser.parse(cmd, context, (err, argv, output) => {
      if (err && !output) throw err
      if (output) {
        this._client.chatMessage(context.chat, output)
      }
    })
  }
}

// -- Exports ------------------------------------------------------------------

module.exports = SteamBot

Steps to produce:

Scriptdaemon: !ch help // First call
CheevoBot:  !ch <cmd> // Description doesn't appear?

Commands:
  dice [roll]  Simulate a dice roll
  uptime       Retrieve bot uptime

Options:
  --help     Show help                                                 [boolean]
  --version  Show version number                                       [boolean]
Scriptdaemon: !ch help // Second call
CheevoBot is now Offline. // Crash

[bcoe]

@scriptdaemon hoping to spend a few hours hacking on yargs tomorrow, sorry that it's taken me a while to get to this.

This gives a pretty good starting point, have you made any more headway -- I'd love to work on getting a minimal failing test around this behavior.

[scriptdaemon]

I'm wondering if it might have something to do with the global() method that gets applied to help() and version() (as that's the first thing I notice that's different from any of the other subcommands I add). I should have some time this weekend to write a test that applies global() directly for a given subcommand and see if I still get the crash.

[bcoe]

@scriptdaemon, we're getting pretty close to landing the 7.x branch of yargs -- I think it would be good to test against this, everything defaults to global now; the distinction between global and local was confusing folks.

[scriptdaemon]

I don't have a lot of time at the moment to look more into this, but a quick test crashes the bot (without help() or version()) with this error:

/mnt/c/Users/kwilliams/Documents/Projects/cheevobot/node_modules/yargs/lib/argsert.js:22
    throw Error('Not enough arguments provided. Expected ' + parsed.demanded.length +
    ^

Error: Not enough arguments provided. Expected 1 but received 0.
    at module.exports (/mnt/c/Users/kwilliams/Documents/Projects/cheevobot/node_modules/yargs/lib/argsert.js:22:11)
    at Object.Yargs.self.wrap (/mnt/c/Users/kwilliams/Documents/Projects/cheevobot/node_modules/yargs/yargs.js:682:5)
    at Object.builder (/mnt/c/Users/kwilliams/Documents/Projects/cheevobot/lib/steam-bot.js:63:17)
    at Object.self.runCommand (/mnt/c/Users/kwilliams/Documents/Projects/cheevobot/node_modules/yargs/lib/command.js:149:35)
    at Object.Yargs.self._parseArgs (/mnt/c/Users/kwilliams/Documents/Projects/cheevobot/node_modules/yargs/yargs.js:983:28)
    at Object.Yargs.self.parse (/mnt/c/Users/kwilliams/Documents/Projects/cheevobot/node_modules/yargs/yargs.js:527:23)
    at SteamBot.exec (/mnt/c/Users/kwilliams/Documents/Projects/cheevobot/lib/steam-bot.js:115:18)
    at SteamUser.SteamBot._client.on (/mnt/c/Users/kwilliams/Documents/Projects/cheevobot/lib/steam-bot.js:40:12)
    at emitThree (events.js:116:13)
    at SteamUser.emit (events.js:195:7)

Code: latest from https://github.com/scriptdaemon/cheevobot

Commands issued: !ch uptime, !ch dice, !ch dice 3d6 (each of these leads to the same error)

The code works using yargs 6.6.0.

[bcoe]

We're starting to enforce argument patterns in the API, it looks like we're too aggressive for wrap(): https://github.com/yargs/yargs/pull/767/files#diff-6dd17733300c85ca1a41786abcb4a1c9R682 Do you think we should explicitly require a null? Or treat empt as no wrap? I can see an argument both ways -- I lean towards allowing undefined.

…

On Saturday, February 18, 2017, Kenneth Williams ***@***.***> wrote: I don't have a lot of time at the moment to look more into this, but a quick test crashes the bot (without help() or version()) with this error: /mnt/c/Users/kwilliams/Documents/Projects/cheevobot/node_modules/yargs/lib/argsert.js:22 throw Error('Not enough arguments provided. Expected ' + parsed.demanded.length + ^ Error: Not enough arguments provided. Expected 1 but received 0. at module.exports (/mnt/c/Users/kwilliams/Documents/Projects/cheevobot/node_modules/yargs/lib/argsert.js:22:11) at Object.Yargs.self.wrap (/mnt/c/Users/kwilliams/Documents/Projects/cheevobot/node_modules/yargs/yargs.js:682:5) at Object.builder (/mnt/c/Users/kwilliams/Documents/Projects/cheevobot/lib/steam-bot.js:63:17) at Object.self.runCommand (/mnt/c/Users/kwilliams/Documents/Projects/cheevobot/node_modules/yargs/lib/command.js:149:35) at Object.Yargs.self._parseArgs (/mnt/c/Users/kwilliams/Documents/Projects/cheevobot/node_modules/yargs/yargs.js:983:28) at Object.Yargs.self.parse (/mnt/c/Users/kwilliams/Documents/Projects/cheevobot/node_modules/yargs/yargs.js:527:23) at SteamBot.exec (/mnt/c/Users/kwilliams/Documents/Projects/cheevobot/lib/steam-bot.js:115:18) at SteamUser.SteamBot._client.on (/mnt/c/Users/kwilliams/Documents/Projects/cheevobot/lib/steam-bot.js:40:12) at emitThree (events.js:116:13) at SteamUser.emit (events.js:195:7) Code: latest from https://github.com/scriptdaemon/cheevobot Commands issued: !ch uptime, !ch dice, !ch dice 3d6 — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#776 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAL4MYXJxr2pGe5nZGeQPetxKHpycVuyks5rd2_ngaJpZM4L3cmR> .

[scriptdaemon]

Oh, I see. I don't have a preference either way, as long as it's documented. I'll test again tomorrow with a value specified.

[scriptdaemon]

@bcoe Sorry "tomorrow" kinda became "the day after tomorrow."

Strange, but it appears I was wrong. It works fine (without help()) using npm install git://github.com/yargs/yargs.git#7.x, but crashes with the same error when adding help(). Also, the error only appears to happen with help(), as version() works just fine (which makes sense if global() isn't the issue after all). I thought I had already tested both individually, but apparently not.

Probably unrelated (so I can open another issue if needed): strict() appears to break in 7.x. I commented it out to test help() itself, but when using it I get the "Unknown arguments" error when using any combination of command string. I also installed from the previous commit before 57a8490 and still get the error (though the command executes as well as gives me the error, the latest commit in that branch which I linked only returns the error, which I assume was part of the idea).

[bcoe]

@scriptdaemon I managed to reproduce the issue with help():

#790

Perhaps you could take that branch for a spin.

As for the issue with strict() I believe this to be things working as expected:

<foo> can be used to capture positional arguments for commands, but inner commands themselves should not be specified using <> I updated your example to the following code, while working on a reproduction:

'use strict'

// Node packaged modules
const yargs = require('./yargs')
const parser = yargs()
parser.command({
  command: '!ch', // config.get('cmd'),
  description: 'test desc', // config.get('description'),
  builder: yargs => {
    yargs.command('dice', '', () => {}, () => {
      console.log('dice')
    })
    yargs.command('uptime', '', () => {}, () => {
      console.log('uptime')
    })
    yargs.help()
    yargs.strict()
    yargs.version()
  }
})

parser.parse('!ch uptime', {}, (err, argv, output) => {
  if (err && !output) throw err
  if (output) {
    console.log(output)
  }
})

parser.parse('!ch dice', {}, (err, argv, output) => {
  if (err && !output) throw err
  if (output) {
    console.log(output)
  }
})

works like a charm on the branch I have open.

[scriptdaemon]

@bcoe That fixes the help() issue, but in my code I had actually been using '!ch' (sorry I neglected to mention that). It seems the issue is with context variables.

Updating your code like so:

'use strict'

// Node packaged modules
const yargs = require('yargs')
const parser = yargs()
parser.command({
  command: '!ch', // config.get('cmd'),
  description: 'test desc', // config.get('description'),
  builder: yargs => {
    yargs.command('dice', '', () => {}, () => {
      console.log('dice')
    })
    yargs.command('uptime', '', () => {}, () => {
      console.log('uptime')
    })
    yargs.help()
    yargs.strict()
    yargs.version()
  }
})

parser.parse('!ch uptime', { 'test': 'test' }, (err, argv, output) => {
  if (err && !output) throw err
  if (output) {
    console.log(output)
  }
})

parser.parse('!ch dice', { 'test': 'test' }, (err, argv, output) => {
  if (err && !output) throw err
  if (output) {
    console.log(output)
  }
})

Gives me:

kwilliams@KENLAPTOP:/mnt/c/Users/kwilliams/Documents/Projects/cheevobot$ node .
 !ch uptime

Options:
  --help     Show help                                                 [boolean]
  --version  Show version number                                       [boolean]

Unknown argument: test
 !ch dice

Options:
  --help     Show help                                                 [boolean]
  --version  Show version number                                       [boolean]

Unknown argument: test

[bcoe]

@scriptdaemon ah, that makes sense:

strict mode requires that all options observed are known about ... this would work:

yargs.option('test', {describe: 'some argument passed in as part of context'})

It seems like to simplify this use-case though, we should add all keys provided in context to the list of known arguments?

[bcoe]

@scriptdaemon mind opening another issue for this, and I'll fix this in a separate branch.

[scriptdaemon]

@bcoe Yeah, I figured we were getting off-track. Done: #791.

[bcoe]

@scriptdaemon I think we've addressed the issues you were bumping into in this thread? let me know if there's still anything giving you trouble, and we can open a new issue.