-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Custom ExceptionController using extended class #5757
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
Conversation
👍 |
and set the arguments to load the specific needed services:: | ||
|
||
# app/config/services.yml | ||
appbundle.twig.controller.exception: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this service be called app.twig.exception_controller
instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, af089f0.
@iagomelanias thanks for proposing this improvement. We really like when people fill in the existing gaps in the documentation. I've left a few comments and proposed fixes. Don't worry because your proposal is right. We just need to tweak it a bit to better fit the Symfony docs. Thanks! |
Thank you @javiereguiluz. No problem, i have changed the commented lines. Could you review it again? I hope i can contribute with the community more times, it's amazing to help a little bit the job you're doing. Thanks! |
Instead of creating a new exception controller from scratch you can, of course, | ||
also extend the default :class:`Symfony\\Bundle\\TwigBundle\\Controller\\ExceptionController`. | ||
In that case, you might want to override one or both of the ``showAction()`` and | ||
``findTemplate()`` methods. The latter one locates the template to be used. | ||
|
||
To create your own controller logic extending from the ExceptionController, simply |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the class name should be enclosed by double backticks
@iagomelanias Please apologise the long delay in reviewing your pull request. I have left some minor comments. But we can also address them while merging if you are not able to address them in the meantime. |
Although I know we documented it this way, I can't see why we extend the base So while I like your contribution, I think we should rewrite it a bit:
This shows exactly what's needed for a custom exception controller: A normal controller action that is designed to show an error message. Can you maybe make this change? (I would very much like that, as you'll get full credits for your work) If you can't, please say so and we can take this over. |
Closing this. See my comment here: #4181 (comment) Unless someone can tell me otherwise, I don't even think that overriding the error controller should be something we recommend. Everything can be done in an event subscriber. I'd rather show just one, clean way of doing things :). Thanks for the PR! |
Last week, i implemented a custom ExceptionController extending from the default class. I didn't find the complete information in this doc, so i made a change explaining a little better how it should be done.