diff --git a/CHANGELOG.md b/CHANGELOG.md index f55575d..4028f5c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Yii Error Handler Change Log +## 5.0 under development + +- Chg #121: Use `NullLogger` as the default logger in the `ErrorHandler` (@olegbaturin) +- Chg #144: Mark `ErrorException` and `UserException` as final (@olegbaturin) +- Chg #146: Remove deprecated `Yiisoft\ErrorHandler\Factory\ThrowableResponseFactory` (@olegbaturin) +- Chg #157: Change parameters order in the `ThrowableResponseActionInterface` (@olegbaturin) + ## 4.3.1 under development - no changes in this release. diff --git a/config/di-web.php b/config/di-web.php index b6bc5d6..241673c 100644 --- a/config/di-web.php +++ b/config/di-web.php @@ -2,9 +2,14 @@ declare(strict_types=1); -use Yiisoft\ErrorHandler\Factory\ThrowableResponseFactory; +use Psr\Container\ContainerInterface; +use Yiisoft\Definitions\DynamicReference; use Yiisoft\ErrorHandler\Renderer\HtmlRenderer; +use Yiisoft\ErrorHandler\RendererProvider\CompositeRendererProvider; +use Yiisoft\ErrorHandler\RendererProvider\ContentTypeRendererProvider; +use Yiisoft\ErrorHandler\RendererProvider\HeadRendererProvider; use Yiisoft\ErrorHandler\ThrowableRendererInterface; +use Yiisoft\ErrorHandler\ThrowableResponseFactory; use Yiisoft\ErrorHandler\ThrowableResponseFactoryInterface; /** @@ -14,4 +19,14 @@ return [ ThrowableRendererInterface::class => HtmlRenderer::class, ThrowableResponseFactoryInterface::class => ThrowableResponseFactory::class, + ThrowableResponseFactory::class => [ + '__construct()' => [ + 'rendererProvider' => DynamicReference::to( + static fn(ContainerInterface $container) => new CompositeRendererProvider( + new HeadRendererProvider(), + new ContentTypeRendererProvider($container), + ) + ), + ], + ], ]; diff --git a/src/ErrorHandler.php b/src/ErrorHandler.php index f9c4aa0..e862099 100644 --- a/src/ErrorHandler.php +++ b/src/ErrorHandler.php @@ -7,6 +7,7 @@ use Psr\EventDispatcher\EventDispatcherInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Log\LoggerInterface; +use Psr\Log\NullLogger; use Throwable; use Yiisoft\ErrorHandler\Event\ApplicationError; use Yiisoft\ErrorHandler\Exception\ErrorException; @@ -41,14 +42,14 @@ final class ErrorHandler private bool $initialized = false; /** - * @param LoggerInterface $logger Logger to write errors to. * @param ThrowableRendererInterface $defaultRenderer Default throwable renderer. + * @param LoggerInterface $logger Logger to write errors to. * @param EventDispatcherInterface|null $eventDispatcher Event dispatcher for error events. * @param int $exitShutdownHandlerDepth Depth of the exit() shutdown handler to ensure it's executed last. */ public function __construct( - private readonly LoggerInterface $logger, private readonly ThrowableRendererInterface $defaultRenderer, + private readonly LoggerInterface $logger = new NullLogger(), private readonly ?EventDispatcherInterface $eventDispatcher = null, private readonly int $exitShutdownHandlerDepth = 2 ) { diff --git a/src/Exception/ErrorException.php b/src/Exception/ErrorException.php index 5f61d77..efc0612 100644 --- a/src/Exception/ErrorException.php +++ b/src/Exception/ErrorException.php @@ -15,10 +15,8 @@ /** * `ErrorException` represents a PHP error. * @psalm-type DebugBacktraceType = list, class?: class-string, file?: string, function?: string, line?: int, object?: object, type?: string}> - * - * @final */ -class ErrorException extends \ErrorException implements FriendlyExceptionInterface +final class ErrorException extends \ErrorException implements FriendlyExceptionInterface { /** @psalm-suppress MissingClassConstType Private constants never change. */ private const ERROR_NAMES = [ diff --git a/src/Exception/UserException.php b/src/Exception/UserException.php index 8c78b1e..bd2f9fc 100644 --- a/src/Exception/UserException.php +++ b/src/Exception/UserException.php @@ -19,11 +19,9 @@ * - throw directly (`throw new UserException(...)`) for explicit user-facing errors; * - annotate any exception class with the `#[UserException]` attribute * to mark its messages as user-facing without extending this class. - * - * @final */ #[Attribute(Attribute::TARGET_CLASS)] -class UserException extends Exception +final class UserException extends Exception { public static function isUserException(Throwable $throwable): bool { diff --git a/src/Factory/ThrowableResponseFactory.php b/src/Factory/ThrowableResponseFactory.php deleted file mode 100644 index 6a16c1b..0000000 --- a/src/Factory/ThrowableResponseFactory.php +++ /dev/null @@ -1,190 +0,0 @@ -> - */ - private array $renderers = [ - 'application/json' => JsonRenderer::class, - 'application/xml' => XmlRenderer::class, - 'text/xml' => XmlRenderer::class, - 'text/plain' => PlainTextRenderer::class, - 'text/html' => HtmlRenderer::class, - '*/*' => HtmlRenderer::class, - ]; - private ?string $contentType = null; - - public function __construct( - private readonly ResponseFactoryInterface $responseFactory, - private readonly ErrorHandler $errorHandler, - private readonly ContainerInterface $container, - ?HeadersProvider $headersProvider = null, - ) { - $this->headersProvider = $headersProvider ?? new HeadersProvider(); - } - - public function create(Throwable $throwable, ServerRequestInterface $request): ResponseInterface - { - $contentType = $this->contentType ?? $this->getContentType($request); - $renderer = $request->getMethod() === Method::HEAD ? new HeaderRenderer() : $this->getRenderer($contentType); - - $data = $this->errorHandler->handle($throwable, $renderer, $request); - $response = $this->responseFactory->createResponse(Status::INTERNAL_SERVER_ERROR); - foreach ($this->headersProvider->getAll() as $name => $value) { - $response = $response->withHeader($name, $value); - } - return $data->addToResponse($response->withHeader(Header::CONTENT_TYPE, $contentType)); - } - - /** - * Returns a new instance with the specified content type and renderer class. - * - * @param string $contentType The content type to add associated renderers for. - * @param string $rendererClass The classname implementing the {@see ThrowableRendererInterface}. - */ - public function withRenderer(string $contentType, string $rendererClass): self - { - if (!is_subclass_of($rendererClass, ThrowableRendererInterface::class)) { - throw new InvalidArgumentException(sprintf( - 'Class "%s" does not implement "%s".', - $rendererClass, - ThrowableRendererInterface::class, - )); - } - - $new = clone $this; - $new->renderers[$this->normalizeContentType($contentType)] = $rendererClass; - return $new; - } - - /** - * Returns a new instance without renderers by the specified content types. - * - * @param string[] $contentTypes The content types to remove associated renderers for. - * If not specified, all renderers will be removed. - */ - public function withoutRenderers(string ...$contentTypes): self - { - $new = clone $this; - - if (count($contentTypes) === 0) { - $new->renderers = []; - return $new; - } - - foreach ($contentTypes as $contentType) { - unset($new->renderers[$this->normalizeContentType($contentType)]); - } - - return $new; - } - - /** - * Force content type to respond with regardless of request. - * - * @param string $contentType The content type to respond with regardless of request. - */ - public function forceContentType(string $contentType): self - { - $contentType = $this->normalizeContentType($contentType); - - if (!isset($this->renderers[$contentType])) { - throw new InvalidArgumentException(sprintf('The renderer for %s is not set.', $contentType)); - } - - $new = clone $this; - $new->contentType = $contentType; - return $new; - } - - /** - * Returns the renderer by the specified content type, or null if the renderer was not set. - * - * @param string $contentType The content type associated with the renderer. - */ - private function getRenderer(string $contentType): ?ThrowableRendererInterface - { - if (isset($this->renderers[$contentType])) { - /** @var ThrowableRendererInterface */ - return $this->container->get($this->renderers[$contentType]); - } - - return null; - } - - /** - * Returns the priority content type from the accept request header. - * - * @return string The priority content type. - */ - private function getContentType(ServerRequestInterface $request): string - { - try { - foreach (HeaderValueHelper::getSortedAcceptTypes($request->getHeader(Header::ACCEPT)) as $header) { - if (array_key_exists($header, $this->renderers)) { - return $header; - } - } - } catch (InvalidArgumentException) { - // The Accept header contains an invalid q factor. - } - - return '*/*'; - } - - /** - * Normalizes the content type. - * - * @param string $contentType The raw content type. - * - * @return string Normalized content type. - */ - private function normalizeContentType(string $contentType): string - { - if (!str_contains($contentType, '/')) { - throw new InvalidArgumentException('Invalid content type.'); - } - - return strtolower(trim($contentType)); - } -} diff --git a/src/Middleware/ErrorCatcher.php b/src/Middleware/ErrorCatcher.php index 097e3da..eab1bd9 100644 --- a/src/Middleware/ErrorCatcher.php +++ b/src/Middleware/ErrorCatcher.php @@ -37,7 +37,7 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface $t = new CompositeException($e, $t); } - return $this->throwableResponseFactory->create($t, $request); + return $this->throwableResponseFactory->create($request, $t); } } } diff --git a/src/ThrowableResponseFactory.php b/src/ThrowableResponseFactory.php index 7e69099..0b706e9 100644 --- a/src/ThrowableResponseFactory.php +++ b/src/ThrowableResponseFactory.php @@ -16,18 +16,15 @@ */ final class ThrowableResponseFactory implements ThrowableResponseFactoryInterface { - private readonly HeadersProvider $headersProvider; - public function __construct( private readonly ResponseFactoryInterface $responseFactory, private readonly ErrorHandler $errorHandler, private readonly RendererProviderInterface $rendererProvider, - ?HeadersProvider $headersProvider = null, + private readonly HeadersProvider $headersProvider = new HeadersProvider(), ) { - $this->headersProvider = $headersProvider ?? new HeadersProvider(); } - public function create(Throwable $throwable, ServerRequestInterface $request): ResponseInterface + public function create(ServerRequestInterface $request, Throwable $throwable): ResponseInterface { $renderer = $this->rendererProvider->get($request); diff --git a/src/ThrowableResponseFactoryInterface.php b/src/ThrowableResponseFactoryInterface.php index 1a16cb9..5daf425 100644 --- a/src/ThrowableResponseFactoryInterface.php +++ b/src/ThrowableResponseFactoryInterface.php @@ -16,5 +16,5 @@ interface ThrowableResponseFactoryInterface /** * Handles a `Throwable` object and produces a response. */ - public function create(Throwable $throwable, ServerRequestInterface $request): ResponseInterface; + public function create(ServerRequestInterface $request, Throwable $throwable): ResponseInterface; } diff --git a/tests/ConfigTest.php b/tests/ConfigTest.php index 838edf0..34aa468 100644 --- a/tests/ConfigTest.php +++ b/tests/ConfigTest.php @@ -4,11 +4,15 @@ namespace Yiisoft\ErrorHandler\Tests; +use HttpSoft\Message\ResponseFactory; use PHPUnit\Framework\TestCase; +use Psr\Http\Message\ResponseFactoryInterface; use Yiisoft\Di\Container; use Yiisoft\Di\ContainerConfig; use Yiisoft\ErrorHandler\Renderer\HtmlRenderer; use Yiisoft\ErrorHandler\ThrowableRendererInterface; +use Yiisoft\ErrorHandler\ThrowableResponseFactory; +use Yiisoft\ErrorHandler\ThrowableResponseFactoryInterface; final class ConfigTest extends TestCase { @@ -17,16 +21,19 @@ public function testDiWeb(): void $container = $this->createContainer('web'); $throwableRenderer = $container->get(ThrowableRendererInterface::class); - $this->assertInstanceOf(HtmlRenderer::class, $throwableRenderer); + + $throwableResponseFactory = $container->get(ThrowableResponseFactoryInterface::class); + $this->assertInstanceOf(ThrowableResponseFactory::class, $throwableResponseFactory); } private function createContainer(?string $postfix = null): Container { return new Container( - ContainerConfig::create()->withDefinitions( - $this->getDiConfig($postfix) - ) + ContainerConfig::create()->withDefinitions([ + ResponseFactoryInterface::class => ResponseFactory::class, + ...$this->getDiConfig($postfix), + ]) ); } diff --git a/tests/ErrorHandlerTest.php b/tests/ErrorHandlerTest.php index a315bb5..f4845de 100644 --- a/tests/ErrorHandlerTest.php +++ b/tests/ErrorHandlerTest.php @@ -23,12 +23,14 @@ protected function setUp(): void { $this->loggerMock = $this->createMock(LoggerInterface::class); $this->throwableRendererMock = $this->createMock(ThrowableRendererInterface::class); - $this->errorHandler = new ErrorHandler($this->loggerMock, $this->throwableRendererMock); + $this->errorHandler = new ErrorHandler($this->throwableRendererMock, $this->loggerMock); $this->errorHandler->memoryReserveSize(0); } - public function testHandleThrowableCallsDefaultRendererWhenNonePassed(): void + public function testHandleThrowableCallsDefaultRendererWithoutLogger(): void { + $errorHandler = new ErrorHandler($this->throwableRendererMock); + $errorHandler->memoryReserveSize(0); $throwable = new RuntimeException(); $this @@ -37,6 +39,18 @@ public function testHandleThrowableCallsDefaultRendererWhenNonePassed(): void ->method('render') ->with($throwable); + $errorHandler->handle($throwable); + } + + public function testHandleThrowableCallsDefaultRendererWhenNonePassed(): void + { + $throwable = new RuntimeException(); + + $this + ->throwableRendererMock + ->expects($this->once()) + ->method('render') + ->with($throwable); $this->errorHandler->handle($throwable); } diff --git a/tests/Factory/ThrowableResponseFactoryTest.php b/tests/Factory/ThrowableResponseFactoryTest.php deleted file mode 100644 index 0c9ae08..0000000 --- a/tests/Factory/ThrowableResponseFactoryTest.php +++ /dev/null @@ -1,258 +0,0 @@ -createThrowableResponseFactory() - ->create( - $this->createThrowable(), - $this->createServerRequest('HEAD', ['Accept' => ['test/html']]) - ); - $response->getBody()->rewind(); - $content = $response->getBody()->getContents(); - - $this->assertEmpty($content); - $this->assertSame([HeaderRenderer::DEFAULT_ERROR_MESSAGE], $response->getHeader('X-Error-Message')); - } - - public function testHandleWithFailAcceptRequestHeader(): void - { - $response = $this - ->createThrowableResponseFactory() - ->create( - $this->createThrowable(), - $this->createServerRequest('GET', ['Accept' => ['text/plain;q=2.0']]) - ); - $response - ->getBody() - ->rewind(); - $content = $response - ->getBody() - ->getContents(); - - $this->assertNotSame(PlainTextRenderer::DEFAULT_ERROR_MESSAGE, $content); - $this->assertStringContainsString('createThrowableResponseFactory() - ->withRenderer($mimeType, PlainTextRenderer::class); - $response = $factory->create( - $this->createThrowable(), - $this->createServerRequest('GET', ['Accept' => [$mimeType]]) - ); - $response - ->getBody() - ->rewind(); - $content = $response - ->getBody() - ->getContents(); - - $this->assertSame(PlainTextRenderer::DEFAULT_ERROR_MESSAGE, $content); - } - - public function testThrownExceptionWithRendererIsNotImplementThrowableRendererInterface() - { - $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage( - 'Class "' . self::class . '" does not implement "' . ThrowableRendererInterface::class . '".', - ); - $this - ->createThrowableResponseFactory() - ->withRenderer('test/test', self::class); - } - - public function testThrownExceptionWithInvalidContentType() - { - $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('Invalid content type.'); - $this - ->createThrowableResponseFactory() - ->withRenderer('test invalid content type', PlainTextRenderer::class); - } - - public function testWithoutRenderers(): void - { - $factory = $this - ->createThrowableResponseFactory() - ->withoutRenderers(); - $response = $factory->create( - $this->createThrowable(), - $this->createServerRequest('GET', ['Accept' => ['test/html']]) - ); - $response - ->getBody() - ->rewind(); - $content = $response - ->getBody() - ->getContents(); - - $this->assertSame(PlainTextRenderer::DEFAULT_ERROR_MESSAGE, $content); - } - - public function testWithoutRenderer(): void - { - $factory = $this - ->createThrowableResponseFactory() - ->withoutRenderers('*/*'); - $response = $factory->create( - $this->createThrowable(), - $this->createServerRequest('GET', ['Accept' => ['test/html']]) - ); - $response - ->getBody() - ->rewind(); - $content = $response - ->getBody() - ->getContents(); - - $this->assertSame(PlainTextRenderer::DEFAULT_ERROR_MESSAGE, $content); - } - - public function testAdvancedAcceptHeader(): void - { - $contentType = 'text/html;version=2'; - $factory = $this - ->createThrowableResponseFactory() - ->withRenderer($contentType, PlainTextRenderer::class); - $response = $factory->create( - $this->createThrowable(), - $this->createServerRequest('GET', ['Accept' => ['text/html', $contentType]]) - ); - $response - ->getBody() - ->rewind(); - $content = $response - ->getBody() - ->getContents(); - - $this->assertSame(PlainTextRenderer::DEFAULT_ERROR_MESSAGE, $content); - } - - public function testDefaultContentType(): void - { - $factory = $this - ->createThrowableResponseFactory() - ->withRenderer('*/*', PlainTextRenderer::class); - $response = $factory->create( - $this->createThrowable(), - $this->createServerRequest('GET', ['Accept' => ['test/test']]) - ); - $response - ->getBody() - ->rewind(); - $content = $response - ->getBody() - ->getContents(); - - $this->assertSame(PlainTextRenderer::DEFAULT_ERROR_MESSAGE, $content); - } - - public function testForceContentType(): void - { - $factory = $this - ->createThrowableResponseFactory() - ->forceContentType('application/json'); - $response = $factory->create( - $this->createThrowable(), - $this->createServerRequest('GET', ['Accept' => ['text/xml']]) - ); - $response - ->getBody() - ->rewind(); - - $this->assertSame('application/json', $response->getHeaderLine(Header::CONTENT_TYPE)); - } - - public function testForceContentTypeSetInvalidType(): void - { - $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('The renderer for image/gif is not set.'); - $this - ->createThrowableResponseFactory() - ->forceContentType('image/gif'); - } - - public function testAddedHeaders(): void - { - $provider = new HeadersProvider([ - 'X-Default' => 'default', - 'Content-Type' => 'incorrect', - ]); - $provider->add('X-Test', 'test'); - $provider->add('X-Test2', ['test2', 'test3']); - $factory = $this - ->createThrowableResponseFactory(provider: $provider) - ->withRenderer('*/*', PlainTextRenderer::class); - $response = $factory->create( - $this->createThrowable(), - $this->createServerRequest('GET', ['Accept' => ['test/test']]) - ); - $headers = $response->getHeaders(); - - $this->assertArrayHasKey('Content-Type', $headers); - $this->assertNotEquals('incorrect', $headers['Content-Type']); - - $this->assertArrayHasKey('X-Default', $headers); - $this->assertEquals(['default'], $headers['X-Default']); - $this->assertArrayHasKey('X-Test', $headers); - $this->assertEquals(['test'], $headers['X-Test']); - $this->assertArrayHasKey('X-Test2', $headers); - $this->assertEquals(['test2', 'test3'], $headers['X-Test2']); - } - - private function createThrowableResponseFactory( - ?HeadersProvider $provider = null, - ): ThrowableResponseFactoryInterface { - $container = new SimpleContainer([], fn (string $className): object => new $className()); - return new ThrowableResponseFactory( - new ResponseFactory(), - $this->createErrorHandler(), - $container, - $provider ?? new HeadersProvider() - ); - } - - private function createErrorHandler(): ErrorHandler - { - $logger = $this->createMock(LoggerInterface::class); - return new ErrorHandler($logger, new PlainTextRenderer()); - } - - private function createServerRequest(string $method, array $headers = []): ServerRequestInterface - { - return new ServerRequest([], [], [], [], [], $method, '/', $headers); - } - - private function createThrowable(): Throwable - { - return new RuntimeException(); - } -} diff --git a/tests/Middleware/ErrorCatcherTest.php b/tests/Middleware/ErrorCatcherTest.php index e274aa8..2b46a31 100644 --- a/tests/Middleware/ErrorCatcherTest.php +++ b/tests/Middleware/ErrorCatcherTest.php @@ -69,7 +69,7 @@ public function testErrorWithEventDispatcher(): void private function createThrowableResponseFactory(): ThrowableResponseFactoryInterface { return new class () implements ThrowableResponseFactoryInterface { - public function create(Throwable $throwable, ServerRequestInterface $request): ResponseInterface + public function create(ServerRequestInterface $request, Throwable $throwable): ResponseInterface { return new Response(Status::INTERNAL_SERVER_ERROR); } diff --git a/tests/ThrowableResponseFactoryTest.php b/tests/ThrowableResponseFactoryTest.php index ed5d045..00e5275 100644 --- a/tests/ThrowableResponseFactoryTest.php +++ b/tests/ThrowableResponseFactoryTest.php @@ -4,10 +4,9 @@ namespace Yiisoft\ErrorHandler\Tests; -use HttpSoft\Message\ResponseFactory; use LogicException; +use HttpSoft\Message\ResponseFactory; use PHPUnit\Framework\TestCase; -use Psr\Log\NullLogger; use Yiisoft\ErrorHandler\ErrorHandler; use Yiisoft\ErrorHandler\HeadersProvider; use Yiisoft\ErrorHandler\Renderer\PlainTextRenderer; @@ -27,7 +26,6 @@ public function testBase(): void $factory = new ThrowableResponseFactory( new ResponseFactory(), new ErrorHandler( - new NullLogger(), new PlainTextRenderer(), ), new ContentTypeRendererProvider( @@ -36,8 +34,8 @@ public function testBase(): void ); $response = $factory->create( - new LogicException('test message'), TestHelper::createRequest(), + new LogicException('test message') ); assertSame(500, $response->getStatusCode()); @@ -49,7 +47,6 @@ public function testHeaders(): void $factory = new ThrowableResponseFactory( new ResponseFactory(), new ErrorHandler( - new NullLogger(), new PlainTextRenderer(), ), new ContentTypeRendererProvider( @@ -59,8 +56,8 @@ public function testHeaders(): void ); $response = $factory->create( - new LogicException('test message'), TestHelper::createRequest(), + new LogicException('test message') ); assertTrue($response->hasHeader('X-Test'));