Skip to content

Commit 816e296

Browse files
lyrixxlyrixx
committed
[Debug] Better error handling
1. Send the raw exception in the log context instead of custom formatting 2. Add config option to log in Symfony all PHP errors
1 parent 904279e commit 816e296

File tree

8 files changed

+225
-137
lines changed

8 files changed

+225
-137
lines changed

‎src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,15 @@ public function getConfigTreeBuilder()
9494
->beforeNormalization()->ifString()->then(function ($v) { return array($v); })->end()
9595
->prototype('scalar')->end()
9696
->end()
97+
->arrayNode('debug')
98+
->addDefaultsIfNotSet()
99+
->children()
100+
->booleanNode('log_php_errors')
101+
->info('Log PHP errors and disable native error logging')
102+
->defaultValue($this->debug)
103+
->end()
104+
->end()
105+
->end()
97106
->end()
98107
;
99108

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,9 @@ public function load(array $configs, ContainerBuilder $container)
164164
$definition->setPublic(false);
165165
$container->setDefinition('debug.event_dispatcher.parent', $definition);
166166
$container->setAlias('event_dispatcher', 'debug.event_dispatcher');
167-
} else {
167+
}
168+
169+
if (!$config['debug']['log_php_errors']) {
168170
$definition->replaceArgument(1, null);
169171
}
170172

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,9 @@ protected static function getBundleDefaultConfig()
274274
'default_redis_provider' => 'redis://localhost',
275275
),
276276
'workflows' => array(),
277+
'debug' => array(
278+
'log_php_errors' => true,
279+
),
277280
);
278281
}
279282
}

