Tyrrrz / CliFx

Class-first framework for building command-line interfaces
MIT License
1.5k stars 61 forks source link

Add the ability to set exit code without displaying StackTrace details #66

Closed ron-myers closed 4 years ago

ron-myers commented 4 years ago

So far working with CliFx has been excellent. I see this framework solving several of the issues I have with other options in the dotnet space today. Thank you @Tyrrrz

One edge I hit is the app that I am working on needs to set the exit code based on some application logic.

Within CliFx, it seems this can only be set by throwing an exception. I understand that as a design approach, but my need is to set the failure to a known error code without displaying a stack trace. This is an expected exception.

What I tried:

      if (WarningsAreErrors && result.Findings.Any())
      {
        // set the exit code
        throw new CommandException(string.Empty, exitCode: (int)AppExitCode.ErrorsFound);
      }

but the stack trace is still displayed:

CliFx.Exceptions.CommandException
   at UnderTest.FeatureLint.Commands.DefaultCommand.ThrowExitCodeIfError(FeatureLintResult result) in C:\dev\oss\under-test\undertest.featurelint\src\UnderTest.FeatureLint\Commands\DefaultCommand.cs:line 78
   at UnderTest.FeatureLint.Commands.DefaultCommand.ExecuteAsync(IConsole console) in C:\dev\oss\under-test\undertest.featurelint\src\UnderTest.FeatureLint\Commands\DefaultCommand.cs:line 60
   at CliFx.CliApplication.RunAsync(IReadOnlyList`1 commandLineArguments, IReadOnlyDictionary`2 environmentVariables)

So I am proposing a specific exception for this, say CommandExpectedException(open to feedback on the name)

This should just need the exception and an additional catch statement in CliApplication.RunAsync and I ready to create a PR if this is seen as a welcome change.

Tyrrrz commented 4 years ago

Note that the stack trace is only shown if the exception contains no message. Wouldn't it make sense to provide an error message along with the exit code in your case? Something like "There were X errors in Y".

ron-myers commented 4 years ago

In my case, no. There is a flag to have the output the result in JSON. So any other messages break the functionality of my app.

Are you open the idea of the exception type to handle this type of situation? This is the last blocker to me moving my app to CliFx.

Tyrrrz commented 4 years ago

There is a flag to have the output the result in JSON

But the error message is printed to StdErr, not StdOut.

ron-myers commented 4 years ago

Update: I am restructuring some components of my app to see if we can make this work.

The edge I have hit is that some clients are using the full output (StdErr + StdOut) to collect the result - if I can work with them to change that then this may work but if not this will be a blocker for our adoption.

Tyrrrz commented 4 years ago

That's okay. The design rationale behind it is that while exit code is visible to other programs, it's not shown in terminal unless you specifically check it. So every error should have at least some text supporting it to inform user what went wrong.