Slim: Suggestions for error-handling in v4

[alexweissman]

I propose that error-handling for specific types of exceptions be decoupled from the main application class in v4. Specifically referencing issues #1646 and #1337, singling out the NotFoundException and MethodNotAllowedExceptions for special treatment in App seems kind of arbitrary.

If you take a look at the CoreErrorHandler class we've developed in UserFrosting, it allows you to map an Exception type to a custom ExceptionHandler class. This allows you to map as many different types of exceptions as you want to designated handlers, and even map child exception types to new handlers. IMO, this is a much more flexible way of dealing with exceptions.

Right now, we have the errorHandler service returning an instance of CoreErrorHandler, but the problem is that Slim's custom handlers for NotFoundException and MethodNotAllowedException make it more difficult for developers to do their own switchyarding for these exceptions.

It would be far more flexible if the notFoundHandler and notAllowedHandler services were removed, and everything were passed through the errorHandler service. Perhaps something like my CoreErrorHandler could be built into Slim, or developers could do their own type-checking on the exception to determine an appropriate course of action.

[l0gicgate]

@alexweissman I also have had those thoughts when looking at the error handling in the app class and thought it was a bit clunky.

I'm going to throw a PR together that implements your concept within the next couple days here and submit it for review.

[alexweissman]

@l0gicgate do you have any experience with whoops? I wonder if Slim could just delegate all error-handling to this library.

[l0gicgate]

@geggleto @alexweissman looking further at the source, I see that the notFoundHandler and the notAllowedHandler give insight into the error itself and that's why those handlers were probably created in the first place.

Do we really need to render these in the specific content type? The handlers have capabilities of rendering in HTML, XML and JSON. I'm going to boldly say that I suggest scratching all of that and decoupling the error rendering from the error handling.

We could give the option to the user to pass in a renderer if need be. We could also include by default HTML, JSON, XML and Plain Text renderers.

I dislike the fact that they are tightly coupled together.

[l0gicgate]

@alexweissman whoops seems like a great library. I'm not sure though if @geggleto @akrabat want to depend on that.

[l0gicgate]

@alexweissman @geggleto @akrabat see PR #2222

[alexweissman]

I dislike the fact that they are tightly coupled together.

@l0gicgate Are you talking about my CoreErrorHandler, or Whoops?

[l0gicgate]

@alexweissman I'm saying that I dislike the way the handlers in Slim 3.x are tightly coupled to the rendering of the exception. There's a lot of code that is being repeated between NotFoundHandler, NotAllowedHandler and the default error handler.

We should continue the discussion in my proposed PR #2222 and close this issue.

[akrabat]

@alexweissman whoops seems like a great library. I'm not sure though if @geggleto @akrabat want to depend on that.

You can safely assume that Whoops will never be a dependency of Slim.

[damianopetrungaro]

I do not see any problem with the actual error handler.
It's pretty simple and easy to use and override for your custom behavior.

In the ex agency where i was working i build this very simple boilerplate with a custom error handler

You may also built your own "main" exception handler as we did because our provider didn't support php 7.x and invoke it for grab all the exception.

Not an elegant one, but it was helpful.

Anyway imho the container is the perfect place where bind this stuff. I don't see any possible issue on that :/

[ncou]

just a silly question, but why Slim doesn't use the built in "set_error_handler" and "set_exception_handler" functions from PHP ?
For example, with the set_exception_handler() function doing a call to the "app->handleException()" function.

Actually if an exception/error is throw in the respond() function it will not be catched.

Am i missing something ?

[damianopetrungaro]

@ncou I ask you:
Why should you prefer a set_exception_handler function instead of using the one that is in the container?

[ncou]

i think you can keep using the container with some pseudo code like below, and you could handle the cases for "trigger(E_USER_ERROR)", as a side effect, no more try/catch in the Slim Application.php file, and if there is an error/exception in the respond() function it will be catched :

set_exception_handler([$this, 'handleException']); // use the existing function (searching in the container) as the callable.

set_error_handler(function($e) {
     throw new ErrorException($e->message, $e->severity....etc);
});

[damianopetrungaro]

I do not agree using set_error_handler, i prefer it to be implemented by userland (if needed).

Btw the point (at least i see it in this way) is that the Slim application should catch any possible exception from the router.

If you have errors outside of the execution of the application middleware stack, it's up to you catch it.

I like the way Slim do this job about it, so i do not see any real benefits about it.
You can try/catch around the fw run, or use set_error_handler if you prefer, i don't see any limit about it.

Of course imho 😄

[ncou]

Yes actually Slim mainly catch the "router exception" "middleware stack exception", it's not acting like a "debugger".
I suppose it's the intention since the set_exception_handler is not used.

I wanted to have a confirmation, that it's the direction/philosophy wanted by slim framework.

[damianopetrungaro]

Change "router exception" in "middleware stack exception" and i agree ahahah 😄

[designermonkey]

In a CQRS app I've been working on, we have the concept of Buses for messages; Commands, Queries, Events...

It seemed to me that an Exception is simply another form of message, and could be handled the same way. We would just need a message bus in the app, and allow registration of ExceptionHandlers to types of Exception on that message bus. That allows us to simply catch any Throwable and attempt to handle it with the message bus. If handling failed, we could then respond with a default 'Slim' Exception handler.

I have yet to implement this in my CQRS work...

I know work was done to better our handling in Slim, and alas I have not had the chance to properly review it and see if it fit the above pattern.

If anyone would like to discuss it further, let me know :)

[akrabat]

Slim 4's error handling was reworked as per #2398.

If further improvements are required, let us know.