refactor: clean up and test exception logging

Author: innocenzi

docs/2-features/14-exception-handling.md

@@ -37,7 +37,7 @@ final readonly class CreateUser
 
 ## Disabling exception logging
 
-The default logging reporter, {b`Tempest\Core\Exceptions\ExceptionReporter`}, is automatically added to the list of reporters. To disable it, create an {b`Tempest\Core\Exceptions\ExceptionsConfig`} [configuration file](../1-essentials/06-configuration.md#configuration-files) and set `logging` to `false`:
+The default logging reporter, {b`Tempest\Core\Exceptions\LoggingExceptionReporter`}, is automatically added to the list of reporters. To disable it, create a {b`Tempest\Core\Exceptions\ExceptionsConfig`} [configuration file](../1-essentials/06-configuration.md#configuration-files) and set `logging` to `false`:
 
 ```php app/exceptions.config.php
 use Tempest\Core\Exceptions\ExceptionsConfig;

packages/core/src/Exceptions/LoggingExceptionReporter.php

@@ -3,7 +3,6 @@
 namespace Tempest\Core\Exceptions;
 
 use Tempest\Core\ProvidesContext;
-use Tempest\Debug\Debug;
 use Tempest\Log\Logger;
 use Throwable;
 
@@ -18,17 +17,13 @@ public function __construct(
 
     public function report(Throwable $throwable): void
     {
-        $items = [
-            'class' => $throwable::class,
-            'exception' => $throwable->getMessage(),
-            'trace' => $throwable->getTraceAsString(),
-            'context' => $throwable instanceof ProvidesContext
+        $this->logger->error(
+            message: $throwable->getMessage() ?: '(no message)',
+            context: $throwable instanceof ProvidesContext
                 ? $throwable->context()
                 : [],
-        ];
+        );
 
-        $this->logger->error($throwable->getMessage(), $items);
-
-        Debug::resolve()->log($items, writeToOut: false);
+        $this->logger->error($throwable->getTraceAsString());
     }
 }

packages/http/src/HttpRequestFailed.php

@@ -34,7 +34,6 @@ public function context(): array
             'request_uri' => $this->request?->uri,
             'request_method' => $this->request?->method->value,
             'status_code' => $this->status->value,
-            'message' => $this->message !== '' ? $this->message : null,
             'original_response' => $this->cause ? $this->cause::class : null,
         ]);
     }

tests/Integration/Core/Exceptions/LoggingExceptionReporterTest.php

@@ -0,0 +1,132 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Tests\Tempest\Integration\Core\Exceptions;
+
+use Exception;
+use PHPUnit\Framework\Attributes\Test;
+use Tempest\Core\Exceptions\LoggingExceptionReporter;
+use Tempest\Core\ProvidesContext;
+use Tempest\Log\Logger;
+use Tests\Tempest\Integration\FrameworkIntegrationTestCase;
+
+final class LoggingExceptionReporterTest extends FrameworkIntegrationTestCase
+{
+    #[Test]
+    public function logs_exception_with_message(): void
+    {
+        $logger = new TestLogger();
+        $reporter = new LoggingExceptionReporter($logger);
+        $reporter->report(new Exception('Something went wrong'));
+
+        $this->assertCount(2, $logger->logs);
+        $this->assertSame('error', $logger->logs[0]['level']);
+        $this->assertSame('Something went wrong', $logger->logs[0]['message']);
+    }
+
+    #[Test]
+    public function logs_exception_with_fallback_message(): void
+    {
+        $logger = new TestLogger();
+        $reporter = new LoggingExceptionReporter($logger);
+        $reporter->report(new Exception(''));
+
+        $this->assertCount(2, $logger->logs);
+        $this->assertSame('error', $logger->logs[0]['level']);
+        $this->assertSame('(no message)', $logger->logs[0]['message']);
+    }
+
+    #[Test]
+    public function logs_exception_trace_separately(): void
+    {
+        $logger = new TestLogger();
+        $reporter = new LoggingExceptionReporter($logger);
+        $reporter->report(new Exception('Test'));
+
+        $this->assertCount(2, $logger->logs);
+        $this->assertSame('error', $logger->logs[1]['level']);
+        $this->assertIsString($logger->logs[1]['message']);
+        $this->assertStringStartsWith('#0 ', $logger->logs[1]['message']);
+    }
+
+    #[Test]
+    public function logs_exception_with_context_when_exception_provides_context(): void
+    {
+        $logger = new TestLogger();
+        $reporter = new LoggingExceptionReporter($logger);
+        $reporter->report(new ExceptionWithContext('Test'));
+
+        $this->assertCount(2, $logger->logs);
+        $this->assertSame(['foo' => 'bar', 'baz' => 'qux'], $logger->logs[0]['context']);
+    }
+
+    #[Test]
+    public function logs_exception_with_empty_context_when_exception_does_not_provide_context(): void
+    {
+        $logger = new TestLogger();
+        $reporter = new LoggingExceptionReporter($logger);
+        $reporter->report(new Exception('Test'));
+
+        $this->assertCount(2, $logger->logs);
+        $this->assertEmpty($logger->logs[0]['context']);
+    }
+}
+
+final class TestLogger implements Logger
+{
+    public array $logs = [];
+
+    public function emergency(string|\Stringable $message, array $context = []): void
+    {
+        $this->logs[] = ['level' => 'emergency', 'message' => $message, 'context' => $context];
+    }
+
+    public function alert(string|\Stringable $message, array $context = []): void
+    {
+        $this->logs[] = ['level' => 'alert', 'message' => $message, 'context' => $context];
+    }
+
+    public function critical(string|\Stringable $message, array $context = []): void
+    {
+        $this->logs[] = ['level' => 'critical', 'message' => $message, 'context' => $context];
+    }
+
+    public function error(string|\Stringable $message, array $context = []): void
+    {
+        $this->logs[] = ['level' => 'error', 'message' => $message, 'context' => $context];
+    }
+
+    public function warning(string|\Stringable $message, array $context = []): void
+    {
+        $this->logs[] = ['level' => 'warning', 'message' => $message, 'context' => $context];
+    }
+
+    public function notice(string|\Stringable $message, array $context = []): void
+    {
+        $this->logs[] = ['level' => 'notice', 'message' => $message, 'context' => $context];
+    }
+
+    public function info(string|\Stringable $message, array $context = []): void
+    {
+        $this->logs[] = ['level' => 'info', 'message' => $message, 'context' => $context];
+    }
+
+    public function debug(string|\Stringable $message, array $context = []): void
+    {
+        $this->logs[] = ['level' => 'debug', 'message' => $message, 'context' => $context];
+    }
+
+    public function log(mixed $level, string|\Stringable $message, array $context = []): void
+    {
+        $this->logs[] = ['level' => $level, 'message' => $message, 'context' => $context];
+    }
+}
+
+final class ExceptionWithContext extends Exception implements ProvidesContext
+{
+    public function context(): iterable
+    {
+        return ['foo' => 'bar', 'baz' => 'qux'];
+    }
+}