‎src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/logger.html.twig‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
{% if collector.counterrors or collector.countdeprecations or collector.countscreams %}
77
{% set icon %}
88
{% set status_color = collector.counterrors ? 'red' : collector.countdeprecations ? 'yellow' : '' %}
9-
{% set error_count = collector.counterrors + collector.countdeprecations + collector.countscreams %}
9+
{% set error_count = collector.counterrors + collector.countdeprecations %}
1010
{{ include('@WebProfiler/Icon/logger.svg') }}
1111
<span class="sf-toolbar-value">{{ error_count }}</span>
1212
{% endset %}
@@ -55,7 +55,7 @@
5555
{# sort collected logs in groups #}
5656
{% set deprecation_logs, debug_logs, info_and_error_logs, silenced_logs = [], [], [], [] %}
5757
{% for log in collector.logs %}
58-
{% if log.context.level is defined and log.context.type is defined and log.context.type in [constant('E_DEPRECATED'), constant('E_USER_DEPRECATED')] %}
58+
{% if log.context.errorCount is defined and log.context.type is defined and log.context.type in [constant('E_DEPRECATED'), constant('E_USER_DEPRECATED')] %}
5959
{% set deprecation_logs = deprecation_logs|merge([log]) %}
6060
{% elseif log.context.scream is defined and log.context.scream == true %}
6161
{% set silenced_logs = silenced_logs|merge([log]) %}
@@ -170,21 +170,22 @@
170170
{% macro render_log_message(category, log_index, log, is_deprecation = false) %}
171171
{{ log.message }}
172172

173+
{% if log.context.errorCount is defined and log.context.errorCount > 1 %}
174+
<span class="text-small text-bold">({{ log.context.errorCount }} times)</span>
175+
{% endif %}
176+
173177
{% if is_deprecation %}
174-
{% set stack = log.context.stack|default([]) %}
175-
{% set stack_id = 'sf-call-stack-' ~ category ~ '-' ~ log_index %}
178+
{% set trace = log.context.trace|default([]) %}
179+
{% set trace_id = 'sf-call-trace-' ~ category ~ '-' ~ log_index %}
176180

177-
{% if log.context.errorCount is defined %}
178-
<span class="text-small text-bold">({{ log.context.errorCount }} times)</span>
179-
{% endif %}
180181

181-
{% if stack %}
182-
<button class="btn-link text-small sf-toggle" data-toggle-selector="#{{ stack_id }}" data-toggle-alt-content="Hide stack trace">Show stack trace</button>
182+
{% if trace %}
183+
<button class="btn-link text-small sf-toggle" data-toggle-selector="#{{ trace_id }}" data-toggle-alt-content="Hide trace trace">Show trace trace</button>
183184
{% endif %}
184185

185-
{% for index, call in stack if index > 1 %}
186+
{% for index, call in trace if index > 1 %}
186187
{% if index == 2 %}
187-
<ul class="sf-call-stack hidden" id="{{ stack_id }}">
188+
<ul class="sf-call-trace hidden" id="{{ trace_id }}">
188189
{% endif %}
189190

190191
{% if call.class is defined %}
@@ -206,7 +207,7 @@
206207
{% endif %}
207208
</li>
208209

209-
{% if index == stack|length - 1 %}
210+
{% if index == trace|length - 1 %}
210211
</ul>
211212
{% endif %}
212213
{% endfor %}
@@ -224,7 +225,7 @@
224225
<a class="btn-link text-small sf-toggle" data-toggle-selector="#{{ context_id }}" data-toggle-alt-content="Hide full context">Show full context</a>
225226

226227
<div id="{{ context_id }}" class="context">
227-
<pre>{{ context_dump }}</pre>
228+
{{ dump(log.context) }}
228229
</div>
229230
{% else %}
230231
{{ context_dump }}

‎src/Symfony/Component/Debug/ErrorHandler.php‎

Lines changed: 39 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use Symfony\Component\Debug\Exception\FatalErrorException;
1818
use Symfony\Component\Debug\Exception\FatalThrowableError;
1919
use Symfony\Component\Debug\Exception\OutOfMemoryException;
20+
use Symfony\Component\Debug\Exception\SilencedErrorContext;
2021
use Symfony\Component\Debug\FatalErrorHandler\UndefinedFunctionFatalErrorHandler;
2122
use Symfony\Component\Debug\FatalErrorHandler\UndefinedMethodFatalErrorHandler;
2223
use Symfony\Component\Debug\FatalErrorHandler\ClassNotFoundFatalErrorHandler;
@@ -43,6 +44,7 @@
4344
* can see them and weight them as more important to fix than others of the same level.
4445
*
4546
* @author Nicolas Grekas <p@tchwork.com>
47+
* @author Grégoire Pineau <lyrixx@lyrixx.info>
4648
*/
4749
class ErrorHandler
4850
{
@@ -88,7 +90,6 @@ class ErrorHandler
8890
private $screamedErrors = 0x55; // E_ERROR + E_CORE_ERROR + E_COMPILE_ERROR + E_PARSE
8991
private $loggedErrors = 0;
9092

91-
private $loggedTraces = array();
9293
private $isRecursive = 0;
9394
private $isRoot = false;
9495
private $exceptionHandler;
@@ -221,7 +222,7 @@ public function setLoggers(array $loggers)
221222

222223
if ($flush) {
223224
foreach ($this->bootstrappingLogger->cleanLogs() as $log) {
224-
$type = $log[2]['type'];
225+
$type = $log[2]['exception']->getSeverity();
225226
if (!isset($flush[$type])) {
226227
$this->bootstrappingLogger->log($log[0], $log[1], $log[2]);
227228
} elseif ($this->loggers[$type][0]) {
@@ -361,6 +362,8 @@ private function reRegister($prev)
361362
*/
362363
public function handleError($type, $message, $file, $line, array $context, array $backtrace = null)
363364
{
365+
// Level is the current error reporting level to manage silent error.
366+
// Strong errors are not authorized to be silenced.
364367
$level = error_reporting() | E_RECOVERABLE_ERROR | E_USER_ERROR | E_DEPRECATED | E_USER_DEPRECATED;
365368
$log = $this->loggedErrors & $type;
366369
$throw = $this->thrownErrors & $type & $level;
@@ -373,24 +376,28 @@ public function handleError($type, $message, $file, $line, array $context, array
373376
if (null !== $backtrace && $type & E_ERROR) {
374377
// E_ERROR fatal errors are triggered on HHVM when
375378
// hhvm.error_handling.call_user_handler_on_fatals=1
376-
// which is the way to get their backtrace.
379+
// which is the way to get their backtraces.
377380
$this->handleFatalError(compact('type', 'message', 'file', 'line', 'backtrace'));
378381

379382
return true;
380383
}
381384

382-
if ($throw) {
383-
if (null !== self::$toStringException) {
384-
$throw = self::$toStringException;
385-
self::$toStringException = null;
386-
} elseif (($this->scopedErrors & $type) && class_exists(ContextErrorException::class)) {
387-
$throw = new ContextErrorException($this->levels[$type].': '.$message, 0, $type, $file, $line, $context);
388-
} else {
389-
$throw = new \ErrorException($this->levels[$type].': '.$message, 0, $type, $file, $line);
390-
}
385+
$logMessage = $this->levels[$type].': '.$message;
386+
387+
if (null !== self::$toStringException) {
388+
$errorAsException = self::$toStringException;
389+
self::$toStringException = null;
390+
} elseif (!$throw && !($type & $level)) {
391+
$errorAsException = new SilencedErrorContext($type, $file, $line);
392+
} elseif ($this->scopedErrors & $type) {
393+
$errorAsException = new ContextErrorException($logMessage, 0, $type, $file, $line, $context);
394+
} else {
395+
$errorAsException = new \ErrorException($logMessage, 0, $type, $file, $line);
396+
}
391397

398+
if ($throw) {
392399
if (E_USER_ERROR & $type) {
393-
$backtrace = $backtrace ?: $throw->getTrace();
400+
$backtrace = $backtrace ?: $errorAsException->getTrace();
394401

395402
for ($i = 1; isset($backtrace[$i]); ++$i) {
396403
if (isset($backtrace[$i]['function'], $backtrace[$i]['type'], $backtrace[$i - 1]['function'])
@@ -410,7 +417,7 @@ public function handleError($type, $message, $file, $line, array $context, array
410417
if (($e instanceof \Exception || $e instanceof \Throwable) && $e->__toString() === $message) {
411418
if (1 === $i) {
412419
// On HHVM
413-
$throw = $e;
420+
$errorAsException = $e;
414421
break;
415422
}
416423
self::$toStringException = $e;
@@ -421,7 +428,7 @@ public function handleError($type, $message, $file, $line, array $context, array
421428

422429
if (1 < $i) {
423430
// On PHP (not on HHVM), display the original error message instead of the default one.
424-
$this->handleException($throw);
431+
$this->handleException($errorAsException);
425432

426433
// Stop the process by giving back the error to the native handler.
427434
return false;
@@ -430,47 +437,23 @@ public function handleError($type, $message, $file, $line, array $context, array
430437
}
431438
}
432439

433-
throw $throw;
434-
}
435-
436-
// For duplicated errors, log the trace only once
437-
$e = md5("{$type}/{$line}/{$file}\x00{$message}", true);
438-
$trace = true;
439-
440-
if (!($this->tracedErrors & $type) || isset($this->loggedTraces[$e])) {
441-
$trace = false;
442-
} else {
443-
$this->loggedTraces[$e] = 1;
444-
}
445-
446-
$e = compact('type', 'file', 'line', 'level');
447-
448-
if ($type & $level) {
449-
if ($this->scopedErrors & $type) {
450-
$e['scope_vars'] = $context;
451-
if ($trace) {
452-
$e['stack'] = $backtrace ?: debug_backtrace(DEBUG_BACKTRACE_PROVIDE_OBJECT);
453-
}
454-
} elseif ($trace) {
455-
if (null === $backtrace) {
456-
$e['stack'] = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS);
457-
} else {
458-
foreach ($backtrace as &$frame) {
459-
unset($frame['args'], $frame);
460-
}
461-
$e['stack'] = $backtrace;
462-
}
463-
}
440+
throw $errorAsException;
464441
}
465442

466443
if ($this->isRecursive) {
467444
$log = 0;
468445
} elseif (self::$stackedErrorLevels) {
469-
self::$stackedErrors[] = array($this->loggers[$type][0], ($type & $level) ? $this->loggers[$type][1] : LogLevel::DEBUG, $message, $e);
446+
self::$stackedErrors[] = array(
447+
$this->loggers[$type][0],
448+
($type & $level) ? $this->loggers[$type][1] : LogLevel::DEBUG,
449+
$logMessage,
450+
array('exception' => $errorAsException),
451+
);
470452
} else {
471453
try {
472454
$this->isRecursive = true;
473-
$this->loggers[$type][0]->log(($type & $level) ? $this->loggers[$type][1] : LogLevel::DEBUG, $message, $e);
455+
$level = ($type & $level) ? $this->loggers[$type][1] : LogLevel::DEBUG;
456+
$this->loggers[$type][0]->log($level, $logMessage, array('exception' => $errorAsException));
474457
} finally {
475458
$this->isRecursive = false;
476459
}
@@ -495,20 +478,13 @@ public function handleException($exception, array $error = null)
495478
$type = $exception instanceof FatalErrorException ? $exception->getSeverity() : E_ERROR;
496479

497480
if (($this->loggedErrors & $type) || $exception instanceof FatalThrowableError) {
498-
$e = array(
499-
'type' => $type,
500-
'file' => $exception->getFile(),
501-
'line' => $exception->getLine(),
502-
'level' => error_reporting(),
503-
'stack' => $exception->getTrace(),
504-
);
505481
if ($exception instanceof FatalErrorException) {
506482
if ($exception instanceof FatalThrowableError) {
507483
$error = array(
508484
'type' => $type,
509485
'message' => $message = $exception->getMessage(),
510-
'file' => $e['file'],
511-
'line' => $e['line'],
486+
'file' => $exception->getFile(),
487+
'line' => $exception->getLine(),
512488
);
513489
} else {
514490
$message = 'Fatal '.$exception->getMessage();
@@ -523,7 +499,7 @@ public function handleException($exception, array $error = null)
523499
}
524500
}
525501
if ($this->loggedErrors & $type) {
526-
$this->loggers[$type][0]->log($this->loggers[$type][1], $message, $e);
502+
$this->loggers[$type][0]->log($this->loggers[$type][1], $message, array('exception' => $exception));
527503
}
528504
if ($exception instanceof FatalErrorException && !$exception instanceof OutOfMemoryException && $error) {
529505
foreach ($this->getFatalErrorHandlers() as $handler) {
@@ -629,19 +605,19 @@ public static function unstackErrors()
629605
$level = array_pop(self::$stackedErrorLevels);
630606

631607
if (null !== $level) {
632-
$e = error_reporting($level);
633-
if ($e !== ($level | E_PARSE | E_ERROR | E_CORE_ERROR | E_COMPILE_ERROR)) {
608+
$errorReportingLevel = error_reporting($level);
609+
if ($errorReportingLevel !== ($level | E_PARSE | E_ERROR | E_CORE_ERROR | E_COMPILE_ERROR)) {
634610
// If the user changed the error level, do not overwrite it
635-
error_reporting($e);
611+
error_reporting($errorReportingLevel);
636612
}
637613
}
638614

639615
if (empty(self::$stackedErrorLevels)) {
640616
$errors = self::$stackedErrors;
641617
self::$stackedErrors = array();
642618

643-
foreach ($errors as $e) {
644-
$e[0]->log($e[1], $e[2], $e[3]);
619+
foreach ($errors as $error) {
620+
$error[0]->log($error[1], $error[2], $error[3]);
645621
}
646622
}
647623
}

‎src/Symfony/Component/Debug/Exception/SilencedErrorContext.php‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Debug\Exception;
13+
14+
/**
15+
* Data Object that represents a Silenced Error.
16+
*
17+
* @author Grégoire Pineau <lyrixx@lyrixx.info>
18+
*/
19+
class SilencedErrorContext implements \JsonSerializable
20+
{
21+
private $severity;
22+
private $file;
23+
private $line;
24+
25+
public function __construct($severity, $file, $line)
26+
{
27+
$this->severity = $severity;
28+
$this->file = $file;
29+
$this->line = $line;
30+
}
31+
32+
public function getSeverity()
33+
{
34+
return $this->severity;
35+
}
36+
37+
public function getFile()
38+
{
39+
return $this->file;
40+
}
41+
42+
public function getLine()
43+
{
44+
return $this->line;
45+
}
46+
47+
public function JsonSerialize()
48+
{
49+
return array(
50+
'severity' => $this->severity,
51+
'file' => $this->file,
52+
'line' => $this->line,
53+
);
54+
}
55+
}

0 commit comments

Comments
 (0)