Skip to content

[FrameworkBundle] enable ErrorHandler in prod #12081

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 3, 2014

Conversation

nicolas-grekas
Copy link
Member

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #11053, #8281
License MIT
Doc PR -
  • 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
  • DebugHandlersListener also has a new $scream parameter to control if silenced errors are logged or not

@nicolas-grekas nicolas-grekas force-pushed the scream-mode branch 2 times, most recently from af0ecf4 to c3d012c Compare September 30, 2014 12:16
@nicolas-grekas
Copy link
Member Author

This PR is ready for review/vote. I updated the patch to have almost no behavioral change but only semantic added-value.
Instead of replacing an argument, I just added a new one :)

@nicolas-grekas nicolas-grekas force-pushed the scream-mode branch 4 times, most recently from f3943a9 to 423a286 Compare October 2, 2014 09:01
@stof
Copy link
Member

stof commented Oct 2, 2014

looks good to me. 👍

@nicolas-grekas nicolas-grekas changed the title [Debug] make screaming configurable [FrameworkBundle] enable ErrorHandler in prod Oct 2, 2014
@@ -6,7 +6,9 @@

<parameters>
<parameter key="debug.debug_handlers_listener.class">Symfony\Component\HttpKernel\EventListener\DebugHandlersListener</parameter>
<parameter key="debug.error_handler.class">Symfony\Component\Debug\ErrorHandler</parameter>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we don't want to introduce more of these params.

Copy link
Member

Choose a reason for hiding this comment

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

agreed

<argument>null</argument><!-- %templating.helper.code.file_link_format% -->
</service>

<service id="debug.error_handler" class="Symfony\Component\Debug\ErrorHandler" factory-class="Symfony\Component\Debug\ErrorHandler" factory-method="register" public="false">
Copy link
Contributor

Choose a reason for hiding this comment

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

what is this service for?

Copy link
Member Author

Choose a reason for hiding this comment

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

Now that it is private I can drop it you're right.
I may reintroduce it later to bind it to new configuration options for controlling scope_at and trace_at,
that's why I added it in the first place.
Removing for now.

@Tobion
Copy link
Contributor

Tobion commented Oct 2, 2014

That the console now correctly displays fatal errors is cool. But this will only be working when using the framework bundle. Can this not be handled within the component?

@nicolas-grekas
Copy link
Member Author

Having this in the framework bundle allows configuration flexibility.
But then doesn't prevent adding fatal error handling right into the console component later in an other PR.

@fabpot
Copy link
Member

fabpot commented Oct 3, 2014

Thank you @nicolas-grekas.

@fabpot fabpot merged commit fac3cc4 into symfony:master Oct 3, 2014
fabpot added a commit that referenced this pull request Oct 3, 2014
…-grekas)

This PR was merged into the 2.6-dev branch.

Discussion
----------

[FrameworkBundle] enable ErrorHandler in prod

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

-  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
- DebugHandlersListener also has a new $scream parameter to control is silenced errors are logged or not

Commits
-------

fac3cc4 [FrameworkBundle] register ErrorHandler at boot time
4acf5d3 [Debug] make screaming configurable
4d0ab7d [FrameworkBundle] enable ErrorHandler in prod
@nicolas-grekas nicolas-grekas deleted the scream-mode branch October 3, 2014 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants