Replies: 1 comment
-
I agree with this level of discipline in the code. It will help us avoid unwanted and unintended behaviors. |
Beta Was this translation helpful? Give feedback.
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
-
[Original context.]
I think our respective consternation over this blob of code is indicative of a deeper problem...and I think that problem is the catch-all exception handler.
Previously, this code was set up to report any error as a
ConversionError
, but now we would like to implement an (um...) exception to that policy, and it's "hard". Perhaps the better approach is to report only certain errors viaConversionError
and let all other errors (especiallyUnVerifiedUser
) propagate past. I think that that would allow this code to embody its function much more straightforwardly.Generally speaking, I think catch-all handlers should only be used at interface boundaries. That is, at the last point of control before returning to an external caller, we need to ensure that our behavior conforms to our interface definition, and that probably means catching unexpected exceptions. Similarly, at the first point of control around calls to external interfaces, we might want to catch unexpected exceptions, as well, in case the function we're calling fails to conform to its interface definition. However, in between those two points, we shouldn't need catch-all handlers: we should only catch exceptions for which we have a specific reaction, and we should let the rest of them fly. And, there is an argument that the same thing is true at our external interfaces, as well. (Although, in any but the most (um...) exceptional case, a human end-user should never see an exception and traceback....)
So, any time we have a catch-all handler, we should be asking some pointed questions about why that handler is there. Are we just being lazy? Are we mistakenly thinking that we're being efficient by avoiding the explicit list of expected cases? Are we simply unaware of the behaviors of the interface that we're calling? (E.g., do we have a documentation problem?) Is the handler code really correct for all possible exceptions?? What would be the effect (and would it be so terrible) of not catching an exception at that point?
Beta Was this translation helpful? Give feedback.
All reactions