NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.8k

[FrameworkBundle] enable ErrorHandler in prod#12062

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

Jump to bottom

Closed

nicolas-grekas wants to merge1 commit intosymfony:masterfromnicolas-grekas:prod-error-handler

Closed

[FrameworkBundle] enable ErrorHandler in prod#12062

nicolas-grekas wants to merge1 commit intosymfony:masterfromnicolas-grekas:prod-error-handler

Conversation

Copy link

Member

nicolas-grekas commentedSep 27, 2014

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#11053,#8281
License	MIT
Doc PR	-

This is a first proposal for enabling error handling (mainly logging) in prod envs.
I'd like to make it for 2.6 but need help to make the best decisions.
@Koc worked on the subject but issue#8281 remains.

This misses configuration options for the ErrorHandler and for php ini settings related to error handling:
I'd like to allow apps to define any log channel per log level,
and I'd also like to be able toini_set('error_log', ...) through symfony configuration system, withapp/logs/php.log as default.

WIP state, but early feedback welcomed. We have only a few days to make it.

https://github.com/symfony/symfony/pull/12062/files?w=1

Copy link

Contributor

Koc commentedSep 27, 2014

We haven't solved this issue at last 3 years and IMHO it is ok wait 6 month until 2.7 will be released. Last-minute rush can decrease quality of the sollution.

Logging fatal errors would be better with stacktrace, but xdebug is so slow for production. What about thinking to this issue from other side? Is it possible create production-ready extension from thishttps://wiki.php.net/rfc/engine_exceptions RFC?

Copy link

MemberAuthor

nicolas-grekas commentedSep 27, 2014

The rush is relative: we have 2 months after feature freeze to finalize, and we have some experience enabling this feature in existing prod apps.
About stack trace for fatal errors, I wonder if we can add a prod friendly variant of xdebug_get_function_stack() in our shinny new debug extension for 2.7. I'll talk with@jpauli about that.

Copy link

Contributor

Nicofuma commentedSep 28, 2014

@Koc The result of the poll was no. So I don't think that's so useful prepare something for it.

nicolas-grekas force-pushed theprod-error-handler branch 3 times, most recently fromd303503 to0fa3ac0Compare

September 29, 2014 15:05

nicolas-grekas changed the title~~[WIP] [FrameworkBundle] enable ErrorHandler in prod~~[FrameworkBundle] enable ErrorHandler in prod

Sep 29, 2014

Copy link

MemberAuthor

nicolas-grekas commentedSep 29, 2014

So this is not WIP anymore:

a new debug.error_handler service is the registered PHP error handler, with ErrorHandler::register() as factory
ErrorHandler::register() is patched so that it checks if the currently registered error handler is an instance of ErrorHandler - in which case it returns this instance and don't create a new one.
DebugHandlersListener now listen to ConsoleEvents and re-injects fatal errors within the $app->renderException code path

When kernel.debug=false, no exception is thrown on error (same as today) but the already configured logger is called now, with screaming switched on for fatal errors (which means silenced fatal errors are logged but not the other silenced ones).
The debug mode is unchanged (throws non silenced errors and logs silenced ones).

Copy link

MemberAuthor

nicolas-grekas commentedSep 29, 2014

~~Merging#12065 is required to make tests pass on this PR.~~

lyrixx reviewed

Sep 29, 2014

View reviewed changes

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php Outdated

Copy link

Member

lyrixxSep 29, 2014

Choose a reason for hiding this comment

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

Missing empty line to keep consistency (group by component)

Copy link

Member

lyrixx commentedSep 29, 2014

It seems good to me.

nicolas-grekas force-pushed theprod-error-handler branch 4 times, most recently from9d725b1 to5e43a69Compare

September 29, 2014 18:46

romainneutron added the FrameworkBundle label

Sep 29, 2014

nicolas-grekas force-pushed theprod-error-handler branch 2 times, most recently fromf14c2d9 to513c9a0Compare

September 29, 2014 21:13

Copy link

MemberAuthor

nicolas-grekas commentedSep 29, 2014

So, I switched myself in@stof mode.
I reduced the patch to its minimum, added tests and doc block.
I also replaced the dep on the Console component by a callingdefined() before using theConsoleEvents::COMMAND const.