From d8e8ea9af3c2411aa6a9e3bf694cdfb60cbc4de9 Mon Sep 17 00:00:00 2001 From: Alexandre Gomes Gaigalas Date: Thu, 19 Mar 2026 21:37:39 -0300 Subject: [PATCH] Fix exception handling, rank fallthrough, and add statusRoute() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Exception routes now use is_a() for class matching, supporting inheritance and Throwable. handle() no longer swallows exceptions silently — unhandled exceptions propagate per PSR-15. routineMatch() no longer falls through from exact-method (rank 0) to any-method (rank 1) routes when a when() routine fails, preventing validation bypass via catch-all routes. New statusRoute() API lets users handle framework-generated HTTP status codes (404, 405, 400, etc.) with custom callbacks, including content negotiation via accept(). Passing null as status code creates a catch-all handler for any status. --- src/DispatchContext.php | 47 +++++++++++++-- src/DispatchEngine.php | 22 +++---- src/RouteProvider.php | 3 + src/Router.php | 14 +++++ src/Routes/Status.php | 16 +++++ tests/DispatchEngineTest.php | 8 +-- tests/RouterTest.php | 8 +-- tests/Routes/ExceptionTest.php | 41 +++++++++++++ tests/Routes/StatusTest.php | 104 +++++++++++++++++++++++++++++++++ 9 files changed, 239 insertions(+), 24 deletions(-) create mode 100644 src/Routes/Status.php create mode 100644 tests/Routes/StatusTest.php diff --git a/src/DispatchContext.php b/src/DispatchContext.php index d75998e..9c6d5cd 100644 --- a/src/DispatchContext.php +++ b/src/DispatchContext.php @@ -15,6 +15,7 @@ use Respect\Rest\Routines\Routinable; use Throwable; +use function is_a; use function rawurldecode; use function rtrim; use function set_error_handler; @@ -49,6 +50,9 @@ final class DispatchContext private string $effectivePath = ''; + /** @var array */ + private array $sideRoutes = []; + public function __construct( public ServerRequestInterface $request, public ResponseFactoryInterface&StreamFactoryInterface $factory, @@ -121,6 +125,11 @@ public function response(): ResponseInterface|null { if (!$this->route instanceof AbstractRoute) { if ($this->responseDraft !== null) { + $statusResponse = $this->forwardToStatusRoute($this->responseDraft); + if ($statusResponse !== null) { + return $statusResponse; + } + return $this->finalizeResponse($this->responseDraft); } @@ -210,6 +219,12 @@ public function setRoutinePipeline(RoutinePipeline $routinePipeline): void $this->routinePipeline = $routinePipeline; } + /** @param array $sideRoutes */ + public function setSideRoutes(array $sideRoutes): void + { + $this->sideRoutes = $sideRoutes; + } + public function setResponder(Responder $responder): void { $this->responder = $responder; @@ -260,12 +275,7 @@ protected function catchExceptions(Throwable $e, AbstractRoute $route): Response continue; } - $exceptionClass = $e::class; - if ( - $exceptionClass === $sideRoute->class - || $sideRoute->class === 'Exception' - || $sideRoute->class === '\Exception' - ) { + if (is_a($e, $sideRoute->class)) { $sideRoute->exception = $e; return $this->forward($sideRoute); @@ -275,6 +285,31 @@ protected function catchExceptions(Throwable $e, AbstractRoute $route): Response return null; } + protected function forwardToStatusRoute(ResponseInterface $preparedResponse): ResponseInterface|null + { + $statusCode = $preparedResponse->getStatusCode(); + + foreach ($this->sideRoutes as $sideRoute) { + if ( + $sideRoute instanceof Routes\Status + && ($sideRoute->statusCode === $statusCode || $sideRoute->statusCode === null) + ) { + $this->hasStatusOverride = true; + + // Run routine negotiation (e.g. Accept) before forwarding, + // since the normal route-selection phase was skipped + $this->routinePipeline()->matches($this, $sideRoute, $this->params); + + $result = $this->forward($sideRoute); + + // Preserve the original status code on the forwarded response + return $result?->withStatus($statusCode); + } + } + + return null; + } + /** @param array $params */ protected function extractRouteParam( ReflectionFunctionAbstract $callback, diff --git a/src/DispatchEngine.php b/src/DispatchEngine.php index 59cacd1..e621ed9 100644 --- a/src/DispatchEngine.php +++ b/src/DispatchEngine.php @@ -12,7 +12,6 @@ use Psr\Http\Server\RequestHandlerInterface; use Respect\Rest\Routes\AbstractRoute; use SplObjectStorage; -use Throwable; use function array_filter; use function array_keys; @@ -49,11 +48,7 @@ public function dispatch(ServerRequestInterface $serverRequest): DispatchContext public function handle(ServerRequestInterface $request): ResponseInterface { - try { - $response = $this->dispatch($request)->response(); - } catch (Throwable) { - return $this->factory->createResponse(500); - } + $response = $this->dispatch($request)->response(); return $response ?? $this->factory->createResponse(500); } @@ -65,6 +60,7 @@ public function dispatchContext(DispatchContext $context): DispatchContext } $context->setRoutinePipeline($this->routinePipeline); + $context->setSideRoutes($this->routeProvider->getSideRoutes()); if (!$this->isRoutelessDispatch($context) && $context->route === null) { $this->routeDispatch($context); @@ -289,9 +285,9 @@ private function routineMatch( DispatchContext $context, SplObjectStorage $matchedByPath, ): DispatchContext|bool|null { - $badRequest = false; - foreach ([0, 1, 2] as $rank) { + $rankMatched = false; + foreach ($matchedByPath as $route) { if ($this->getMethodMatchRank($context, $route) !== $rank) { continue; @@ -309,11 +305,17 @@ private function routineMatch( ); } - $badRequest = true; + $rankMatched = true; + } + + // If a route at this rank matched the method but failed routines, + // don't fall through to lower-priority ranks + if ($rankMatched) { + return false; } } - return $badRequest ? false : null; + return null; } /** diff --git a/src/RouteProvider.php b/src/RouteProvider.php index 5491416..75fb1cb 100644 --- a/src/RouteProvider.php +++ b/src/RouteProvider.php @@ -11,5 +11,8 @@ interface RouteProvider /** @return array */ public function getRoutes(): array; + /** @return array */ + public function getSideRoutes(): array; + public function getBasePath(): string; } diff --git a/src/Router.php b/src/Router.php index 65d0dea..a3954a1 100644 --- a/src/Router.php +++ b/src/Router.php @@ -186,6 +186,20 @@ public function errorRoute(callable $callback): Routes\Error return $route; } + public function statusRoute(int|null $statusCode, callable $callback): Routes\Status + { + $route = new Routes\Status($statusCode, $callback); + $this->appendSideRoute($route); + + return $route; + } + + /** @return array */ + public function getSideRoutes(): array + { + return $this->sideRoutes; + } + public function factoryRoute(string $method, string $path, string $className, callable $factory): Routes\Factory { $route = new Routes\Factory($method, $path, $className, $factory); diff --git a/src/Routes/Status.php b/src/Routes/Status.php new file mode 100644 index 0000000..a3da99d --- /dev/null +++ b/src/Routes/Status.php @@ -0,0 +1,16 @@ +getBody()); } - public function testHandleReturns500OnException(): void + public function testHandlePropagatesUnhandledExceptions(): void { $engine = $this->engine([ new Callback('GET', '/boom', static function (): never { @@ -144,9 +144,9 @@ public function testHandleReturns500OnException(): void }), ]); - $response = $engine->handle(new ServerRequest('GET', '/boom')); - - self::assertSame(500, $response->getStatusCode()); + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('fail'); + $engine->handle(new ServerRequest('GET', '/boom')); } public function testOnContextReadyCallbackIsInvoked(): void diff --git a/tests/RouterTest.php b/tests/RouterTest.php index 4a16465..7cb594f 100644 --- a/tests/RouterTest.php +++ b/tests/RouterTest.php @@ -1843,16 +1843,16 @@ public function testDispatchEngineHandleReturnsSameResponseAsDispatch(): void self::assertSame((string) $dispatched->getBody(), (string) $handled->getBody()); } - public function testDispatchEngineHandleReturns500ForUncaughtExceptions(): void + public function testDispatchEngineHandlePropagatesUncaughtExceptions(): void { $router = self::newRouter(); $router->get('/', static function (): void { throw new InvalidArgumentException('boom'); }); - $response = $router->dispatchEngine()->handle(new ServerRequest('GET', '/')); - - self::assertSame(500, $response->getStatusCode()); + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('boom'); + $router->dispatchEngine()->handle(new ServerRequest('GET', '/')); } public function testDispatchEngineHandlePreservesExceptionRoutes(): void diff --git a/tests/Routes/ExceptionTest.php b/tests/Routes/ExceptionTest.php index c12659d..f0bdd9c 100644 --- a/tests/Routes/ExceptionTest.php +++ b/tests/Routes/ExceptionTest.php @@ -6,9 +6,11 @@ use Nyholm\Psr7\Factory\Psr17Factory; use Nyholm\Psr7\ServerRequest; +use PDOException; use PHPUnit\Framework\TestCase; use Respect\Rest\Router; use RuntimeException; +use Throwable; /** @covers Respect\Rest\Routes\Exception */ final class ExceptionTest extends TestCase @@ -48,4 +50,43 @@ public function testMagicConstuctorCanCreateRoutesToExceptions(): void 'An exception should be caught by the router and forwarded', ); } + + public function testExceptionRouteCatchesSubclassViaInheritance(): void + { + $router = new Router('', new Psr17Factory()); + $router->exceptionRoute('RuntimeException', static fn($e) => 'caught: ' . $e->getMessage()); + $router->get('/', static function (): void { + throw new PDOException('db error'); + }); + + $resp = $router->dispatch(new ServerRequest('GET', '/'))->response(); + self::assertNotNull($resp); + self::assertSame('caught: db error', (string) $resp->getBody()); + } + + public function testThrowableExceptionRouteCatchesAll(): void + { + $router = new Router('', new Psr17Factory()); + $router->exceptionRoute('Throwable', static fn(Throwable $e) => 'caught: ' . $e::class); + $router->get('/', static function (): void { + throw new RuntimeException('test'); + }); + + $resp = $router->dispatch(new ServerRequest('GET', '/'))->response(); + self::assertNotNull($resp); + self::assertSame('caught: RuntimeException', (string) $resp->getBody()); + } + + public function testExceptionRouteWorksViaHandle(): void + { + $router = new Router('', new Psr17Factory()); + $router->exceptionRoute('Throwable', static fn(Throwable $e) => 'handled: ' . $e->getMessage()); + $router->get('/', static function (): void { + throw new RuntimeException('boom'); + }); + + $response = $router->handle(new ServerRequest('GET', '/')); + self::assertSame(200, $response->getStatusCode()); + self::assertSame('handled: boom', (string) $response->getBody()); + } } diff --git a/tests/Routes/StatusTest.php b/tests/Routes/StatusTest.php new file mode 100644 index 0000000..b9c080f --- /dev/null +++ b/tests/Routes/StatusTest.php @@ -0,0 +1,104 @@ +get('/exists', static fn() => 'ok'); + $router->statusRoute(404, static fn(ServerRequestInterface $r) => 'Not found: ' . $r->getUri()->getPath()); + + $response = $router->handle(new ServerRequest('GET', '/nope')); + + self::assertSame(404, $response->getStatusCode()); + self::assertSame('Not found: /nope', (string) $response->getBody()); + } + + public function testStatusRoute405(): void + { + $router = new Router('', new Psr17Factory()); + $router->get('/resource', static fn() => 'ok'); + $router->statusRoute(405, static fn() => 'Method not allowed'); + + $response = $router->handle(new ServerRequest('DELETE', '/resource')); + + self::assertSame(405, $response->getStatusCode()); + self::assertSame('Method not allowed', (string) $response->getBody()); + self::assertStringContainsString('GET', $response->getHeaderLine('Allow')); + } + + public function testStatusRoute400FromWhenFailure(): void + { + $router = new Router('', new Psr17Factory()); + $router->get('/guarded', static fn() => 'ok')->when(static fn() => false); + $router->statusRoute(400, static fn() => 'Bad request'); + + $response = $router->handle(new ServerRequest('GET', '/guarded')); + + self::assertSame(400, $response->getStatusCode()); + self::assertSame('Bad request', (string) $response->getBody()); + } + + public function testStatusRouteDoesNotInterfereWithNormalRoutes(): void + { + $router = new Router('', new Psr17Factory()); + $router->get('/hello', static fn() => 'world'); + $router->statusRoute(404, static fn() => 'not found'); + + $response = $router->handle(new ServerRequest('GET', '/hello')); + + self::assertSame(200, $response->getStatusCode()); + self::assertSame('world', (string) $response->getBody()); + } + + public function testWithoutStatusRouteBareResponseIsReturned(): void + { + $router = new Router('', new Psr17Factory()); + $router->get('/exists', static fn() => 'ok'); + + $response = $router->handle(new ServerRequest('GET', '/nope')); + + self::assertSame(404, $response->getStatusCode()); + self::assertSame('', (string) $response->getBody()); + } + + public function testStatusRouteReturnsArrayAsJson(): void + { + $router = new Router('', new Psr17Factory()); + $router->get('/exists', static fn() => 'ok'); + $router->statusRoute( + 404, + static fn(ServerRequestInterface $r) => ['error' => 'Not found', 'path' => $r->getUri()->getPath()], + ); + + $response = $router->handle(new ServerRequest('GET', '/missing')); + + self::assertSame(404, $response->getStatusCode()); + self::assertStringContainsString('application/json', $response->getHeaderLine('Content-Type')); + self::assertSame('{"error":"Not found","path":"\/missing"}', (string) $response->getBody()); + } + + public function testStatusRouteWorksViaDispatch(): void + { + $router = new Router('', new Psr17Factory()); + $router->get('/exists', static fn() => 'ok'); + $router->statusRoute(404, static fn() => 'custom 404'); + + $response = $router->dispatch(new ServerRequest('GET', '/nope'))->response(); + + self::assertNotNull($response); + self::assertSame(404, $response->getStatusCode()); + self::assertSame('custom 404', (string) $response->getBody()); + } +}