Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

need way to send errors back from subcommand #672

Closed
garretwilson opened this issue Apr 20, 2019 · 3 comments
Closed

need way to send errors back from subcommand #672

garretwilson opened this issue Apr 20, 2019 · 3 comments
Milestone

Comments

@garretwilson
Copy link

A CLI is supposed to return some exit code using e.g. System.exit(2) if there is an error. But this should be called at the top level, and not several levels deep.

My application is calling the following to run the Runnable application:

CommandLine.run(this, getArgs());

My application has a method-based subcommand—let's call it bar. If there is an error (e.g. let's say that a command-line switch was invalid), how does the bar() method return an error so that the main application can call System.exit() appropriately?

On first glance it might appear that I should switch from using Runnable to Callable. But this irrelevant for method subcommands, which are processed differently. I can't throw an exception, because Picocli hijacks the handling of any errors inside execute(CommandLine parsed, List<Object> executionResult).

It appears that Picocli is collecting any return value from the subcommand methods into executionResult. But how do I get those values back?

(Personally I almost wish I could just throw my own exception from the subcommands and somehow bypass Picocli and let my higher-level application handle them.)

@garretwilson
Copy link
Author

So Picocli seems to have some exit code system, but I'm not understanding exactly how it works:

@Command class ExitCodeDemo implements Runnable {
    public void run() { throw new ParameterException(new CommandLine(this), "exit code demo"); }

    public static void main(String... args) {
        CommandLine cmd = new CommandLine(new ExitCodeDemo());
        cmd.parseWithHandlers(
                new RunLast().andExit(123),
                CommandLine.defaultExceptionHandler().andExit(456),
                args);
    }
}
  • It's not clear to me how a subcommand can send back a specific exit code or error condition based upon the problem.
  • I'm not sure how the outside caller can do cleanup when there is an error. It appears that Picocli is calling System.exit() itself.

@remkop
Copy link
Owner

remkop commented Apr 21, 2019

