Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Added new ErrorController + Preview and enabling there the error renderer mechanism#33271

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

Merged
fabpot merged 1 commit intosymfony:4.4fromyceruto:error_controller
Sep 5, 2019

Conversation

@yceruto
Copy link
Member

@ycerutoyceruto commentedAug 21, 2019•
edited
Loading

QA
Branch?4.4
Bug fix?no
New feature?yes
BC breaks?no
Deprecations?yes
Tests pass?yes (deps=high failure is normal)
Fixed tickets-
LicenseMIT
Doc PRTODO

After deprecating theExceptionController in TwigBundle (refs#31398) thetwig.exception_controller config key becomes useless as feature provided by TwigBundle, while the preview controller is taking more relevance for the error renderer mechanish.

Proposal

  • Deprecate thetwig.exception_controller config key in favor offramework.error_controller with defaultErrorController that activates the error renderer mechanism through the currentExceptionListener, meaning also thatDebugHandlersListener::onKernelException method becomes useless too.
  • Deprecate thePreviewErrorController from TwigBundle in favor of similar in FrameworkBundle.

So you no longer need to install TwigBundle to create a custom error controller or check the preview output of an error renderer (includedTwigHtmlErrorRenderer).

Btw this wouldfix#31398 (comment), removing here workaround in SecurityBundle.

TODO:

  • Update CHANGELOG & UPGRADE files
  • Add tests

WDYT?

Copy link
Member

@fabpotfabpot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

LGTM

@ycerutoyceruto changed the titleAdd ErrorController and PreviewErrorController to FrameworkBundleAdded new ErrorController + Preview and enabling there the error renderer mechanismAug 21, 2019
@ycerutoycerutoforce-pushed theerror_controller branch 2 times, most recently frome64481f to9bc74f9CompareAugust 21, 2019 17:32
@yceruto
Copy link
MemberAuthor

yceruto commentedAug 22, 2019•
edited
Loading

I'm introducing here a default exception controller that will use the error rendering mechanism. If you've a completely different error rendering system you'll can to replace the default controller configuring it onframework.error_controller and done.

That possibility is available with TwigBundle installed (deprecated here), now it'll be possible with FrameworkBundle only.

@yceruto
Copy link
MemberAuthor

yceruto commentedAug 24, 2019•
edited
Loading

Update

As there are two exception listeners when FrameworkBundle and TwigBundle are enabled, I've removed one conditionally: thetwig.exception_listener service unless the optiontwig.exception_listener.controller contains a custom exception controller configured, in that case, I remove theexception_listener instead.

This will be the migration path:

twig:-    exception_controller: 'App\Controller\ExceptionController'+    exception_controller: nullframework:+    error_controller: 'App\Controller\ExceptionController'

@ycerutoycerutoforce-pushed theerror_controller branch 2 times, most recently from1449e0f to2841e59CompareAugust 24, 2019 12:46
@yceruto
Copy link
MemberAuthor

yceruto commentedAug 24, 2019•
edited
Loading

Btw, with this new implementation we should revertsymfony/recipes#640. not after5a12b6d

}

// register the exception controller only if Twig is enabled and required dependencies do exist
if (!class_exists('Symfony\Component\ErrorRenderer\Exception\FlattenException') || !interface_exists('Symfony\Component\EventDispatcher\EventSubscriberInterface')) {
Copy link
MemberAuthor

@ycerutoycerutoAug 24, 2019•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I've removed allinterface_exists() check here becase both interfaces are currently installed as hard deps.

@yceruto
Copy link
MemberAuthor

This is ready for review!

@ycerutoycerutoforce-pushed theerror_controller branch 5 times, most recently from83755cb to7cce955CompareSeptember 2, 2019 21:02
@fabpot
Copy link
Member

Thank you@yceruto.

fabpot added a commit that referenced this pull requestSep 5, 2019
… the error renderer mechanism (yceruto)This PR was merged into the 4.4 branch.Discussion----------Added new ErrorController + Preview and enabling there the error renderer mechanism| Q             | A| ------------- | ---| Branch?       | 4.4| Bug fix?      | no| New feature?  | yes| BC breaks?    | no| Deprecations? | yes| Tests pass?   | yes (deps=high failure is normal)| Fixed tickets | -| License       | MIT| Doc PR        | TODOAfter deprecating the `ExceptionController` in TwigBundle (refs#31398) the `twig.exception_controller` config key becomes useless as feature provided by TwigBundle, while the preview controller is taking more relevance for the error renderer mechanish.**Proposal** * Deprecate the `twig.exception_controller` config key in favor of `framework.error_controller` with default `ErrorController` that activates the error renderer mechanism through the current `ExceptionListener`, meaning also that `DebugHandlersListener::onKernelException` method becomes useless too. * Deprecate the `PreviewErrorController` from TwigBundle in favor of similar in FrameworkBundle.So you no longer need to install TwigBundle to create a custom error controller or check the preview output of an error renderer (included `TwigHtmlErrorRenderer`).Btw this wouldfix#31398 (comment), removing here workaround in SecurityBundle.TODO:- [x] Update CHANGELOG & UPGRADE files- [x] Add testsWDYT?Commits-------b79532a Add ErrorController to preview and render errors
@fabpotfabpot merged commitb79532a intosymfony:4.4Sep 5, 2019
@ycerutoyceruto deleted the error_controller branchSeptember 5, 2019 12:06

// register the exception controller only if Twig is enabled and required dependencies do exist
if (!class_exists('Symfony\Component\ErrorRenderer\Exception\FlattenException') || !interface_exists('Symfony\Component\EventDispatcher\EventSubscriberInterface')) {
// to be removed in 5.0

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

@yceruto can you please send a PR on master to cleanup this?

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

nicolas-grekas added a commit that referenced this pull requestSep 5, 2019
This PR was merged into the 5.0-dev branch.Discussion----------[TwigBundle] Remove legacy code| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | -| License       | MIT| Doc PR        | TODOSee#33271Commits-------abb3258 Remove legacy code
@nicolas-grekasnicolas-grekas modified the milestones:next,4.4Oct 27, 2019
This was referencedNov 12, 2019
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas left review comments

@fabpotfabpotfabpot approved these changes

+1 more reviewer

@TobionTobionTobion left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

4.4

Development

Successfully merging this pull request may close these issues.

5 participants

@yceruto@fabpot@nicolas-grekas@Tobion@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp