Skip to content

Commit

Permalink
minor #36353 [ErrorHandler] Remove trigger_deprecation frame from tra…
Browse files Browse the repository at this point in the history
…ce (fancyweb, nicolas-grekas)

This PR was merged into the 5.1-dev branch.

Discussion
----------

[ErrorHandler] Remove trigger_deprecation frame from trace

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

This accounts for the new `trigger_deprecation` function from [symfony/deprecation-contracts](https://github.com/symfony/deprecation-contracts).

Replaces #36329

Commits
-------

d4eb4a4 [ErrorHandler] Remove trigger_deprecation frame from trace (add tests)
c293aee [ErrorHandler] Remove trigger_deprecation frame from trace
  • Loading branch information
fabpot committed Apr 12, 2020
2 parents 1ee1c81 + d4eb4a4 commit 607e8d6
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 7 deletions.
31 changes: 25 additions & 6 deletions src/Symfony/Component/ErrorHandler/ErrorHandler.php
Expand Up @@ -91,7 +91,7 @@ class ErrorHandler
private $tracedErrors = 0x77FB; // E_ALL - E_STRICT - E_PARSE
private $screamedErrors = 0x55; // E_ERROR + E_CORE_ERROR + E_COMPILE_ERROR + E_PARSE
private $loggedErrors = 0;
private $traceReflector;
private $configureException;
private $debug;

private $isRecursive = 0;
Expand Down Expand Up @@ -187,8 +187,14 @@ public function __construct(BufferingLogger $bootstrappingLogger = null, bool $d
$this->bootstrappingLogger = $bootstrappingLogger;
$this->setDefaultLogger($bootstrappingLogger);
}
$this->traceReflector = new \ReflectionProperty('Exception', 'trace');
$this->traceReflector->setAccessible(true);
$traceReflector = new \ReflectionProperty('Exception', 'trace');
$traceReflector->setAccessible(true);
$this->configureException = \Closure::bind(static function ($e, $trace, $file = null, $line = null) use ($traceReflector) {
$traceReflector->setValue($e, $trace);
$e->file = $file ?? $e->file;
$e->line = $line ?? $e->line;
}, null, new class() extends \Exception {
});
$this->debug = $debug;
}

Expand Down Expand Up @@ -473,9 +479,9 @@ public function handleError(int $type, string $message, string $file, int $line)
if ($throw || $this->tracedErrors & $type) {
$backtrace = $errorAsException->getTrace();
$lightTrace = $this->cleanTrace($backtrace, $type, $file, $line, $throw);
$this->traceReflector->setValue($errorAsException, $lightTrace);
($this->configureException)($errorAsException, $lightTrace, $file, $line);
} else {
$this->traceReflector->setValue($errorAsException, []);
($this->configureException)($errorAsException, []);
$backtrace = [];
}
}
Expand Down Expand Up @@ -736,7 +742,7 @@ protected function getErrorEnhancers(): iterable
/**
* Cleans the trace by removing function arguments and the frames added by the error handler and DebugClassLoader.
*/
private function cleanTrace(array $backtrace, int $type, string $file, int $line, bool $throw): array
private function cleanTrace(array $backtrace, int $type, string &$file, int &$line, bool $throw): array
{
$lightTrace = $backtrace;

Expand All @@ -746,6 +752,19 @@ private function cleanTrace(array $backtrace, int $type, string $file, int $line
break;
}
}
if (E_USER_DEPRECATED === $type) {
for ($i = 0; isset($lightTrace[$i]); ++$i) {
if (!isset($lightTrace[$i]['file'], $lightTrace[$i]['line'], $lightTrace[$i]['function'])) {
continue;
}
if (!isset($lightTrace[$i]['class']) && 'trigger_deprecation' === $lightTrace[$i]['function']) {
$file = $lightTrace[$i]['file'];
$line = $lightTrace[$i]['line'];
$lightTrace = \array_slice($lightTrace, 1 + $i);
break;
}
}
}
if (class_exists(DebugClassLoader::class, false)) {
for ($i = \count($lightTrace) - 2; 0 < $i; --$i) {
if (DebugClassLoader::class === ($lightTrace[$i]['class'] ?? null)) {
Expand Down
25 changes: 25 additions & 0 deletions src/Symfony/Component/ErrorHandler/Tests/ErrorHandlerTest.php
Expand Up @@ -652,4 +652,29 @@ public function testAssertQuietEval()
$this->assertSame('warning', $logs[0][0]);
$this->assertSame('Warning: assert(): assert(false) failed', $logs[0][1]);
}

public function testHandleTriggerDeprecation()
{
try {
$handler = ErrorHandler::register();
$handler->setDefaultLogger($logger = new BufferingLogger());

$expectedLine = __LINE__ + 1;
trigger_deprecation('foo', '1.2.3', 'bar');

/** @var \ErrorException $exception */
$exception = $logger->cleanLogs()[0][2]['exception'];

$this->assertSame($expectedLine, $exception->getLine());
$this->assertSame(__FILE__, $exception->getFile());

$frame = $exception->getTrace()[0];
$this->assertSame(__CLASS__, $frame['class']);
$this->assertSame(__FUNCTION__, $frame['function']);
$this->assertSame('->', $frame['type']);
} finally {
restore_error_handler();
restore_exception_handler();
}
}
}
3 changes: 2 additions & 1 deletion src/Symfony/Component/ErrorHandler/composer.json
Expand Up @@ -23,7 +23,8 @@
},
"require-dev": {
"symfony/http-kernel": "^4.4|^5.0",
"symfony/serializer": "^4.4|^5.0"
"symfony/serializer": "^4.4|^5.0",
"symfony/deprecation-contracts": "^2.1"
},
"autoload": {
"psr-4": { "Symfony\\Component\\ErrorHandler\\": "" },
Expand Down
12 changes: 12 additions & 0 deletions src/Symfony/Component/HttpKernel/Kernel.php
Expand Up @@ -491,6 +491,18 @@ protected function initializeContainer()
break;
}
}
for ($i = 0; isset($backtrace[$i]); ++$i) {
if (!isset($backtrace[$i]['file'], $backtrace[$i]['line'], $backtrace[$i]['function'])) {
continue;
}
if (!isset($backtrace[$i]['class']) && 'trigger_deprecation' === $backtrace[$i]['function']) {
$file = $backtrace[$i]['file'];
$line = $backtrace[$i]['line'];
$backtrace = \array_slice($backtrace, 1 + $i);
break;
}
}

// Remove frames added by DebugClassLoader.
for ($i = \count($backtrace) - 2; 0 < $i; --$i) {
if (\in_array($backtrace[$i]['class'] ?? null, [DebugClassLoader::class, LegacyDebugClassLoader::class], true)) {
Expand Down

0 comments on commit 607e8d6

Please sign in to comment.