-
Notifications
You must be signed in to change notification settings - Fork 3
Description
The current flow of a command's execution runs through the pre_run hook -> run main method -> post_run hook. However, if pre_run or run are interrupted or stopped, post_run is never called.
Lines 61 to 66 in 0dcfb17
| begin | |
| resolved_command.run executed.parsed_arguments, executed.parsed_options | |
| resolved_command.post_run executed.parsed_arguments, executed.parsed_options | |
| rescue ex | |
| resolved_command.on_error ex | |
| end |
This means that post_run is effectively limited to pre_run and run working successfully. So in cases where cleanup needs to be performed before an application exits, your only option is to handle it in pre_run, run or the on_error hook method. That leaves 2 possible options for post_run: to call this method after pre_run/run but not directly require them to exit successfully, or to remove it entirely.
Despite my large advocacy for Cling and its structure, I have never used post_run for any existing CLI applications. It was mostly inspired from Cobra's PostRun methods which operates on a similar premise: postRun is deferred and will always run regardless of the state of the PreRun and Run methods. Assuming none of the methods fail, PostRunE/PostRun is called just before execution ends.
Cling does not differentiate between erroneous and non-erroneous execution methods like Cobra does with "E" suffixed methods because exceptions can be raised at any point of the program making everything erroneous. That means post_run could effectively be changed to always be called as the final part of execution, with any exceptions from it or previous hook methods still being funneled to the on_error hook method.
While this sounds like a clear-cut solution, there are some undefined semantics as to what precedence post_run has in regards to exception handling (or rather, ignoring) which up until this point, has been preceded by the on_error hook method. Would this enforce a stricter exception handling structure? What affects would this impose on existing applications? And most importantly, is it worth it?