-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[4.x] Make error renderers in the error handler customizable #2734
Conversation
I wonder if we should just pass Everything looks good though so far for this PR, great work! |
I am currently away. Will have to check that next week. Thanks for your kind words. |
I think, that could work. In this case though, the renderer(s) would be instantiated in every request, even if no error gets triggered. Could that lead to a performance issue? To achieve that, we would have to change the signature of the constructor |
If someone extended the base handler and overrode the existing constructor then they'd have to change the constructor signature then I guess we'd break their code but that's a pretty minor fix. We're still in beta so I'd rather break things now if anything. It's a much cleaner way to instantiate things to just use |
@adriansuter this PR looks good to go, when you remove the WIP from the title I'll merge. Thanks |
What about #2737? |
This PR addresses #2731.
The default error handler has now a registry of known content types and their corresponding error renderers. It is possible to register new content types (or update existing content types) using
\Slim\Handlers\ErrorHandler::registerErrorRenderer()
.The default error renderer can also be defined using
\Slim\Handlers\ErrorHandler::setDefaultErrorRenderer()
.I removed the property
$knownContentTypes
as this information could now be read directly from the array keys of the new property$errorRenderers
. Not sure if that would break things for users.Usage
A user could define a new renderer
and then after instantiating the app, the user could do the following
Further work to do
I think it would be nice, if we could get the Container into the renderer (if defined). So that the renderer can get the template engine for example.
Idea: A new interface called
ContainerAwareInterface
that has exactly one method calledsetContainer()
. So when the renderer gets instantiated, we could check if the renderer implements this interface and if so, we could inject the container into the renderer.