8000 [BC Break][Debug] #2042 implementation of fatal error logging by Koc · Pull Request #6474 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[BC Break][Debug] #2042 implementation of fatal error logging #6474

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

[BC Break][Debug] #2042 implementation of fatal error logging #6474

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1bb5677
#2042 fatal error logging under when debug enabled
Koc Apr 10, 2013
a5e38d4
minimize and document BC breaks
Koc Apr 25, 2013
6ca1339
fatal errors logging in production environment #2042
Koc Apr 25, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,15 +48,18 @@ public function load(array $configs, ContainerBuilder $container)
// will be used and everything will still work as expected.
$loader->load('translation.xml');

if ($container->getParameter('kernel.debug')) {
$loader->load('debug.xml');
$loader->load('debug.xml');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fabpot Please suggest how can I enable listener that located in this file excluding loading all the file? Add debug_prod.xml?

Copy link
Member

Choose a reason for hiding this comment

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

Splitting the file in two makes sense.


if ($container->getParameter('kernel.debug')) {
// only HttpKernel needs the debug event dispatcher
$definition = $container->findDefinition('http_kernel');
$arguments = $definition->getArguments();
$arguments[0] = new Reference('debug.event_dispatcher');
$arguments[2] = new Reference('debug.controller_resolver');
$definition->setArguments($arguments);
} else {
$container->removeDefinition('debug.deprecation_logger_listener');
$container->setParameter('debug.container.dump', null);
}

$configuration = $this->getConfiguration($configs, $container);
Expand Down
12 changes: 10 additions & 2 deletions src/Symfony/Bundle/FrameworkBundle/Resources/config/debug.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
<parameter key="debug.stopwatch.class">Symfony\Component\Stopwatch\Stopwatch</parameter>
<parameter key="debug.container.dump">%kernel.cache_dir%/%kernel.container_class%.xml</parameter>
<parameter key="debug.controller_resolver.class">Symfony\Component\HttpKernel\Controller\TraceableControllerResolver</parameter>
<parameter key="debug.deprecation_logger_listener.class">Symfony\Component\HttpKernel\EventListener\DeprecationLoggerListener</parameter>
<parameter key="debug.errors_logger_listener.class">Symfony\Component\HttpKernel\EventListener\ErrorsLoggerListener</parameter>
</parameters>

<services>
Expand All @@ -28,9 +28,17 @@
<argument type="service" id="debug.stopwatch" />
</service>

<service id="debug.deprecation_logger_listener" class="%debug.deprecation_logger_listener.class%">
<service id="debug.deprecation_logger_listener" class="%debug.errors_logger_listener.class%">
<tag name="kernel.event_subscriber" />
<tag name="monolog.logger" channel="deprecation" />
<argument>deprecation</argument>
<argument type="service" id="logger" on-invalid="null" />
</service>

<service id="debug.emergency_logger_listener" class="%debug.errors_logger_listener.class%">
<tag name="kernel.event_subscriber" />
<tag name="monolog.logger" channel="emergency" />
<argument>emergency</argument>
<argument type="service" id="logger" on-invalid="null" />
</service>
</services>
Expand Down
7 changes: 4 additions & 3 deletions src/Symfony/Component/Debug/Debug.php
View file
Open in desktop
8000
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,9 @@ class Debug
* class loader is also registered.
*
* @param integer $errorReportingLevel The level of error reporting you want
* @param Boolean $displayErrors Display errors (for dev environment) or just log they (production usage)
Copy link
Member

Choose a reason for hiding this comment

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

typo: log them

*/
public static function enable($errorReportingLevel = null)
public static function enable($errorReportingLevel = null, $displayErrors = true)
{
if (static::$enabled) {
return;
Expand All @@ -42,10 +43,10 @@ public static function enable($errorReportingLevel = null)

error_reporting(-1);

ErrorHandler::register($errorReportingLevel);
ErrorHandler::register($errorReportingLevel, $displayErrors);
if ('cli' !== php_sapi_name()) {
ExceptionHandler::register();
} elseif (!ini_get('log_errors') || ini_get('error_log')) {
} elseif (!ini_get('log_errors') || ini_get('error_log') && $displayErrors) {
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 it's not correct as && takes precedence over ||, see #7895

ini_set('display_errors', 1);
}

Expand Down
42 changes: 34 additions & 8 deletions src/Symfony/Component/Debug/ErrorHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
* ErrorHandler.
*
* @author Fabien Potencier <fabien@symfony.com>
* @author Konstantin Myakshin <koc-dp@yandex.ru>
*/
class ErrorHandler
{
Expand All @@ -43,20 +44,26 @@ class ErrorHandler

private $reservedMemory;

/** @var LoggerInterface */
private static $logger;
private $displayErrors;

/**
* @var LoggerInterface[] Loggers for channels
*/
private static $loggers = array();

/**
* Registers the error handler.
*
* @param integer $level The level at which the conversion to Exception is done (null to use the error_reporting() value and 0 to disable)
* @param Boolean $displayErrors Display errors (for dev environment) or just log they (production usage)
*
* @return The registered error handler
*/
public static function register($level = null)
public static function register($level = null, $displayErrors = true)
{
$handler = new static();
$handler->setLevel($level);
$handler->setDisplayErrors($displayErrors);

ini_set('display_errors', 0);
set_error_handler(array($handler, 'handle'));
Expand All @@ -71,9 +78,14 @@ public function setLevel($level)
$this->level = null === $level ? error_reporting() : $level;
}

public static function setLogger(LoggerInterface $logger)
public function setDisplayErrors($displayErrors)
{
$this->displayErrors = $displayErrors;
}

public static function setLogger(LoggerInterface $logger, $channel = 'deprecation')
{
self::$logger = $logger;
self::$loggers[$channel] = $logger;
}

/**
Expand All @@ -86,7 +98,7 @@ public function handle($level, $message, $file, $line, $context)
}

if ($level & (E_USER_DEPRECATED | E_DEPRECATED)) {
if (null !== self::$logger) {
if (isset(self::$loggers['deprecation'])) {
if (version_compare(PHP_VERSION, '5.4', '<')) {
$stack = array_map(
function ($row) {
Expand All @@ -99,13 +111,13 @@ function ($row) {
$stack = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 10);
}

self::$logger->warning($message, array('type' => self::TYPE_DEPRECATION, 'stack' => $stack));
self::$loggers['deprecation']->warning($message, array('type' => self::TYPE_DEPRECATION, 'stack' => $stack));
}

return true;
}

if (error_reporting() & $level && $this->level & $level) {
if ($this->displayErrors && error_reporting() & $level && $this->level & $level) {
throw new \ErrorException(sprintf('%s: %s in %s line %d', isset($this->levels[$level]) ? $this->levels[$level] : $level, $message, $file, $line), 0, $level, $file, $line);
}

Expand All @@ -124,6 +136,20 @@ public function handleFatal()
return;
}

if (isset(self::$loggers['emergency'])) {
$fatal = array(
'type' => $type,
'file' => $error['file'],
'line' => $error['line'],
);

self::$loggers['emergency']->emerg($error['message'], $fatal);
}

if (!$this->displayErrors) {
return;
}

// get current exception handler
$exceptionHandler = set_exception_handler(function() {});
restore_exception_handler();
Expand Down
3 changes: 2 additions & 1 deletion src/Symfony/Component/HttpKernel/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ CHANGELOG
2.3.0
-----

* [BC BREAK] renamed `Symfony\Component\HttpKernel\EventListener\DeprecationLoggerListener` to `Symfony\Component\HttpKernel\EventListener\ErrorsLoggerListener` and changed its constructor
* deprecated `Symfony\Component\HttpKernel\Debug\ErrorHandler`, `Symfony\Component\HttpKernel\Debug\ExceptionHandler`,
`Symfony\Component\HttpKernel\Exception\FatalErrorException`, and `Symfony\Component\HttpKernel\Exception\FlattenException`
* deprecated `Symfony\Component\HttpKernel\Kernel::init()``
Expand All @@ -21,7 +22,7 @@ CHANGELOG
* added ControllerReference to create reference of Controllers (used in the FragmentRenderer class)
* [BC BREAK] renamed TimeDataCollector::getTotalTime() to
TimeDataCollector::getDuration()
* updated the MemoryDataCollector to include the memory used in the
* updated the MemoryDataCollector to include the memory used in the
kernel.terminate event listeners
* moved the Stopwatch classes to a new component
* added TraceableControllerResolver
Expand Down
12 changes: 8 additions & 4 deletions ...entListener/DeprecationLoggerListener.php → ...el/EventListener/ErrorsLoggerListener.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,23 +17,27 @@
use Symfony\Component\HttpKernel\KernelEvents;

/**
* Injects the logger into the ErrorHandler, so that it can log deprecation errors.
* Injects the logger into the ErrorHandler, so that it can log various errors.
*
* @author Colin Frei <colin@colinfrei.com>
* @author Konstantin Myakshin <koc-dp@yandex.ru>
*/
class DeprecationLoggerListener implements EventSubscriberInterface
class ErrorsLoggerListener implements EventSubscriberInterface
Copy link
Member

Choose a reason for hiding this comment

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

Renaming the class is a BC break

Copy link
Member

Choose a reason for hiding this comment

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

and changing its constructor is also a BC break

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This class not tagged with @api, but I can revert renaming if needed. But this is inconsistent inject emergency logger via deprecation event listener.

Copy link
Member

Choose a reason for hiding this comment

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

Renaming this class is ok for me as this is not a widely used class anyway (and it was added recently).

{
private $channel;

private $logger;

public function __construct(LoggerInterface $logger = null)
public function __construct($channel, LoggerInterface $logger = null)
{
$this->channel = $channel;
$this->logger = $logger;
}

public function injectLogger()
{
if (null !== $this->logger) {
ErrorHandler::setLogger($this->logger);
ErrorHandler::setLogger($this->logger, $this->channel);
}
}

Expand Down
0