Slim: Suggestions for error-handling in v4
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.
About this issue
- Original URL
- State: closed
- Created 7 years ago
- Reactions: 1
- Comments: 17 (8 by maintainers)
Commits related to this issue
- Merge branch 'l0gicgate-4.x-ErrorMiddleware' into 4.x Closes #2398 Fixes #2041 Fixes #2170 Fixes #2393 — committed to slimphp/Slim by akrabat 6 years ago
You can safely assume that Whoops will never be a dependency of Slim.
@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,NotAllowedHandlerand the default error handler.We should continue the discussion in my proposed PR #2222 and close this issue.
@geggleto @alexweissman looking further at the source, I see that the
notFoundHandlerand thenotAllowedHandlergive 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.
@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.