From ecfe37f11f33a8ffa4147072dcb7568ddd622e14 Mon Sep 17 00:00:00 2001 From: Dustin Wilson Date: Wed, 5 Jul 2023 15:24:47 -0500 Subject: [PATCH] Changed argument printing in stack traces --- README.md | 40 ++-------------------------- lib/Catcher/Handler.php | 41 ++++++++++++++++++----------- lib/Catcher/PlainTextHandler.php | 4 +-- lib/Catcher/ThrowableController.php | 38 +++++++++++++------------- tests/cases/TestHandler.php | 34 ++++++++++++++++++++++-- tests/lib/TestingHandler.php | 15 ++++++++++- tests/phpunit.xml | 7 ++--- 7 files changed, 99 insertions(+), 80 deletions(-) diff --git a/README.md b/README.md index 374eef0..2a55566 100644 --- a/README.md +++ b/README.md @@ -197,7 +197,6 @@ abstract class Handler { protected bool $_outputToStderr = true; protected bool $_silent = false; protected string $_timeFormat = 'Y-m-d\TH:i:s.vO'; - protected ?\Closure $_varExporter = null; public function __construct(array $options = []); @@ -246,8 +245,7 @@ _outputPrevious_: When true will output previous throwables. Defaults to _true_. _outputTime_: When true will output times to the output. Defaults to _true_. _outputToStderr_: When the SAPI is cli output errors to stderr. Defaults to _true_. _silent_: When true the handler won't output anything. Defaults to _false_. -_timeFormat_: The PHP-standard date format which to use for times in output. Defaults to _"Y-m-d\\TH:i:s.vO"_. -_varExporter_: A user-defined closure to use when printing arguments in backtraces. Defaults to _null_. +_timeFormat_: The PHP-standard date format which to use for times in output. Defaults to _"Y-m-d\\TH:i:s.vO"_. #### MensBeam\Catcher\Handler::__invoke #### @@ -334,38 +332,4 @@ class PlainTextHandler extends Handler { #### Options #### -_timeFormat_: Same as in `Handler`, but the default changes to _"[H:i:s]"_. - - -## Setting a Custom Variable Exporter ## - -By default internally [`print_r`][e] is used. This is due to tests made internally where it performed the best out of built-in options, including other functions which might otherwise be preferred. Starting in v1.0.2 `Handler`'s `varExporter` option allows for defining how arguments are printed in backtraces in Catcher. Here is an example: - -```php -#!/usr/bin/env php - true, - 'varExporter' => fn(mixed $value): string|bool => VarExporter::export($value) -])); - -throw new \Exception('Ook!'); -``` - -Output: -``` -[21:12:00] Exception: Ook! in file /home/mensbeam/super-awesome-project/ook.php on line 13 - - Stack trace: - 1. Exception /home/mensbeam/super-awesome-project/ook.php:13 - | [ - | 'Ook!', - | ] -``` - -This example above uses the [symfony/var-exporter][f] package for a more modern human-readable variable export. However, using any variable printer is possible. \ No newline at end of file +_timeFormat_: Same as in `Handler`, but the default changes to _"[H:i:s]"_. \ No newline at end of file diff --git a/lib/Catcher/Handler.php b/lib/Catcher/Handler.php index 5ae65eb..0ec53be 100644 --- a/lib/Catcher/Handler.php +++ b/lib/Catcher/Handler.php @@ -59,12 +59,6 @@ abstract class Handler { protected bool $_silent = false; /** The PHP-standard date format which to use for timestamps in output */ protected string $_timeFormat = 'Y-m-d\TH:i:s.vO'; - /** - * A user-defined closure to use when printing arguments in backtraces - * - * @var ?(mixed): string|bool - */ - protected ?\Closure $_varExporter = null; @@ -164,15 +158,14 @@ abstract class Handler { return false; } - // Iterate through the handlers and disable both logging and the custom - // varExporter to prevent infinite looping of error handlers + // Iterate through the handlers and disable logging to prevent + // infinite looping of error handlers $catcher = $exceptionHandler[0]; $handlers = $catcher->getHandlers(); $handlersCount = count($handlers); $silentCount = 0; foreach ($handlers as $h) { $h->setOption('logger', null); - $h->setOption('varExporter', null); if ($h->getOption('silent')) { $silentCount++; @@ -312,11 +305,29 @@ abstract class Handler { echo $string; } - protected function varExporter(mixed $value): string|bool { - $exporter = $this->_varExporter; - set_error_handler([ $this, 'handleError' ]); - $value = ($exporter === null) ? print_r($value, true) : $exporter($value); - restore_error_handler(); - return $value; + protected function serializeArgs(mixed $value): string { + $o = ''; + if (count($value) > 0) { + $o .= '(' . \PHP_EOL; + $a = ''; + foreach ($value as $v) { + $aa = null; + if ($v instanceof \Closure) { + $aa = 'Closure'; + } elseif (is_array($v)) { + $aa = 'array'; + } elseif (is_object($v)) { + $type = gettype($v); + $aa = ($type === 'object') ? get_class($v) : $type; + } else { + $aa = var_export($v, true); + } + $a .= sprintf(' %s,' . \PHP_EOL, $aa); + } + $a = rtrim($a, ',' . \PHP_EOL) . \PHP_EOL; + $o .= "$a)" . \PHP_EOL; + } + + return $o; } } \ No newline at end of file diff --git a/lib/Catcher/PlainTextHandler.php b/lib/Catcher/PlainTextHandler.php index 2b7bddb..a9bbaa5 100644 --- a/lib/Catcher/PlainTextHandler.php +++ b/lib/Catcher/PlainTextHandler.php @@ -49,7 +49,7 @@ class PlainTextHandler extends Handler { $outputThrowable['line'] ); - if (isset($outputThrowable['previous']) && $outputThrowable['previous'] instanceof \Throwable) { + if (isset($outputThrowable['previous']) && is_array($outputThrowable['previous'])) { if ($previous) { $output .= ' '; } @@ -80,7 +80,7 @@ class PlainTextHandler extends Handler { ); if (isset($frame['args']) && $this->_backtraceArgFrameLimit > $key) { - $output .= preg_replace('/^/m', "$indent| ", $this->varExporter($frame['args'])) . \PHP_EOL; + $output .= preg_replace('/^/m', "$indent| ", $this->serializeArgs($frame['args'])) . \PHP_EOL; } } diff --git a/lib/Catcher/ThrowableController.php b/lib/Catcher/ThrowableController.php index 447cb1c..0341ede 100644 --- a/lib/Catcher/ThrowableController.php +++ b/lib/Catcher/ThrowableController.php @@ -151,25 +151,25 @@ class ThrowableController { } // Add a frame for the throwable to the beginning of the array - // $f = [ - // 'file' => $this->throwable->getFile() ?: '[UNKNOWN]', - // 'line' => (int)$this->throwable->getLine(), - // 'class' => $this->throwable::class, - // 'args' => [ - // $this->throwable->getMessage() - // ] - // ]; - - // // Add the error code and type if it is an Error. - // if ($this->throwable instanceof \Error) { - // $errorType = $this->getErrorType(); - // if ($errorType !== null) { - // $f['code'] = $this->throwable->getCode(); - // $f['errorType'] = $errorType; - // } - // } - - // array_unshift($frames, $f); + $f = [ + 'file' => $this->throwable->getFile() ?: '[UNKNOWN]', + 'line' => (int)$this->throwable->getLine(), + 'class' => $this->throwable::class, + 'args' => [ + $this->throwable->getMessage() + ] + ]; + + // Add the error code and type if it is an Error. + if ($this->throwable instanceof \Error) { + $errorType = $this->getErrorType(); + if ($errorType !== null) { + $f['code'] = $this->throwable->getCode(); + $f['errorType'] = $errorType; + } + } + + array_unshift($frames, $f); // Go through previous throwables and merge in their frames if ($prev = $this->getPrevious()) { diff --git a/tests/cases/TestHandler.php b/tests/cases/TestHandler.php index c1b03f4..7c5b1db 100644 --- a/tests/cases/TestHandler.php +++ b/tests/cases/TestHandler.php @@ -31,6 +31,21 @@ class TestHandler extends ErrorHandlingTestCase { ]); } + /** @dataProvider provideArgumentSerializationTests */ + public function testArgumentSerialization(mixed $arg): void { + // This looks silly because the argument is never used, but the handler will + // pick it up and print it anyway which is what we're testing here + $this->handler->setOption('print', true); + $this->handler->setOption('printJSON', false); + $this->handler->setOption('outputToStderr', false); + $this->handler->handle(new ThrowableController(new \Exception('Ook!'))); + $h = $this->handler; + ob_start(); + $h(); + $o = ob_get_clean(); + $this->assertNotEmpty($o); + } + /** @dataProvider provideHandlingTests */ public function testHandling(\Throwable $throwable, int $expectedCode, array $options = []): void { foreach ($options as $k => $v) { @@ -126,6 +141,22 @@ class TestHandler extends ErrorHandlingTestCase { } + public static function provideArgumentSerializationTests(): iterable { + $options = [ + [ fn() => true ], + [ new \stdClass() ], + [ new class{} ], + [ (object)[] ], + [ 'ook' ], + [ 42 ], + [ \M_PI ] + ]; + + foreach ($options as $o) { + yield $o; + } + } + public static function provideHandlingTests(): iterable { $options = [ [ new \Exception('Ook!'), Handler::BUBBLES | Handler::OUTPUT | Handler::EXIT, [ 'forceExit' => true ] ], @@ -209,8 +240,7 @@ class TestHandler extends ErrorHandlingTestCase { [ 'outputTime', false ], [ 'outputToStderr', false ], [ 'silent', true ], - [ 'timeFormat', 'Y-m-d' ], - [ 'varExporter', fn(mixed $value): string|bool => var_export($value, true) ] + [ 'timeFormat', 'Y-m-d' ] ]; foreach ($options as $o) { diff --git a/tests/lib/TestingHandler.php b/tests/lib/TestingHandler.php index b9a8849..158a750 100644 --- a/tests/lib/TestingHandler.php +++ b/tests/lib/TestingHandler.php @@ -44,7 +44,20 @@ class TestingHandler extends Handler { $o = $this->cleanOutputThrowable($o); if ($this->_print) { - $this->print(($this->_printJSON) ? json_encode($o, \JSON_THROW_ON_ERROR) : var_dump($o) ?? 'FAIL'); + if (!$this->_printJSON) { + $oo = ''; + foreach ($o['frames'] as $f) { + if (!isset($f['args'])) { + continue; + } + $oo .= $this->serializeArgs($f['args']) . "\n"; + } + $oo = rtrim($oo); + } else { + $oo = json_encode($o, \JSON_THROW_ON_ERROR); + } + + $this->print($oo); } $this->output[] = $o; diff --git a/tests/phpunit.xml b/tests/phpunit.xml index 92e62e7..fe39e7c 100644 --- a/tests/phpunit.xml +++ b/tests/phpunit.xml @@ -1,5 +1,5 @@ -./cases - + + ../lib - +