parse vs parseAndExit

[binarymist]

Hi @nexdrew

What's the correct "recommended" way to establish whether to parse or parseAndExit?

I have the following code (cli.js) which feels a bit dirty and is also missing a few cases... which I think
sywac should be dealing with. For example, when a user runs the CLI with test <unknownOption> parseAndExit executes my test command and I'm expecting the command to handle and spit out the specific command (test in my case) help along with a contextual help error message, which doesn't happen.

Also when a user runs the CLI with an (any) unknown command (for example testttt), just the generic help is displayed, I think sywac can and usually does also produce a contextual error message which is not currently the case. Am I doing something incorrectly here?

cli.js

const processCommands = async (options) => { // eslint-disable-line no-unused-vars
  log.debug('Configuring sywac\n', { tags: ['cli'] });
  const api = await sywac // eslint-disable-line no-unused-vars
    .usage('Usage: $0 [command] [option(s)]')
    .commandDirectory('cmds')
    // This overrides the --help and --version and adds their aliases
    .showHelpByDefault()
    .version('-v, --version', { desc: 'Show version number' })
    .help('-h, --help')
    .preface(figlet.textSync(pkg.name, 'Chunky'), chalk.bgHex('#9961ed')(pkg.description))
    .epilogue('For more informatiion, find the manual at https://docs.purpleteam-labs.com')
    .style({
      // usagePrefix: str => chalk.hex('#9961ed').bold(str),
      flags: str => chalk.bold(str),
      group: str => chalk.hex('#9961ed').bold(str),
      messages: str => chalk.keyword('orange').bold(str)
    });

  const shouldParseAndexit = (argv) => {
    const command = argv[2];
    const arg = argv[3];
    return argv.length < 3
      || command === 'about'
      || command === '-v' || command === '--version'
      || command === '-h' || command === '--help'
      || (command !== 'test' && command !== 'testplan')
      || (command === 'test' && !!arg) || (command === 'testplan' && !!arg);
  };

  const cliArgs = shouldParseAndexit(options.argv) ? await api.parseAndExit() : await api.parse();

  if (cliArgs.errors) log.error(cliArgs.errors);
};

The test command (test.js)

const config = require('config/config'); // eslint-disable-line no-unused-vars
const api = require('src/presenter/apiDecoratingAdapter');


exports.flags = 'test';
exports.description = 'Launch purpleteam to attack your specified target';
exports.setup = (sywac) => {
  sywac
    .option('-c, --config-file <config-file path>', {
      type: 'file',
      desc: 'Build user supplied configuration file. Must be a file conforming to the schema defined in the purpleteam documentation.',
      mustExist: true,
      defaultValue: config.get('buildUserConfig.fileUri')
    });
};
exports.run = async (parsedArgv, context) => {
  if (parsedArgv.c) {
    const configFileContents = await api.getBuildUserConfigFile(parsedArgv.c);

    await api.test(configFileContents);

    //  stream tester log           Print each tester to a table row, and to log file
    //  stream slave log            To artifacts dir
  } else {
    context.cliMessage('You must provide a valid build user configuration file that exists on the local file system.');
  }
};

Thanks.

[nexdrew]

I am AFK right now, but concerning when to use parse vs parseAndExit, they are not designed to be interchangeable, your CLI should use one or the other.

parseAndExit is a special case only meant for "simple" use-cases where you want your CLI to exit if validation fails or if a command reports an error via cliMessage. parseAndExit resolves to the parsed argv object, which is just one property of the entire parsing result.

parse is meant for all other cases, resolving to the entire parsing result, and it never exits the app - any errors that occur are included in the result, your app would have to handle them (decide what to do) on its own.

[binarymist]

I am AFK right now, but concerning when to use parse vs parseAndExit, they are not designed to be interchangeable, your CLI should use one or the other.

OK

Most of the time parse does what I need, and parseAndExit does not.

but: parse doesn't print about, version, help (generic, command specific (incl specific messages)).

So: If I use only parse, the end user has no idea how to use the Cli as there is no help, about, version, etc. If I use only parseAndExit The commands that run, won't.

So you can probably see the predicament. If I use one or the other, which would you suggest, and how would you suggest I work around the issues that come with either of the options?

Thanks.

[nexdrew]

There's no magic to parseAndExit - this is what it does:

sywac/api.js

