Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[TwigBundle] Fix error page preview for custom twig.exception_controller#12147
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
…g.exception_controllerThe testErrorPageAction() always used the showAction() of TwigBundle's ExceptionController.You can, however, configure an alternate controller by setting twig.exception_controller.Thus, in order to get a proper preview, we need to forward to this configured controller (whichmay be the default one).This requires us to pass an additional parameter to ExceptionController::showAction to be able toget the *error* page even if configured otherwise in the constructor.
This PR was merged into the master branch.Discussion----------Document error page preview (Symfony ~2.6)This adds documentation how to use the new preview feature fromsymfony/symfony#12096 (and hopefully the fix insymfony/symfony#12147).Commits-------d02c7c4 Updates according to GH feedback8e70373 Document error page preview in Symfony ~2.6 (#4293)
lyrixx commentedOct 23, 2014
@mpdude In your PR description, the link to "Fixed tickets" targetsthis PR. |
mpdude commentedOct 23, 2014
Yes,this is the issue that describes the problem an provides a fix for it. Should I do it differently...? |
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.
bc break
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.
Right. Is that an issue even if it is not@api?
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.
Yes, we try to avoid Bc breaks whenever possible. So, the old name should be kept.
mpdude commentedNov 2, 2014
If I am not missing anything, this should be 100% BC now. |
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.
For others, this method was introduced in 2.6, so removing it is not a BC break.
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.
but only if we merge this before making 2.6 stable
fabpot commentedNov 2, 2014
👍 |
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 be$request->attributes->get('showException' actually
stof commentedNov 21, 2014
Except for the change I requested above, this is fine for me |
fabpot commentedNov 21, 2014
Thank you@mpdude. |
…ption_controller (mpdude)This PR was submitted for the master branch but it was merged into the 2.6 branch instead (closes#12147).Discussion----------[TwigBundle] Fix error page preview for custom twig.exception_controller| Q | A| ------------- | ---| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#12147| License | MIT| Doc PR | n/aThe `testErrorPageAction()` always used the `showAction()` of TwigBundle's `ExceptionController`.You can, however, configure an alternate controller by setting `twig.exception_controller`.Thus, in order to get a proper preview, we need to forward to this configured controller (whichmay be the default one).This requires us to pass an additional parameter to `ExceptionController::showAction` to be able toget the *error* page even if configured otherwise in the constructor.(The other approach would have been to fiddle around with `ExceptionController`'s `debug` flag through a setter when going through the preview action, but that would have been even more messy.)Commits-------2065e00 [TwigBundle] Fix error page preview for custom twig.exception_controller
fabpot commentedNov 21, 2014
fabpot commentedNov 21, 2014
@mpdude Actually, that breaks tests :( Can you have a look at the problems?https://travis-ci.org/symfony/symfony/jobs/41700375 |
mpdude commentedNov 21, 2014
It’s the change in7bb1abe that@stof requested. Is the way Iset up the request wrong? That parameter does not show up in |
xabbuh commentedNov 21, 2014
stof commentedNov 21, 2014
hmm yeah, you are creating the request with a query string setting, not an attribute (so you are not testing the same thing than what the preview controller uses) |
mpdude commentedNov 21, 2014
Fix (at least for one issue) is in#12541. |
mpdude commentedNov 21, 2014
Where can I find more about the different semantics of Request::get and Request::$attribtues and when to use which? |
xabbuh commentedNov 21, 2014
@mpdude Doeshttp://symfony.com/doc/current/components/http_foundation/introduction.html#accessing-request-data make it clear for you? |
mpdude commentedNov 21, 2014
Yes, orhttp://symfony.com/doc/current/book/http_fundamentals.html#book-fundamentals-attributes is even better. Thanks! |
The
testErrorPageAction()always used theshowAction()of TwigBundle'sExceptionController.You can, however, configure an alternate controller by setting
twig.exception_controller.Thus, in order to get a proper preview, we need to forward to this configured controller (which
may be the default one).
This requires us to pass an additional parameter to
ExceptionController::showActionto be able toget theerror page even if configured otherwise in the constructor.
(The other approach would have been to fiddle around with
ExceptionController'sdebugflag through a setter when going through the preview action, but that would have been even more messy.)