[TwigBundle] Fix error page preview for custom twig.exception_controller #12147
Conversation
…g.exception_controller The 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 (which may be the default one). This requires us to pass an additional parameter to ExceptionController::showAction to be able to get 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 from symfony/symfony#12096 (and hopefully the fix in symfony/symfony#12147). Commits ------- d02c7c4 Updates according to GH feedback 8e70373 Document error page preview in Symfony ~2.6 (#4293)
|
@mpdude In your PR description, the link to "Fixed tickets" targets this PR. |
|
Yes, this is the issue that describes the problem an provides a fix for it. Should I do it differently...? |
| @@ -26,47 +27,43 @@ | |||
| class ExceptionController | |||
| { | |||
| protected $twig; | |||
| protected $debug; | |||
| protected $showException; | |||
There was a problem hiding this comment.
Right. Is that an issue even if it is not @api?
There was a problem hiding this comment.
Yes, we try to avoid Bc breaks whenever possible. So, the old name should be kept.
|
If I am not missing anything, this should be 100% BC now. |
| * | ||
| * @return Response | ||
| */ | ||
| protected function createResponse(Request $request, FlattenException $exception, $debug, DebugLoggerInterface $logger = null, $currentContent = '') |
There was a problem hiding this comment.
For others, this method was introduced in 2.6, so removing it is not a BC break.
There was a problem hiding this comment.
but only if we merge this before making 2.6 stable
|
👍 |
| @@ -48,25 +57,20 @@ public function __construct(\Twig_Environment $twig, $debug) | |||
| public function showAction(Request $request, FlattenException $exception, DebugLoggerInterface $logger = null) | |||
| { | |||
| $currentContent = $this->getAndCleanOutputBuffering($request->headers->get('X-Php-Ob-Level', -1)); | |||
| $showException = $request->get('showException', $this->debug); // As opposed to an additional parameter, this maintains BC | |||
There was a problem hiding this comment.
should be $request->attributes->get('showException' actually
|
Except for the change I requested above, this is fine for me |
|
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/a The `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 (which may be the default one). This requires us to pass an additional parameter to `ExceptionController::showAction` to be able to get 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
|
@mpdude Actually, that breaks tests :( Can you have a look at the problems? https://travis-ci.org/symfony/symfony/jobs/41700375 |
|
It’s the change in 7bb1abe that @stof requested. Is the way I set up the request wrong? That parameter does not show up in |
|
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) |
|
Fix (at least for one issue) is in #12541. |
|
Where can I find more about the different semantics of Request::get and Request::$attribtues and when to use which? |
|
@mpdude Does http://symfony.com/doc/current/components/http_foundation/introduction.html#accessing-request-data make it clear for you? |
|
Yes, or http://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 the error 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.)