From 88b9298110ab1131095e623bf69dec31709d74f2 Mon Sep 17 00:00:00 2001 From: Thomas Flori Date: Mon, 3 Dec 2018 08:24:44 +0100 Subject: [PATCH 1/2] add previous exception messages to plain text output Can be disabled via `PlainTextHandler::addPreviousToOutput(false)` and adds lines between the exception and the trace containing the exception and line of creator. The method `Inspector::getFrames()` already appends all frames from the previous exceptions. So there is no need in adding the trace for each previous exception. IMO the test tests something that it should not test: the output of the line number. But that should be in a separate PR. I've at least added constants for the line numbers so that you only have to change them once for all tests. --- src/Whoops/Handler/PlainTextHandler.php | 56 +++++++++++-- tests/Whoops/Handler/PlainTextHandlerTest.php | 81 ++++++++++++++++--- 2 files changed, 119 insertions(+), 18 deletions(-) diff --git a/src/Whoops/Handler/PlainTextHandler.php b/src/Whoops/Handler/PlainTextHandler.php index 2f5be906..bd9105ee 100644 --- a/src/Whoops/Handler/PlainTextHandler.php +++ b/src/Whoops/Handler/PlainTextHandler.php @@ -46,6 +46,11 @@ class PlainTextHandler extends Handler */ private $traceFunctionArgsOutputLimit = 1024; + /** + * @var bool + */ + private $addPreviousToOutput = true; + /** * @var bool */ @@ -114,6 +119,21 @@ public function addTraceToOutput($addTraceToOutput = null) return $this; } + /** + * Add previous exceptions to output. + * @param bool|null $addPreviousToOutput + * @return bool|$this + */ + public function addPreviousToOutput($addPreviousToOutput = null) + { + if (func_num_args() == 0) { + return $this->addPreviousToOutput; + } + + $this->addPreviousToOutput = (bool) $addPreviousToOutput; + return $this; + } + /** * Add error trace function arguments to output. * Set to True for all frame args, or integer for the n first frame args. @@ -151,14 +171,18 @@ public function setTraceFunctionArgsOutputLimit($traceFunctionArgsOutputLimit) public function generateResponse() { $exception = $this->getException(); - return sprintf( - "%s: %s in file %s on line %d%s\n", - get_class($exception), - $exception->getMessage(), - $exception->getFile(), - $exception->getLine(), - $this->getTraceOutput() - ); + $message = $this->getExceptionOutput($exception); + + if ($this->addPreviousToOutput) { + $previous = $exception->getPrevious(); + while ($previous) { + $message .= "\nBefore: " . $this->getExceptionOutput($previous); + $previous = $previous->getPrevious(); + } + } + + + return $message . $this->getTraceOutput() . "\n"; } /** @@ -284,6 +308,22 @@ private function getTraceOutput() return $response; } + /** + * Get the exception as plain text. + * @param \Throwable $exception + * @return string + */ + protected function getExceptionOutput($exception) + { + return sprintf( + "%s: %s in file %s on line %d", + get_class($exception), + $exception->getMessage(), + $exception->getFile(), + $exception->getLine() + ); + } + /** * @return int */ diff --git a/tests/Whoops/Handler/PlainTextHandlerTest.php b/tests/Whoops/Handler/PlainTextHandlerTest.php index 0b6b21d4..bda09576 100644 --- a/tests/Whoops/Handler/PlainTextHandlerTest.php +++ b/tests/Whoops/Handler/PlainTextHandlerTest.php @@ -13,6 +13,9 @@ class PlainTextHandlerTest extends TestCase { + const DEFAULT_EXCEPTION_LINE = 34; + const DEFAULT_LINE_OF_CALLER = 65; + /** * @throws \InvalidArgumentException If argument is not null or a LoggerInterface * @param \Psr\Log\LoggerInterface|null $logger @@ -32,28 +35,34 @@ public function getException($message = 'test message') } /** - * @param bool $withTrace - * @param bool $withTraceArgs - * @param bool $loggerOnly + * @param bool $withTrace + * @param bool $withTraceArgs + * @param int $traceFunctionArgsOutputLimit + * @param bool $loggerOnly + * @param bool $previousOutput + * @param null $exception * @return array */ private function getPlainTextFromHandler( $withTrace = false, $withTraceArgs = false, $traceFunctionArgsOutputLimit = 1024, - $loggerOnly = false + $loggerOnly = false, + $previousOutput = false, + $exception = null ) { $handler = $this->getHandler(); $handler->addTraceToOutput($withTrace); $handler->addTraceFunctionArgsToOutput($withTraceArgs); $handler->setTraceFunctionArgsOutputLimit($traceFunctionArgsOutputLimit); + $handler->addPreviousToOutput($previousOutput); $handler->loggerOnly($loggerOnly); $run = $this->getRunInstance(); $run->pushHandler($handler); $run->register(); - $exception = $this->getException(); + $exception = $exception ?: $this->getException(); ob_start(); $run->handleException($exception); @@ -209,7 +218,59 @@ public function testReturnsWithoutFramesOutput() get_class($this->getException()), 'test message', __FILE__, - 31 + self::DEFAULT_EXCEPTION_LINE + ), + $text + ); + } + + public function testReturnsWithoutPreviousExceptions() + { + $text = $this->getPlainTextFromHandler( + $withTrace = false, + $withTraceArgs = true, + $traceFunctionArgsOutputLimit = 1024, + $loggerOnly = false, + $previousOutput = false, + new RuntimeException('Outer exception message', 0, new RuntimeException('Inner exception message')) + ); + + // Check that the response does not contain Inner exception message: + $this->assertNotContains( + sprintf( + "%s: %s in file %s", + RuntimeException::class, + 'Inner exception message', + __FILE__ + ), + $text + ); + } + + public function testReturnsWithPreviousExceptions() + { + $text = $this->getPlainTextFromHandler( + $withTrace = false, + $withTraceArgs = true, + $traceFunctionArgsOutputLimit = 1024, + $loggerOnly = false, + $previousOutput = true, + new RuntimeException('Outer exception message', 0, new RuntimeException('Inner exception message')) + ); + + // Check that the response has the correct message: + $this->assertEquals( + sprintf( + "%s: %s in file %s on line %d\n" . + "%s: %s in file %s on line %d\n", + RuntimeException::class, + 'Outer exception message', + __FILE__, + 258, + 'Before: ' . RuntimeException::class, + 'Inner exception message', + __FILE__, + 258 ), $text ); @@ -238,10 +299,10 @@ public function testReturnsWithFramesOutput() sprintf( '%3d. %s->%s() %s:%d', 2, - 'Whoops\Handler\PlainTextHandlerTest', + __CLASS__, 'getException', __FILE__, - 56 + self::DEFAULT_LINE_OF_CALLER ), $text ); @@ -280,7 +341,7 @@ public function testReturnsWithFramesAndArgsOutput() 'Whoops\Handler\PlainTextHandlerTest', 'getException', __FILE__, - 56 + self::DEFAULT_LINE_OF_CALLER ), $text ); @@ -322,7 +383,7 @@ public function testReturnsWithFramesAndLimitedArgsOutput() 'Whoops\Handler\PlainTextHandlerTest', 'getException', __FILE__, - 56 + self::DEFAULT_LINE_OF_CALLER ), $text ); From 54bf2dfdd0c1ca0b0871db106ad3f396a15c0edf Mon Sep 17 00:00:00 2001 From: Thomas Flori Date: Mon, 3 Dec 2018 10:18:07 +0100 Subject: [PATCH 2/2] suggested changes for previous output in plaintext handler --- src/Whoops/Handler/PlainTextHandler.php | 4 ++-- tests/Whoops/Handler/PlainTextHandlerTest.php | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Whoops/Handler/PlainTextHandler.php b/src/Whoops/Handler/PlainTextHandler.php index bd9105ee..192d0ffd 100644 --- a/src/Whoops/Handler/PlainTextHandler.php +++ b/src/Whoops/Handler/PlainTextHandler.php @@ -176,7 +176,7 @@ public function generateResponse() if ($this->addPreviousToOutput) { $previous = $exception->getPrevious(); while ($previous) { - $message .= "\nBefore: " . $this->getExceptionOutput($previous); + $message .= "\n\nCaused by\n" . $this->getExceptionOutput($previous); $previous = $previous->getPrevious(); } } @@ -313,7 +313,7 @@ private function getTraceOutput() * @param \Throwable $exception * @return string */ - protected function getExceptionOutput($exception) + private function getExceptionOutput($exception) { return sprintf( "%s: %s in file %s on line %d", diff --git a/tests/Whoops/Handler/PlainTextHandlerTest.php b/tests/Whoops/Handler/PlainTextHandlerTest.php index bda09576..4b92fcc1 100644 --- a/tests/Whoops/Handler/PlainTextHandlerTest.php +++ b/tests/Whoops/Handler/PlainTextHandlerTest.php @@ -267,7 +267,7 @@ public function testReturnsWithPreviousExceptions() 'Outer exception message', __FILE__, 258, - 'Before: ' . RuntimeException::class, + "\nCaused by\n" . RuntimeException::class, 'Inner exception message', __FILE__, 258