Do not exit if commands return non-ExitCoder errors

[jszwedko]

Doing so limits the ability for users to have only some of their errors
cause the application to terminate while allowing others to bubble up.

This was originally an adjustment to #361 in #496 to fix #495, but, in
hindsight, I believe that the better approach is to recommend the use of
RunAndExitOnError for this use case (which is undeprecated here).

Fixes #565 #594

[jszwedko]

@gravis this is a revert of the behavior added in #475 (see the reasoning above) so, after this is merged and you update, you'll want to switch to RunAndExitOnError to preserve the behavior. Let me know if you have any thoughts though!

[gravis]

I think this will make the behaviour of many programs uncertain. I'd recommend to add a new method instead of undeprecating an old one + change the existing Run. This will confuse so many users...

[awsmsrc]

Will this be reaching master soon? Anything I can do to help it a long?

[jszwedko]

@gravis I concede that changing the behavior back will confuse some users, but I think there may be a number of users that were (and will be when they upgrade to the latest tag of cli) confused by the introduction of the behavior in #475 (which, again in hindsight, I would have qualified as a breaking change in the behavior of the library). There isn't a great path forward, but I think putting the behavior back will result in the least overall confusion to users of this library.

[jszwedko]

Pinging @meatballhat to get his thoughts

[meatballhat]

This is a tough one alright. I think that we're in a bit of a bind. Maybe a silly question, but can we leave master as is and target v2 with the reverted/new behavior?

[awsmsrc]

@jszwedko @meatballhat the change in #475 was breaking for us in production, the advantage to reverting is that the current state is a removal of functionality which we require where as reverting the change (though a breaking change again) may cause some users to update there applications but at least both paths are available to the developer

[vincentbernat]

I was also quite confused by the change. The current behaviour is a bug as this is not working as documented. This makes error handling quite inconsistent currently.

[jszwedko]

@vincentbernat thank you for the nudge, after further reflection on the matter, I agree that it was a regression in behavior that will cause more confusion by being left in than by being reverted. I was trying to reconcile with master and appeared to have closed this PR, but will get it into master now.