I have inherited a codebase which contains code such as the following (note: example code is PHP):
try {
// Do something which doesn't intentionally throw exceptions.
} catch (\Exception $e) {
$this->log->log($e->getMessage());
$this->product->setError($e->getMessage());
return false;
}
So essentially, the code is catching the exception. Logging it, and the failing silently (apart from the log message).
This behaviour seems to make sense in production, but makes development much more difficult (as stack traces have to be looked up in log files, rather than being printed to the console). So I have come up with the following function:
private function tryCatch ($func) {
// Bind closure, so that $this allows it to access class properties
if (is_object($func) && ($func instanceof Closure)) {
\Closure::bind($func, $this, "static");
}
if (\App::environment('test')) {
return $func();
} else {
try {
return $func();
} catch (\Exception $e) {
$this->log->log($e->getMessage());
$this->product->setError($e->getMessage());
return false;
}
}
}
which can then be used like so:
$this->tryCatch(function () {
// Do something
});
This code special-cases the 'test' environment which it calls the passed in function without exception handling (so any exceptions remain unhandled). In every other environment (e.g. production), it wraps the passed in closure in a try-catch block in production behaves as the code originally behaved.
This solutions solves my problems, but it seems a bit hacky, and leaves me with aI have nagging feeling that it isn't a good idea.
My question: Is there any reason why I shouldn't do this? Or is there a better way of handling this situation?
Don't try to reinvent the wheel with regards to exceptions. There is one and only one scenario in which you should catch an exception:
Catch an exception if you have an alternative plan what to do with it.
An exception means that your code encountered an exceptional condition in which it cannot continue its work and has no choice but to throw in the towel. That's a perfectly good way to abandon a function/module/execution context and signal to the caller higher up what happened. That's exactly what exceptions do.
During development you want to see the exception in all its ugly glory to be able to debug. In production you do not want your users to see exceptions, but instead present them with a nice looking error screen and/or have all sorts of bells and whistles go off which notify the admin/developer/CTO/whoever.
That means, in production you only want one global error handler which responds appropriately if an unexpected, uncaught exception occurs. The exception should be thrown and (not) caught exactly as in development, you do not want two completely separate code paths. This global error handler could be set conditionally by some bootstrapping script simply with set_exception_handler; or probably even better, you configure your web server appropriately to serve useful error pages. Configuring the web server is the best possible way, since this is a system specific setting (production only) without any need to change anything about your code.
The only time you should actually write a try..catch is if there's a legitimate reason a subsystem might fail and you have a backup plan. E.g.:
try {
$file = download_file_from_url($url);
echo "Cool, got your file.";
} catch (HttpNotFound $e) {
echo "Hey user, that file doesn't exist.";
} catch (HttpEmptyResponse $e) {
echo "Hey user, that file seems empty.";
}
..
In this scenario a failed HTTP download is an expected outcome and can be handled well with exceptions, so this is a good use case. But don't reflexively try to catch them all even if they don't represent expected outcomes.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With