Lines 574 to 583 in 7e7eff1

	parseAndExit (args) {
	return this.parse(args).then(result => {
	if (result.output) {
	console.log(result.output)
	process.exit(result.code)
	}
	if (result.code !== 0) process.exit(result.code)
	return result.argv
	})
	}

You could do something similar. If help was requested, or if there is an error to report, the content to display will be given in result.output as a string. The reason parse doesn't print it to the console for you is that it makes no assumptions about which stream or medium to use - for instance, if you were using sywac to build a chatbot, printing to the console (stdout) would not be relevant.

[binarymist]

Understood, but this doesn't seem to be working. As I mentioned above, when I run test <unknownOption> ([known command] ) result.output is empty string. The test command has everything it needs to run, but I provided an additional piece of text that is meaningless, I'd expect a help message that informs the user that the <unknownOption> is invalid.

Do I need to add something extra to the test command setup, or am I missing something else?

Thanks.

[nexdrew]

What you describe is known as "strict" mode in yargs.

Sywac does not support "strict" mode out-of-the-box; rather, any unknown flags are parsed on a best-effort basis and added as keys in argv, and any unknown commands/positionals are added as values to the argv._ array.

If you want to use "strict" mode with sywac, you'll need to write some custom validation code. I recommend using a .check() handler.

[binarymist]

I replaced my shouldParseAndExit function with the .check() function call and the passed handler doesn't get called. the .check() function is called, but the handler is not.
I just tested by running the programme with a random string as it's first arg/command.
What causes the handler to be called?

I see there is a check for shouldCoerceAndCheck, but I'm not aware of that option, do I need to know about that, if so, can you tell me about it?

I also see on the .check docs you provided a link for, that it says

Define any custom validation logic to run after all types have been parsed, validated, and coerced. The handler will not be called if help or version are explicitly requested.

I noticed that when I used parseAndExit instead of my default of parse that help was displayed, obviously help or version wasn't explicitly requested, but I guess it was implicitly requested, is this the problem?

Thanks.

[nexdrew]

How about something like this? https://gitlab.com/nexdrew/purpleteam/compare/master...fix-cli

If you pass unknown args to the test or testplan command, it will fail-fast and display an error message.

If you pass unknown flags, they are simply ignored and the command is run.

If you pass an unknown command, the top-level help content is displayed (this is due to .showHelpByDefault()).

One thing I noticed is that you are catching API errors in the code that is called by the test command, which means the CLI code won't know that anything went wrong and will exit the process with a normal (0) exit code.

[binarymist]

This seems to work well, except...

One thing I noticed is that you are catching API errors in the code that is called by the test command, which means the CLI code won't know that anything went wrong and will exit the process with a normal (0) exit code.

This is why I was using parse most of the time, so the application (not cli) errors could be handled by the blessed screen in the dashboard view. The reason for this from memory was that when I used parseAndExit any application errors seemed to dissapear, and as you say, the process would exit with 0

I'm also re-throwing any app errors which I would have thought would bubble up for the CLI code to display and exit, but they don't. How should a sywac consumer deal with these?

I also didn't realise that the check function was supposed to be within each commands setup, I had it in the top level under the style function.

Thanks very much for helping so far!

[binarymist]

Thoughts @nexdrew ?

[nexdrew]

Any unhandled errors will bubble up to sywac and sywac will handle them and report them in the result object resolved by the .parse() method.

The error handling I was talking about in your code is found here: https://gitlab.com/purpleteam-labs/purpleteam/blob/49e1341ca37f0b5f81cd239f92b9f86344c51f97/src/presenter/apiDecoratingAdapter.js#L50-77

Since it catches the error from the rejected Promise and doesn't re-throw it or notify sywac, the CLI code doesn't know that any error occurred.

This is what I see:

$ NODE_ENV=test ./bin/purpleteam test -c ./testResources/jobs/job_0.1.0-alpha.1
▶  debug      [index] Starting the CLI
▶  debug      [cli] Configuring sywac

✖  critical   [apiDecoratingAdapter] Error occurred while attempting to communicate with the purpleteam SaaS. Error was: "The purpleteam backend is currently unreachable".
$ echo $?
0

How you want to handle this is entirely up to you. Perhaps the easiest thing might be to set process.exitCode wherever you are handling errors this way.

let response
try {
  response = await something()
} catch (err) {
  handleError(err)
  process.exitCode = 1
}

[nexdrew]

I'm closing this issue as I believe I have fully answered the question about when to use parseAndExit vs when to use parse. All the other details are specific to your implementation and do not reflect issues with sywac.