devnote-dev
Semantics of `post_run` hooks
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.
https://github.com/devnote-dev/cling/blob/0dcfb17786eeeb129e095d87d7ea0b1a96beb46b/src/cling/executor.cr#L61-L66
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?
Hi, this project seems interesting. You seem to take good care of it!
From what I understand the post_run should run regardless, after the on_error hook just like you said. Otherwise it wouldn't have any real purpose as you could just as well call a command at the end of run.
For existing apps to migrate they could move their old post_run logic to end of run or implement a new hook i guess. Guess not too many affected users.
Don't see any need to enforce stricter exception handling for this change. I guess post_run would be able to access variables set in the previous hooks including on_error? So postrun action can be customized accordingly.
