Closed edudobay closed 6 years ago
Hi @edudobay. I'm not sure whether I get your question right. Have you tried to implement a custom slim error handler? There you have the possibility to decide which (and when) error details are displayed to the client and can log all errors in detail using a logger (e. g. Monolog).
Hi! Yes, I had no problem implementing a custom error handler, but to keep things simple I wanted to reuse the logging behavior that Slim already provides out of the box. The problem is that I can't reuse just the logging part because it's coupled to the rendering part. If I turn on error rendering, logging gets turned off and that’s what I’d like to change.
My current solution is an ugly subclass that “fakes” the displayErrorDetails setting when delegating to the log writer. Of course this is coupled to the Slim implementation; the proposal for Slim 4.x at #2222 (AbstractErrorHandler) would break this, for example.
class ErrorHandlerWithLogging extends \Slim\Handlers\Error
{
protected function writeToErrorLog($throwable)
{
$oldValue = $this->displayErrorDetails;
$this->displayErrorDetails = false;
parent::writeToErrorLog($throwable);
$this->displayErrorDetails = $oldValue;
}
}
The most obvious “pretty” solution for me would be for the constructor to take two flags, say displayErrorDetails
and writeToErrorLog
- and the latter could default to the opposite of the former when not provided, to guarantee backwards-compatible behavior. Something like this:
class Error extends AbstractError
{
public function __construct($displayErrorDetails = false, $writeToErrorLog = null)
{
if ($writeToErrorLog === null) {
$this->writeToErrorLog = !$displayErrorDetails;
} else {
$this->writeToErrorLog = (bool) $writeToErrorLog;
}
// ...
}
}
Am I going too far suggesting this? Or is it possible that Slim could have this separate setting, at least configurable from the constructor of the Error
handler class?
The problem is, that in Slim 4 the method writeToErrorLog
is getting only called if displayErrorDetails
is true
. See here. That means overwriting the writeToErrorLog
method in a extended class will not work in Slim 4.
Have you tried to implement a custom ExceptionMiddleware?
Example:
// Exception middleware
$app->add(function (Request $request, Response $response, $next) {
try {
return $next($request, $response);
} catch (Exception $ex) {
// logging etc...
}
});
@odan despite not being a viable solution in Slim 4, someone will be able to simply extend the AbstractErrorHandler
and stub the constructor and do whatever they want.
If you have any suggestions, feel free to comment on that PR!
Thanks
@l0gicgate Now, that the PR #2222 has been closed, my previous answer is not valid anymore. I think the new Error middleware #2398 is the way to go. I will review it and give you a feedback as soon as possible.
closing as resolved with #2398
Hi! I was configuring error reporting for an application and started to wonder about the
displayErrorDetails
setting: why is displaying errors to the client coupled to not showing them on the log? There are use cases during development where I might want to display the errors to the client but also have them at the log (e.g. if the client may be a mobile app or another application which might not give me a chance to look at the full response body).Overall, it seems to me that switching displaying of error details and printing to the log are separate concepts and should be independent settings. Does this make sense to implement? Of course Slim is not a logging framework and the implementation for this should be kept simple. I would be happy to contribute a PR for this!