Generate errors through Commander from client code

[hildjj]

As mentioned on the Implied Options bug, I would like to perform inter-option checking after the main checks are done in parse. In these extra checks, I would like to throw InvalidArgumentError, and have the same behavior as if I had done so in a coercion routine. In particular, I would like the exception to be caught, and depending on what I've done with exitOverride and configureOutput, either re-raised or printed appropriately and process.exit() called.

I'm open to lots of different designs. Here's a starting point for discussion:

const argv = new Command()
  .option("-a", "First things first")
  .option("-b", "Second class")
  .check(opts => {
    if (opts.a && opts.b) {
      throw new InvalidArgumentError("-a and -b are mutually exclusive");
    }
    return { foo: opts.a || opts.b }; // Optional: replaces opts if truthy value returned
  })
  .parse()
  .opts();

The check routine gets called right before parse returns, perhaps inside the existing catch block for options. If check returns undefined, the existing opts object (which might have been modified by the check method) is returned by opts(), otherwise whatever check returns will be used for opts().

[shadowspawn]

Interesting question. Before Commander v4 there was not much extra that Commander did, but now as you noted there is exitOverride and configureOutput. And showHelpAfterError. And in the future...

The InvalidArgumentError exception is a special case though. Commander uses the context of the option coercion routine to know the option name and includes that in the error message for you. That particular handling can't be done when the exception is thrown from other parts of the code.

• commander.js/lib/command.js

  Line 571 in 43f4743

  this.on('option:' + oname, (val) => {

The life-cycle hook for "preAction" is called after parse and before an action handler, so is a potential place for putting code for checking options. (i.e. rather than adding a new .check() method.)

(I will think about convenient calling signatures to make use of the Commander built-in behaviours. Haven't got a discussion suggestion for that yet.)

[shadowspawn]

Related prior art:

• http://yargs.js.org/docs/#api-reference-checkfn-globaltrue
• http://yargs.js.org/docs/#api-reference-middlewarecallbacks-applybeforevalidation
• https://docs.python.org/dev/library/argparse.html#argparse.ArgumentParser.error
• https://oclif.io/docs/commands#thiserrormessage-string--error-options-code-string-exit-number-ref-string-suggestions-string

[shadowspawn]

Experimenting with simple .error("message"). In style of console.error(), and argparse .error(), and oclif .error().

program.error('-a and -b are mutually exclusive');

A potential downside with simple "error" is the name may clash with option name used in legacy programs still using . storeOptionsAsProperties(true).

[hildjj]

Can there be an optional code parameter? Would this need to be called from action, or would it just exit/throw depending on exitOverride?

[shadowspawn]

Can there be an optional code parameter?

Yes. Is that something you would use straight away? For unit tests, or for try/catch in calling code?

Would this need to be called from action, or would it just exit/throw depending on exitOverride?

I was thinking just exit/throw depending on exitOverride.

[hildjj]

Yes, I'm using the error code in unit tests. I call exitOverride and configureOutput (passing in a function that writes to a small stream.Transform class to simulates stdio). I check the error code is the one I expect, and that some chunk of the text is what I expect.

Duplicating or renaming the current behavior of _displayError would work very well for me, which means also having an optional exitCode, and perhaps more direct control over whether the help gets output.

[shadowspawn]

Experiment: shadowspawn@c223fae

[hildjj]

I like your experiment. One possible improvement would be to give code and exitCode default values. (Sorry, I didn't read down ONE MORE LINE to see you set the defaults.)

[hildjj]

Did this make it into v9.0.0?

[shadowspawn]

No, or at least not yet. This issue has not generated any likes or comments from other people, and I was not working on it.

I'll take another look.

[hildjj]

Thanks! I'm doing ok using the undocumented internal function, and I think I've got a test that will catch when it changes so I can fix it up later. It would of course be better if I didn't have to release like that. :)

[shadowspawn]

Opened a draft PR: #1675

[shadowspawn]

.error() is included in prerelease Commander v9.0.0-1.

[shadowspawn]

Commander v9 has been released.