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

[TwigBundle] Deprecating error templates for non-html formats and using ErrorRenderer as fallback#31398

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
Tobion merged 1 commit intosymfony:4.4fromyceruto:twig_exception_controller
Jul 24, 2019

Conversation

@yceruto
Copy link
Member

@ycerutoyceruto commentedMay 6, 2019
edited
Loading

QA
Branch?4.4
Bug fix?no
New feature?yes
BC breaks?no
Deprecations?yes
Tests pass?yes
Fixed tickets-
LicenseMIT
Doc PR-

In the previousPR we created a new mechanism to render any PHP error/exception in a formatted string, which if the FB is enabled, would return an HTTP Response according to the preferred Request format (html, json, xml, txt, etc.), but when installing the TwigBundle this rendering mechanism is replaced by the current ExceptionController.

This ExceptionController allows us to render custom error pages based on Twig in many formats, just what is already supported with the new ErrorRenderer component, so let's deprecate this in favor of the native.

@ycerutoycerutoforce-pushed thetwig_exception_controller branch 4 times, most recently from752e607 toefaf91eCompareMay 6, 2019 17:11
@yceruto
Copy link
MemberAuthor

Hi@OskarStark thanks for your review, but they aren't part of this PR (see description), the last commit should be reviewed for now.

Please, move them to#31065 to track them in the right place ;)

OskarStark reacted with thumbs up emoji

@ycerutoycerutoforce-pushed thetwig_exception_controller branch 2 times, most recently fromdafcc21 toeea898fCompareMay 7, 2019 12:58
@yceruto
Copy link
MemberAuthor

yceruto commentedMay 7, 2019
edited
Loading

eea898f#diff-21899d8070b2bcedfb8ab7f6993331deR73
https://ci.appveyor.com/project/fabpot/symfony/builds/24407001#L1264

I have a problem with the ErrorHandler in functional tests, it does not work because theDeprecationErrorHandler is used instead, hence$kernel->terminateWithException() method is never called and we can't get the json response. Thoughts?

@ycerutoycerutoforce-pushed thetwig_exception_controller branch fromeea898f to61d2b9aCompareMay 8, 2019 21:53
@yceruto
Copy link
MemberAuthor

Status: Needs work

@yceruto
Copy link
MemberAuthor

yceruto commentedJun 5, 2019
edited
Loading

Check later if#31868 solve this issue#31398 (comment).

@fabpot
Copy link
Member

@yceruto Yes, it should fix the issue. I found it while reviewing your PR and Nicolas kindly fixed it. It will soon be on all branches so that you can rebase and see if that works (I tested locally and it worked well).

@yceruto
Copy link
MemberAuthor

@fabpot perfect!! thanks.

@ycerutoycerutoforce-pushed thetwig_exception_controller branch from61d2b9a toc2f4fc2CompareJune 5, 2019 14:37
@ycerutoyceruto changed the base branch frommaster to4.4June 5, 2019 14:37
@ycerutoycerutoforce-pushed thetwig_exception_controller branch 4 times, most recently from35755ef to46f9183CompareJune 5, 2019 14:47
@yceruto
Copy link
MemberAuthor

yceruto commentedJul 23, 2019
edited
Loading

requires#32693 to make it work properly

@ycerutoycerutoforce-pushed thetwig_exception_controller branch from9266107 tobf0c24aCompareJuly 24, 2019 02:46
Tobion added a commit that referenced this pull requestJul 24, 2019
…ode (preview mode) (yceruto)This PR was merged into the 4.4 branch.Discussion----------[ErrorRenderer] Allow disabling debug content in debug mode (preview mode)| Q             | A| ------------- | ---| Branch?       | 4.4| Bug fix?      | no| New feature?  | yes| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | -| License       | MITRequired by#31398 to show a preview mode of the error for each content format.Usage:https://github.com/symfony/symfony/pull/31398/files#diff-9ff5216ab011f5e48c7835d4138bf825R42Note that you can't enable the debug content in non-debug mode via `X-Debug`. I also added more tests.Commits-------a6bef5e Allow disabling debug content in debug mode (preview mode)
@Tobion
Copy link
Contributor

It took time, but here we go, this is in now. Thank you very much@yceruto.

yceruto reacted with hooray emoji

@TobionTobion merged commitbf0c24a intosymfony:4.4Jul 24, 2019
Tobion added a commit that referenced this pull requestJul 24, 2019
…formats and using ErrorRenderer as fallback (yceruto)This PR was merged into the 4.4 branch.Discussion----------[TwigBundle] Deprecating error templates for non-html formats and using ErrorRenderer as fallback| Q             | A| ------------- | ---| Branch?       | 4.4| Bug fix?      | no| New feature?  | yes| BC breaks?    | no| Deprecations? | yes| Tests pass?   | yes| Fixed tickets | -| License       | MIT| Doc PR        | -In the previous [PR](#31065) we created a new mechanism to render any PHP error/exception in a formatted string, which if the FB is enabled, would return an HTTP Response according to the preferred Request format (html, json, xml, txt, etc.), but when installing the TwigBundle this rendering mechanism is replaced by the current ExceptionController.This ExceptionController allows us to render custom error pages based on Twig in many formats, just what is already supported with the new ErrorRenderer component, so let's deprecate this in favor of the native.Commits-------bf0c24a Deprecating error templates for non-html formats and using ErrorRenderer
fabpot added a commit that referenced this pull requestJul 24, 2019
This PR was merged into the 5.0-dev branch.Discussion----------[TwigBundle] remove deprecations and fix tests| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->| BC breaks?    | yes     <!-- seehttps://symfony.com/bc -->| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->| Fixed tickets || License       | MIT| Doc PR        |Removing deprecations from#31398 which should also fix tests in master as I removed some legacy code like (`exception_controller: ~ # to be removed in 5.0 relying on default`) while resolving conflicts when merging 4.4 into master.Commits-------452f66d [TwigBundle] remove deprecations
fabpot added a commit that referenced this pull requestJul 24, 2019
…e new ErrorRenderer mechanism (yceruto)This PR was merged into the 4.4 branch.Discussion----------[WebProfilerBundle] Decoupling TwigBundle and using the new ErrorRenderer mechanism| Q             | A| ------------- | ---| Branch?       | 4.4| Bug fix?      | no| New feature?  | yes| BC breaks?    | no| Deprecations? | yes| Tests pass?   | yes| Fixed tickets |#31398 (comment)| License       | MIT| Doc PR        | -Commits-------846d3e0 Decoupling TwigBundle and using the new ErrorRenderer mechanism
@ycerutoyceruto deleted the twig_exception_controller branchJuly 24, 2019 12:00
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
symfony-splitter pushed a commit to symfony/framework-bundle 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 (refssymfony/symfony#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 wouldfixsymfony/symfony#31398 (comment), removing here workaround in SecurityBundle.TODO:- [x] Update CHANGELOG & UPGRADE files- [x] Add testsWDYT?Commits-------b79532ab0e Add ErrorController to preview and render errors
@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

@fabpotfabpotfabpot left review comments

@stofstofstof left review comments

@OskarStarkOskarStarkOskarStark left review comments

@srozesrozeAwaiting requested review from sroze

+1 more reviewer

@TobionTobionTobion approved these changes

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.

7 participants

@yceruto@fabpot@Tobion@stof@OskarStark@nicolas-grekas@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp