How to remove stacktraces on error

[nicolasmingo]

I'm trying to use picocli, but when I have a business exception, it is encapsulated inside ExecutionException. A stracktrace is printed out, but I need to have a cleaner error message (without stacktrace). How to do it ?

[remkop]

Two options I can think of:

Option 1. Try-catch

try {
  CommandLine.run(new MyApp(), args);
} catch (ExecutionException ex) {
  System.err.println("Error: " + ex.getMessage());
}

Option 2. Custom error handler

IExceptionHandler2<Void> handler = new DefaultExceptionHandler<Void>() {
  public Void handleExecutionException(ExecutionException ex,
                                 ParseResult parseResult) {
    System.err.println("Error: " + ex.getMessage());
    return null;
  }
}

CommandLine cmd = new CommandLine(new MyApp());
cmd.parseWithHandlers(new RunLast(),
    handler, args);

[jrevault]

Hi,

We were using the first option :

try {
  CommandLine.run( new MyApp() , args );
} 
catch ( Exception ex ) {
  if (ex.getCause() instanceof SGEx){
    System.err.println( ex.getCause().getMessage() );
  }
  else {
    System.out.println( "An internal error has occured, please contact support." );
    System.err.println( "Error : " + ex.getMessage();
  }
}

We are using our own functional exceptions, that prints end user specific messages, and using instance of upon the exception was quite strange.

[remkop]

Okay, so basically you’re asking, is there a way to avoid our business exception being wrapped in an ExecutionException.

Let me take a look to see what is possible in upcoming 4.0.

[remkop]

One idea is for picocli to provide API like the below. The tryExecute method would act similarly to CommandLine.run or CommandLine.call, so App should implement Runnable or Callable:

CommandLine cmd = new CommandLine(new App());
try {
    cmd.tryExecute(MyException.class, args);
} catch (MyException ex) {
    // handle...
}

If your application potentially throws multiple business exceptions, your code could look like this:

CommandLine cmd = new CommandLine(new App());
try {
    cmd.tryExecute(MyException1.class, MyException2.class, args);
} catch (MyException1 ex) {
    // handle...
} catch (MyException2 ex) {
    // handle...
} catch (ExecutionException other) {
    // handle...
}

Thoughts?

[nicolasmingo]

Disable stacktraces.

1. We can do this today by surrounding the execute() call with a try-catch
2. We can have a new setter provided by picocli to enable/disable this

CommandLine cmd = new CommandLine(new App());
cmd.showStackTracesOnEnabledOption("--stacktrace"); // maybe it can be enabled by default, but we can disable it

Keep original messages

• Get a message as the original business exception's message (without "Error while calling command (..."

// Let wrapping the original exception inside ExecutionException but keeps the original exception message
CommandLine cmd = new CommandLine(new App());
cmd.keepOriginalExceptionMessages(true); // maybe another name could be better
try {
    cmd.execute(args);
} catch (ExecutionException e) {
    // handle...
}

Let throwing original exceptions

CommandLine cmd = new CommandLine(new App());
cmd.keepOriginalException(true); // maybe another name could be better
try {
    cmd.execute(args);
} catch (ExecutionException e) {
    // handle...
} catch (MyException me) {
    // handle...
}

https://github.com/remkop/picocli/blob/master/src/main/java/picocli/CommandLine.java#L1201

[remkop]

I'm thinking to have two methods:

// CommandLine

/**
 * Executes the Runnable, Callable or Method that the CommandLine was constructed with,
 * and returns an exit code that the application can use to call `System.exit` with.
 *
 * This method never throws an exception.
 * 
 * Any exception that occurs during parsing or while executing the business logic
 * is mapped to an exit code.
 */
public int execute(String... args) { ... }

/**
 * Executes the Runnable, Callable or Method that the CommandLine was constructed with,
 * and returns an exit code that the application can use to call `System.exit` with.
 *
 * Any `ParameterException` that occurs while parsing the user input is mapped to an exit code.
 * This can be customized with a custom `IParameterExceptionHandler`.
 *
 * Any `Exception` that occurs while executing the Runnable, Callable or Method is rethrown.
 * (The actual business exception is rethrown, not the picocli wrapper `ExecutionException`.)
 * This can be customized with a custom `IExecutionExceptionHandler`. 
 *
 * @throws by default, throws any exception that occurs while executing 
 *         the Runnable, Callable or Method that the CommandLine was constructed with
 */
public int tryExecute(String... args) throws Exception { ... }

Thoughts?

[remkop]

An initial implementation just landed in master. Please check it out if you have a chance.

[remkop]

I put a working prototype of the above API in master, but I am not entirely happy with it. The prototype has both a tryExecute method and an IExecutionExceptionHandler to handle business exceptions.

It seems simpler to have just one execute method. The contract of the execute method is that it will never throw an exception, and always return an exit code.

If a business exception occurs, the default behaviour will be to print the stack trace of that business exception (not the ExecutionException) and return an exit code. I believe this is reasonable default behaviour.

In your case, your application is using our own functional exceptions, that prints end user specific messages, so you will want to do something different.

To customize, applications can configure an IExecutionExceptionHandler. From your example above, this will look something like this:

IExecutionExceptionHandler handler = new IExecutionExceptionHandler() {
  public int handleExecutionException(ExecutionException execEx, ParseResult parseResult) {
    try {
        execEx.rethrowCauseIf(Throwable.class); // rethrow the wrapped exception
        throw execEx; // in case the ExecutionException had no cause Exception
    } catch (SGEx sgex) {
        System.err.println(sgex.getMessage());
    } catch (Exception exception) {
        System.err.println("An internal error has occurred, please contact support.");
        System.err.println("Error : " + exception.getMessage());
    }
    return 9876;
  }
}

CommandLine cmd = new CommandLine(new MyApp());
cmd.setExecutionExceptionHandler(handler); // install the custom handler
int exitCode = cmd.execute(args);
System.exit(exitCode);

[remkop]

Reopening since I’m not happy with the solution I proposed above.
I will revisit this tomorrow.

[remkop]

@nicolasmingo @jrevault I pushed a change to master:

The IExecutionExceptionHandler interface now gets the actual exception thrown by the business logic, so it is no longer wrapped in an ExecutionException. (The ExecutionException is only passed to the IExecutionExceptionHandler if the business logic threw a Throwable or Error instead of an Exception.)

Example:

IExecutionExceptionHandler errorHandler = new IExecutionExceptionHandler() {
    public int handleExecutionException(Exception ex, 
                                        CommandLine commandLine, 
                                        ParseResult parseResult) {
        //ex.printStackTrace(); // no stack trace
        commandLine.getErr().println(ex.getMessage());
        commandLine.usage(commandLine.getErr());
        return commandLine.getCommandSpec().exitCodeOnExecutionException();
    }
};
int exitCode = new CommandLine(new App())
        .setExecutionExceptionHandler(errorHandler)
        .execute(args);

This looks quite a bit cleaner to me than the previous attempt.

Thoughts?

Note that we have a unique window of opportunity during the 4.0-alpha phase to flesh out API design issues. It will be much harder to change the API after the 4.0 GA release. Please give the above a try and provide feedback.

[remkop]

Closing this ticket. I'm planning to release 4.0-alpha-3 in the next few days. Feedback welcome! :-)

[remkop]

@nicolasmingo, @jrevault 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.