CommandLineException with both exit code and message

[jnm2]

Would it potentially make sense for one or all of the app models to be able to throw a CommandLineException with a message to be printed in red and an exit code to be returned?

This would differ from throwing any other exception in two ways:

• An exit code can be specified.
• An entire stack trace (ex.ToString()) would never be printed like it would for an unhandled exception. Throwing CommandLineException means you want only the message to be printed.

Instead of returning int or Task<int> the user could return void or Task. Instead of returning an int, the user could use throw new CommandLineException("Message to explain the exit code", 42);.

This could halve the number of overloads for things like CommandHandler.Create. It currently has four overloads for each arity; e.g., Action<T>, Func<T, int>, Func<T, Task>, Func<T, Task<int>>.

namespace System.CommandLine
{
    public sealed class CommandLineException : Exception
    {
        public int ExitCode { get; }

        public CommandLineException(string message, int exitCode = 1) : base(message)
        {
            ExitCode = exitCode;
        }
    }
}

[jonsequitur]

There was some discussion of this. For example, validation failures are not "exceptional". They're often responses to expected user input, so we preferred normal control flow. (In this case, we use the middleware pipeline to return early rather than pass invalid results to a command handler.)

The CommandHandler.Create overloads do need to be tamed though. One possibility there is to make them all void- or Task-returning and have all results set via the InvocationContext.

[jnm2]

Whether validation errors are exceptional could be a matter of interpretation. I hear what you're saying and I'm usually inclined to agree. Throwing an exception seems so neat from a pragmatic standpoint though. It is a familiar way to say, "Stop the execution with this exit code and this specialized message," all in a single statement.

I'd prefer it over having side effects on InvocationContext as an app model, for example.

[jonsequitur]

Agreed, I think this may be more intuitive than setting the result on the InvocationContext. InvocationContext is intended to be more of a lower-level API to build more opinionated app models on top of (such as #363 and DragonFruit).

[KathleenDollard]

I'm not sure AppModels are significant here. Isn't the problem the same whether the issue occurs in the app model or the underlying code? If it's only actual AppModels issues (validation, etc), then I think interacting with invocation context is OK, since I don't think app models will be executed in other contexts.

As far as the exit code, I think demanding everything return an integer and having an easy way to wrap tasks that don't normally return integers is fine, since command line stuff returns integers. So, on that side, I think we have other options.

However, the message is an official big deal in my mind. Non-zero exit codes with no other information are terrible and forcing things in the execution chain to know about them is terrible.

I agree with the concerns about exceptions, but this might fall in the category of "And thirdly, the code is more what you'd call 'guidelines' than actual rules."

I haven't spent a lot of time thinking through this, but from an app model perspective, I'd be tempted to capture any exception and return an arbitrary code and message only - probably with an opt in or opt out. The actual application code should not have to worry about any of this while it is running.

These are the options I can think of:

1. Throw an exception
2. Return a tuple of integer/string
3. Return an integer and assume the program has the good sense to send it's error to StdErr
4. Expect code to interact with invocation context

[KathleenDollard]

If we are going to have a special exception to communicate to the core, let's include a "level" for intelligent coloration.

[jnm2]

I didn't have validation in mind so much as unrecoverable errors during operation; e.g., a file is locked and the task cannot be completed, or a server cannot be reached, or a configuration file is missing. In these situations, you want to write a user-friendly explanation and immediately terminate.

If throwing CommandLineException causes the program to exit, wouldn't the colorization level always be "critical error"? If it was a warning, execution should continue. But if the user app threw CommandLineException, the throwing method can't continue.

(Would an app doing manual argument validation want to use ArgumentException and derived types to be potentially more idiomatic than this exception?)

[KathleenDollard]

One more comment on this...

I have tried to fix the redundancy in CommandHandler and have become convinced that is not possible. I hope someone is about to prove me wrong.

And they should definitely extend to the T16.

To limit the insanity in that class, I think we want to limit the return types. We currently have 4. 4 * 16 = 54.

IOW,not adding overloads should be a goal of resolving this issue.

[jonsequitur]

In these situations, you want to write a user-friendly explanation and immediately terminate.

For this, we have a middleware method called UseExceptionHandler which catches exceptions at the top level. It happens to write the exception message out but it could be modified or made configurable so that developers can choose what they want to display, including colorization.

I have tried to fix the redundancy in CommandHandler and have become convinced that is not possible. I hope someone is about to prove me wrong.

Removing the int return code variants from these methods will reduce the number of overloads, and app models likely only need CommandHandler.Create(MethodInfo).

If we consider the example of the ASP.NET MVC "app model" then the allowable return types are mediated by the controller at the app model layer, but the various outputs (HTTP status code, response, headers) are all set on the context object.