Yes, I agree that exit code handling needs to be improved. Very timely: I actually was just about to start working on this. Some earlier thinking is spread out over some older tickets (#561, #424, #541), let me pull it all together here.

Goals

  • Provide API that allows the application to call System.exit instead of calling System.exit from inside the library
  • Provide a flexible mechanism for deriving an exit code from an exception
  • Applications (command implementations) should be able to control the exit code without the use of an exception
  • Documentation: generate an "Exit Status" section of the usage help message (and in future in generated man page documentation (Generate MarkDown or AsciiDoc documentation #299, Linux friendly manpage generation #459)), listing the EXIT STATUS values of the command (like man's man page). This would require picocli to know all exit codes and their description.

Current implementation

Picocli currently has IResultHandler::andExit(int) and IExceptionHandler::andExit(int) methods for error code handling. These are insufficient: they call System.exit directly and are not flexible enough.

The existing andExit methods will be deprecated in picocli 4.0.

Ideas for Solution

Borrowing from Spring Boot's exit code handling:

IExitCodeGenerator

Let picocli provide an interface IExitCodeGenerator (similar to this) with a single no-arg method returning an int. This solves the problem of deeply nested methods needing to return an exit code: there is no need to propagate this value through the call stack any more.

  • Command classes implementing Runnable or Callable can implement this interface to provide the exit code.
  • For command methods, the enclosing class can implement this interface.
  • In Spring Boot, applications may also pass in zero or more IExitCodeGenerator objects to the SpringApplication entry point. Not sure if this is desirable or useful in picocli.
  • In the case of multiple exit codes the highest value will be used (or if all values are negative, the lowest value will be used)

IExitCodeExceptionMapper

Let picocli provide an interface IExitCodeExceptionMapper (similar to this) that maps Throwables to an exit code. Implementors can inspect details of the Exception object before deciding what exit code to return.

TBD: If the exception implements the IExitCodeGenerator interface directly, should this supercede any mapping? Pro: allows generating a specific exit code even if this Exception is missing in the mapping. Con: less flexible.

Auto-documenting?

It would be nice if the usage help message could automatically include an "Exit Status" section that lists the exit codes and their description. (This ticket explores some possibilities.) I'm beginning to think this is unrealistic. An alternative is to add API to let application authors provide a Map<String, String> that lists exit codes and their descriptions. If this map is provided, picocli will add an "Exit Status" section to the usage help.

Entry point

Add a new execute method to CommandLine that invokes the Runnable, Callable or Method and returns the exit code. This makes the application responsible for calling System.exit:

public static void main(String[] args) {
    CommandLine cmd = new CommandLine(new App());

    // map exceptions to exit codes
    cmd.setExitCodeExceptionMapper(new MyMapper());

    // add usage help "Exit Status" section 
    cmd.setExitStatusHelp(createMap("0:All normal", "1:Invalid input", "2:Disk full", "3:No network"));

    // parse, invoke the business logic, handle errors and requests for help/version
    int exitCode = cmd.execute(args);
    System.exit(exitCode);
}

The execute is non-static, which is important to allow configuration of the parser etc. (This solves a problem (#561) with the run and call convenience methods.)

Error Handling

Split up the current IExceptionHandler2 interface into separate interfaces: IParameterExceptionHandler for handling invalid input and IExecutionExceptionHandler for handling runtime exceptions. All methods on the new interfaces return an int.

Execution Flow

The execute can be implemented roughly as follows:

  1. parse the arguments, resulting in either a ParseResult (if valid input) or a ParameterException being thrown (if input invalid)
    1. (invalid input) catch any ParameterException and pass it to the (new) interface IParameterExceptionHandler which will return an int. Processing ends. TBD how the configured IExitCodeExceptionMapper will be passed to this handler.
    2. (valid input) execute the command by passing the ParseResult to the configured IParseResultHandler2 (RunLast by default). This may return normally or result in an ExecutionException being thrown.
      1. (runtime error) catch any ExecutionException and pass it to the (new) interface IExecutionExceptionHandler which will return an int or throw an exception. Processing ends. (There is discussion on improving error handling to allow rethrowing the original exception and more).
      2. (no error) if the user object implements IExitCodeGenerator, then return its exit code. Otherwise return zero. Processing ends.

@remkop remkop added this to the 4.0-alpha-3 milestone Apr 24, 2019
remkop added a commit that referenced this issue Apr 24, 2019
…te` and `tryExecute` methods: configurable convenience methods with improved exit code support.

* The new `execute` and `tryExecute` methods are similar to the `run`, `call` and `invoke` methods, but are not static, so they allow parser configuration.
* In addition, these methods, in combination with the new `IExitCodeGenerator` and `IExitCodeExceptionMapper` interfaces, offer clean exit code support.
* Finally, the `tryExecute` method rethrows any exception thrown from the Runnable, Callable or Method, while `execute` is guaranteed to never throw an exception.
* Many variants of the previous `run`, `call` and `invoke` convenience methods are now deprecated in favor of the new `execute` methods.
* Many methods on `AbstractHandler` are now deprecated.

Still TODO: tests and documentation.
remkop added a commit that referenced this issue Apr 26, 2019
remkop added a commit that referenced this issue Apr 26, 2019
remkop added a commit that referenced this issue Apr 27, 2019
remkop added a commit that referenced this issue Apr 30, 2019
remkop added a commit that referenced this issue May 1, 2019
…invoke() methods and associated classes and interfaces; update Javadoc
@remkop remkop closed this as completed in 1161b05 May 2, 2019
remkop added a commit that referenced this issue May 3, 2019
…er interface to simplify custom implementations; unwrap the `ExecutionException` in the `CommandLine.execute` method
@remkop
Copy link
Owner

remkop commented May 14, 2019

@garretwilson picocli-4.0-alpha-3 has been released with improved exit code support and exception handling.

This is the last alpha! Please take a look and provide feedback if you have a chance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants