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

Ability to supply an array of valid/accepted exitcodes. #298

Closed
nixtar opened this issue May 18, 2021 · 4 comments
Closed

Ability to supply an array of valid/accepted exitcodes. #298

nixtar opened this issue May 18, 2021 · 4 comments
Labels
enhancement New feature or request wontfix This will not be worked on

Comments

@nixtar
Copy link

nixtar commented May 18, 2021

Use case(s)

Some processes use non zero exitcodes when running successfully.
For EG robocopy: https://ss64.com/nt/robocopy-exit.html and msiexec

Description

Add a int array parameter to Run, RunAsync, Read and ReadAsync that defaults to new int[] { 0 }.
Then change the current logic that checks if the exit code is not 0 to check that its not in the array.
To prevent breaking changes add new ExitCodeException and continue to throw NonZeroExitCodeException if the expected exit code only contains 0?
Or make parameter null by default and do a null check and throw NonZeroExitCodeException if null and exit code is non zero.

Alternatives

Consumer of this library catches NonZeroExitCodeException and implements their own exit code checks.

@nixtar nixtar added the enhancement New feature or request label May 18, 2021
@adamralph
Copy link
Owner

hi @nixtar, thanks for raising this. It's a shame that there are widely used programs which do not conform to the non-zero-for-failure convention, but given that is the case, this feature request seems reasonable.

Just to explore alternatives, what about an optional delegate that allowed you to take control of the handling of exit codes?

Off the top of my head, something like:

Run("foo.exe", handleExitCode: exitCode => { ... });

If the delegate is present, then SimpleExec would bypass it's normal handling and call the delegate instead.

@adamralph
Copy link
Owner

adamralph commented May 18, 2021

Or perhaps the delegate could return a Boolean to indicate that it has handled the exit code, where true means handled and false means unhandled. So, given that robocopy exit codes 0-7 indicate success, your code would look something like:

Run("robocopy", handleExitCode: exitCode => exitCode < 8);

@adamralph
Copy link
Owner

adamralph commented May 18, 2021

Actually the Boolean proposal, like your original proposal, wouldn't make sense as long the exception is named NonZeroExitCodeException. If we pursue either of those proposals, I wouldn't mind obsoleting NonZeroExitCodeException in favour of a new ErrorExitCodeException. The new exception would inherit from the old exception until the next major version bump.

@adamralph
Copy link
Owner

Thanks again for raising this @nixtar. I'm replacing it with the more general solution described in #313.

@adamralph adamralph added wontfix This will not be worked on enhancement New feature or request and removed enhancement New feature or request labels Jun 12, 2021
@adamralph adamralph closed this as not planned Won't fix, can't repro, duplicate, stale Jul 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants