From c297f628fd40b60affa122d1ee8d2429383a03d5 Mon Sep 17 00:00:00 2001 From: Woody Gilk Date: Fri, 10 Apr 2026 06:33:54 -0500 Subject: [PATCH] chore!: Refactor exception handling Remove the custom LogException wrapper. Add LogFormatter/JsonFormatter for formatting. --- CHANGELOG.md | 4 +++ README.md | 48 ++++++++++++--------------- captainhook.json | 14 -------- src/Console/LogCommandError.php | 5 +-- src/Http/LogRequestError.php | 3 +- src/Misc/ExceptionLog.php | 31 ----------------- src/Misc/ExceptionSource.php | 13 -------- src/Writer/FileWriter.php | 14 +++----- src/Writer/JsonFormatter.php | 21 ++++++++++++ src/Writer/LogFormatter.php | 12 +++++++ tests/Console/LogCommandErrorTest.php | 8 +++-- tests/Http/LogRequestErrorTest.php | 12 ++++--- tests/Misc/ExceptionLogTest.php | 32 ------------------ tests/Writer/FileWriterTest.php | 33 +++++------------- tests/Writer/JsonFormatterTest.php | 46 +++++++++++++++++++++++++ 15 files changed, 135 insertions(+), 161 deletions(-) delete mode 100644 src/Misc/ExceptionLog.php delete mode 100644 src/Misc/ExceptionSource.php create mode 100644 src/Writer/JsonFormatter.php create mode 100644 src/Writer/LogFormatter.php delete mode 100644 tests/Misc/ExceptionLogTest.php create mode 100644 tests/Writer/JsonFormatterTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index d278030..8ea90e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,12 +9,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- ⚠️ BREAKING! `ExceptionLog` has been removed +- ⚠️ BREAKING! `LogCommandError` and `LogRequestError` now log the exception type and message to `error`, + instead of logging the exception object to `exception` - Switched to [Mago](https://mago.carthage.software/) for formatting and analysis ### Added - Added a `Log::append()` method to allow attaching multiple entries, without using `LogList` - Added `ToggledWriter` to enable or disable log writing at runtime +- Added `LogFormatter` and `JsonFormatter` to allow customization of log lines ## [0.4.0] diff --git a/README.md b/README.md index fc2f789..61f54d1 100644 --- a/README.md +++ b/README.md @@ -135,23 +135,12 @@ The `LogList` maintains insertion order and will encode as a JSON array: ## Exception Logging -Knotlog provides an `ExceptionLog` class to capture exception (`Throwable`) context. - -```php -use Knotlog\Misc\ExceptionLog; - -try { - // Some code that may throw -} catch (Throwable) { - $log->add('exception', ExceptionLog::fromThrowable($throwable)); -} -``` - -⚠️ _The exception log includes the stack trace, which may be very large and include sensitive information. -It is recommended to set `zend.exception_ignore_args = On` in your `php.ini` to minimize the stack trace._ +It is **not** recommended to add exception objects to Knotlog. Exception tracing is better handled by other systems, +such as [Monolog](https://seldaek.github.io/monolog/) or another [PSR-3](https://www.php-fig.org/psr/psr-3/) logger. For convenience, `Log` provides a `hasError()` method that will return true if either the `exception` or `error` -keys are set. This is particularly useful for determining if the log should be output when sampling is enabled. +keys are set in the log. This is particularly useful for determining if the log should be output when sampling +is enabled. ## HTTP Middleware @@ -174,7 +163,7 @@ use Knotlog\Http\ServerErrorResponseFactory; // The default factory creates a minimal 500 response $errorFactory = new ServerErrorResponseFactory($responseFactory, $streamFactory); -// Logs uncaught exceptions and outputs an error response +// Logs uncaught exception type and message, and outputs an error response $stack->add(new LogRequestError($errorFactory, $log)); ``` @@ -210,7 +199,7 @@ use Symfony\Component\Console\ConsoleEvents; // Logs command metadata on execution $eventDispatcher->addListener(ConsoleEvents::COMMAND, new LogCommandEvent($log)); -// Logs command error context on failure +// Logs uncaught exception type and message $eventDispatcher->addListener(ConsoleEvents::ERROR, new LogCommandError($log)); ``` @@ -226,11 +215,11 @@ $log->set('console', LogCommand::fromCommand($command, $input)); ## Log Writers Knotlog provides a `LogWriter` interface to enable flexible output destinations for wide log events. -The interface defines a simple contract with a single method `write(Log $log): void`. +The interface defines a contract with a single method `write(Log $log): void`. ### FileWriter -The `FileWriter` writes log events as JSON-encoded lines to a file or stream. +The `FileWriter` writes log events to a file. ```php use Knotlog\Writer\FileWriter; @@ -241,18 +230,10 @@ $writer = new FileWriter(); // Write to a specific file $writer = new FileWriter('/var/log/app.log'); -// Use custom JSON encoding flags -$writer = new FileWriter('php://stdout', JSON_PRETTY_PRINT); - // Write the log event $writer->write($log); ``` -Each log line is prefixed with a status indicator: - -- `ERROR` - when the log contains an error or exception -- `INFO` - for all other log events - ### LoggerWriter The `LoggerWriter` outputs log events to any [PSR-3](https://www.php-fig.org/psr/psr-3/) compatible logger @@ -333,6 +314,19 @@ $writer->write($log); This is useful for conditionally disabling log output based on runtime configuration, such as feature flags or environment-specific settings. +## Log Formatters + +Knotlog provides a `LogFormatter` interface to enable flexible formatting of log lines. +The interface defines a contract with a single method `format(Log $log): string`. + +### JsonFormatter + +The `JsonFormatter` formats log events as JSON strings. Additional flags may be passed through to `json_encode`: + +```php +$formatter = new JsonFormatter(JSON_PRETTY_PRINT); +``` + ## License MIT License, see `LICENSE` file for details. diff --git a/captainhook.json b/captainhook.json index 25dd7e6..e9093c3 100644 --- a/captainhook.json +++ b/captainhook.json @@ -39,19 +39,5 @@ ] } ] - }, - "commit-msg": { - "enabled": true, - "actions": [ - { - "action": "\\CaptainHook\\App\\Hook\\Message\\Action\\Beams", - "options": { - "subjectLength": 50, - "bodyLineLength": 72, - "checkImperativeBeginningOnly": true - }, - "conditions": [] - } - ] } } diff --git a/src/Console/LogCommandError.php b/src/Console/LogCommandError.php index 85e80af..eaf1f35 100644 --- a/src/Console/LogCommandError.php +++ b/src/Console/LogCommandError.php @@ -5,7 +5,6 @@ namespace Knotlog\Console; use Knotlog\Log; -use Knotlog\Misc\ExceptionLog; use Symfony\Component\Console\Event\ConsoleErrorEvent; final readonly class LogCommandError @@ -16,6 +15,8 @@ public function __construct( public function __invoke(ConsoleErrorEvent $event): void { - $this->log->set('exception', ExceptionLog::fromThrowable($event->getError())); + $throwable = $event->getError(); + + $this->log->set('error', sprintf('%s %s', $throwable::class, $throwable->getMessage())); } } diff --git a/src/Http/LogRequestError.php b/src/Http/LogRequestError.php index 285947e..1ba8963 100644 --- a/src/Http/LogRequestError.php +++ b/src/Http/LogRequestError.php @@ -5,7 +5,6 @@ namespace Knotlog\Http; use Knotlog\Log; -use Knotlog\Misc\ExceptionLog; use Override; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; @@ -29,7 +28,7 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface try { return $handler->handle($request); } catch (Throwable $throwable) { - $this->log->set('exception', ExceptionLog::fromThrowable($throwable)); + $this->log->set('error', sprintf('%s %s', $throwable::class, $throwable->getMessage())); return $this->errorResponseFactory->createErrorResponse($throwable); } diff --git a/src/Misc/ExceptionLog.php b/src/Misc/ExceptionLog.php deleted file mode 100644 index 99e7a8f..0000000 --- a/src/Misc/ExceptionLog.php +++ /dev/null @@ -1,31 +0,0 @@ -getMessage(), - code: $exception->getCode(), - source: new ExceptionSource($exception->getFile(), $exception->getLine()), - // @mago-ignore analysis:possibly-invalid-argument - trace: $exception->getTrace(), - ); - } - - public function __construct( - public string $type, - public string $message, - public int|string $code, - public ExceptionSource $source, - /** @var list> */ - public array $trace = [], - ) {} -} diff --git a/src/Misc/ExceptionSource.php b/src/Misc/ExceptionSource.php deleted file mode 100644 index f4fa421..0000000 --- a/src/Misc/ExceptionSource.php +++ /dev/null @@ -1,13 +0,0 @@ -hasError() ? 'ERROR' : 'INFO'; - - $line = json_encode($log, JSON_THROW_ON_ERROR | $this->flags); + $line = $this->formatter->format($log); // @mago-ignore analysis:unhandled-thrown-type - $this->filesystem->appendToFile($this->path, "{$status} {$line}\n"); + $this->filesystem->appendToFile($this->path, "{$line}\n"); } } diff --git a/src/Writer/JsonFormatter.php b/src/Writer/JsonFormatter.php new file mode 100644 index 0000000..4a78651 --- /dev/null +++ b/src/Writer/JsonFormatter.php @@ -0,0 +1,21 @@ +flags); + } +} diff --git a/src/Writer/LogFormatter.php b/src/Writer/LogFormatter.php new file mode 100644 index 0000000..173799d --- /dev/null +++ b/src/Writer/LogFormatter.php @@ -0,0 +1,12 @@ +assertTrue($log->hasError()); - $this->assertInstanceOf(ExceptionLog::class, $log->all()['exception']); + + $msg = $log->all()['error']; + + $this->assertTrue(is_string($msg)); + $this->assertStringContainsString($exception::class, $msg); + $this->assertStringContainsString($exception->getMessage(), $msg); } } diff --git a/tests/Http/LogRequestErrorTest.php b/tests/Http/LogRequestErrorTest.php index 9cca925..2060a14 100644 --- a/tests/Http/LogRequestErrorTest.php +++ b/tests/Http/LogRequestErrorTest.php @@ -4,11 +4,9 @@ namespace Knotlog\Tests\Http; -use Exception; use Knotlog\Http\ErrorResponseFactory; use Knotlog\Http\LogRequestError; use Knotlog\Log; -use Knotlog\Misc\ExceptionLog; use Nyholm\Psr7\Response; use Nyholm\Psr7\ServerRequest; use Override; @@ -18,6 +16,7 @@ use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\RequestHandlerInterface; +use RuntimeException; use Throwable; #[CoversClass(LogRequestError::class)] @@ -49,14 +48,19 @@ public function it_catches_exception_from_handler(): void $middleware = new LogRequestError($errorFactory, $log); $request = new ServerRequest('GET', 'https://api.example.com/error'); - $exception = new Exception('Something went wrong'); + $exception = new RuntimeException('Something went wrong'); $handler = $this->createThrowingHandler($exception); $middleware->process($request, $handler); $this->assertTrue($log->hasError()); - $this->assertInstanceOf(ExceptionLog::class, $log->all()['exception']); + + $msg = $log->all()['error']; + + $this->assertTrue(is_string($msg)); + $this->assertStringContainsString($exception::class, $msg); + $this->assertStringContainsString($exception->getMessage(), $msg); } private function createMockErrorFactory(): ErrorResponseFactory diff --git a/tests/Misc/ExceptionLogTest.php b/tests/Misc/ExceptionLogTest.php deleted file mode 100644 index 7793581..0000000 --- a/tests/Misc/ExceptionLogTest.php +++ /dev/null @@ -1,32 +0,0 @@ -assertSame($exception::class, $log->type); - $this->assertSame($exception->getMessage(), $log->message); - $this->assertSame($exception->getCode(), $log->code); - $this->assertSame($exception->getFile(), $log->source->file); - $this->assertSame($exception->getLine(), $log->source->line); - $this->assertSame($exception->getTrace(), $log->trace); - } -} diff --git a/tests/Writer/FileWriterTest.php b/tests/Writer/FileWriterTest.php index eec249b..6c66f58 100644 --- a/tests/Writer/FileWriterTest.php +++ b/tests/Writer/FileWriterTest.php @@ -18,7 +18,6 @@ use function sys_get_temp_dir; use function trim; -use const JSON_PRETTY_PRINT; use const JSON_THROW_ON_ERROR; #[CoversClass(FileWriter::class)] @@ -49,7 +48,7 @@ protected function deleteTemporaryFile(): void #[Test] public function it_writes_info_status_for_non_error_logs(): void { - $writer = new FileWriter($this->tempFile); + $writer = new FileWriter(path: $this->tempFile); $log = new Log(); $log->set('message', 'test'); @@ -59,13 +58,14 @@ public function it_writes_info_status_for_non_error_logs(): void $content = new Filesystem()->readFile($this->tempFile); $expectedJson = json_encode($log, JSON_THROW_ON_ERROR); - $this->assertStringContainsString("INFO {$expectedJson}", $content); + + $this->assertStringContainsString($expectedJson, $content); } #[Test] public function it_writes_error_status_for_error_logs(): void { - $writer = new FileWriter($this->tempFile); + $writer = new FileWriter(path: $this->tempFile); $log = new Log(); $log->set('error', 'Something went wrong'); @@ -75,13 +75,13 @@ public function it_writes_error_status_for_error_logs(): void $content = new Filesystem()->readFile($this->tempFile); $expectedJson = json_encode($log, JSON_THROW_ON_ERROR); - $this->assertStringContainsString("ERROR {$expectedJson}", $content); + $this->assertStringContainsString($expectedJson, $content); } #[Test] public function it_writes_error_status_for_exception_logs(): void { - $writer = new FileWriter($this->tempFile); + $writer = new FileWriter(path: $this->tempFile); $log = new Log(); $log->set('exception', 'RuntimeException'); @@ -91,13 +91,13 @@ public function it_writes_error_status_for_exception_logs(): void $content = new Filesystem()->readFile($this->tempFile); $expectedJson = json_encode($log, JSON_THROW_ON_ERROR); - $this->assertStringContainsString("ERROR {$expectedJson}", $content); + $this->assertStringContainsString($expectedJson, $content); } #[Test] public function it_appends_to_existing_file(): void { - $writer = new FileWriter($this->tempFile); + $writer = new FileWriter(path: $this->tempFile); $log1 = new Log(); $log1->set('message', 'first'); @@ -113,25 +113,10 @@ public function it_appends_to_existing_file(): void $this->assertStringContainsString('second', $content); } - #[Test] - public function it_uses_custom_json_flags(): void - { - $writer = new FileWriter($this->tempFile, JSON_PRETTY_PRINT); - - $log = new Log(); - $log->set('message', 'test'); - - $writer->write($log); - - $content = new Filesystem()->readFile($this->tempFile); - - $this->assertStringContainsString("{\n", $content); - } - #[Test] public function it_writes_each_log_on_new_line(): void { - $writer = new FileWriter($this->tempFile); + $writer = new FileWriter(path: $this->tempFile); $log1 = new Log(); $log1->set('message', 'first'); diff --git a/tests/Writer/JsonFormatterTest.php b/tests/Writer/JsonFormatterTest.php new file mode 100644 index 0000000..b101c92 --- /dev/null +++ b/tests/Writer/JsonFormatterTest.php @@ -0,0 +1,46 @@ +set('message', 'test'); + + $expectedJson = json_encode($log, JSON_THROW_ON_ERROR); + + $this->assertSame($expectedJson, $formatter->format($log)); + } + + #[Test] + public function it_allows_custom_json_flags(): void + { + $formatter = new JsonFormatter(flags: JSON_PRETTY_PRINT); + + $log = new Log(); + $log->set('message', 'test'); + + $expectedJson = json_encode($log, JSON_THROW_ON_ERROR | JSON_PRETTY_PRINT); + + $this->assertSame($expectedJson, $formatter->format($log)); + } +}