Skip to content

Commit 56231a7

Browse files
alganetalganet
committed
Fix exception handling, rank fallthrough, and add statusRoute()
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.
1 parent 1d8db28 commit 56231a7

9 files changed

Lines changed: 239 additions & 24 deletions

File tree

‎src/DispatchContext.php‎

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use Respect\Rest\Routines\Routinable;
1616
use Throwable;
1717

18+
use function is_a;
1819
use function rawurldecode;
1920
use function rtrim;
2021
use function set_error_handler;
@@ -49,6 +50,9 @@ final class DispatchContext
4950

5051
private string $effectivePath = '';
5152

53+
/** @var array<int, AbstractRoute> */
54+
private array $sideRoutes = [];
55+
5256
public function __construct(
5357
public ServerRequestInterface $request,
5458
public ResponseFactoryInterface&StreamFactoryInterface $factory,
@@ -121,6 +125,11 @@ public function response(): ResponseInterface|null
121125
{
122126
if (!$this->route instanceof AbstractRoute) {
123127
if ($this->responseDraft !== null) {
128+
$statusResponse = $this->forwardToStatusRoute($this->responseDraft);
129+
if ($statusResponse !== null) {
130+
return $statusResponse;
131+
}
132+
124133
return $this->finalizeResponse($this->responseDraft);
125134
}
126135

@@ -210,6 +219,12 @@ public function setRoutinePipeline(RoutinePipeline $routinePipeline): void
210219
$this->routinePipeline = $routinePipeline;
211220
}
212221

222+
/** @param array<int, AbstractRoute> $sideRoutes */
223+
public function setSideRoutes(array $sideRoutes): void
224+
{
225+
$this->sideRoutes = $sideRoutes;
226+
}
227+
213228
public function setResponder(Responder $responder): void
214229
{
215230
$this->responder = $responder;
@@ -260,12 +275,7 @@ protected function catchExceptions(Throwable $e, AbstractRoute $route): Response
260275
continue;
261276
}
262277

263-
$exceptionClass = $e::class;
264-
if (
265-
$exceptionClass === $sideRoute->class
266-
|| $sideRoute->class === 'Exception'
267-
|| $sideRoute->class === '\Exception'
268-
) {
278+
if (is_a($e, $sideRoute->class)) {
269279
$sideRoute->exception = $e;
270280

271281
return $this->forward($sideRoute);
@@ -275,6 +285,31 @@ protected function catchExceptions(Throwable $e, AbstractRoute $route): Response
275285
return null;
276286
}
277287

288+
protected function forwardToStatusRoute(ResponseInterface $preparedResponse): ResponseInterface|null
289+
{
290+
$statusCode = $preparedResponse->getStatusCode();
291+
292+
foreach ($this->sideRoutes as $sideRoute) {
293+
if (
294+
$sideRoute instanceof Routes\Status
295+
&& ($sideRoute->statusCode === $statusCode || $sideRoute->statusCode === null)
296+
) {
297+
$this->hasStatusOverride = true;
298+
299+
// Run routine negotiation (e.g. Accept) before forwarding,
300+
// since the normal route-selection phase was skipped
301+
$this->routinePipeline()->matches($this, $sideRoute, $this->params);
302+
303+
$result = $this->forward($sideRoute);
304+
305+
// Preserve the original status code on the forwarded response
306+
return $result?->withStatus($statusCode);
307+
}
308+
}
309+
310+
return null;
311+
}
312+
278313
/** @param array<int, mixed> $params */
279314
protected function extractRouteParam(
280315
ReflectionFunctionAbstract $callback,

‎src/DispatchEngine.php‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
use Psr\Http\Server\RequestHandlerInterface;
1313
use Respect\Rest\Routes\AbstractRoute;
1414
use SplObjectStorage;
15-
use Throwable;
1615

1716
use function array_filter;
1817
use function array_keys;
@@ -49,11 +48,7 @@ public function dispatch(ServerRequestInterface $serverRequest): DispatchContext
4948

5049
public function handle(ServerRequestInterface $request): ResponseInterface
5150
{
52-
try {
53-
$response = $this->dispatch($request)->response();
54-
} catch (Throwable) {
55-
return $this->factory->createResponse(500);
56-
}
51+
$response = $this->dispatch($request)->response();
5752

5853
return $response ?? $this->factory->createResponse(500);
5954
}
@@ -65,6 +60,7 @@ public function dispatchContext(DispatchContext $context): DispatchContext
6560
}
6661

6762
$context->setRoutinePipeline($this->routinePipeline);
63+
$context->setSideRoutes($this->routeProvider->getSideRoutes());
6864

6965
if (!$this->isRoutelessDispatch($context) && $context->route === null) {
7066
$this->routeDispatch($context);
@@ -289,9 +285,9 @@ private function routineMatch(
289285
DispatchContext $context,
290286
SplObjectStorage $matchedByPath,
291287
): DispatchContext|bool|null {
292-
$badRequest = false;
293-
294288
foreach ([0, 1, 2] as $rank) {
289+
$rankMatched = false;
290+
295291
foreach ($matchedByPath as $route) {
296292
if ($this->getMethodMatchRank($context, $route) !== $rank) {
297293
continue;
@@ -309,11 +305,17 @@ private function routineMatch(
309305
);
310306
}
311307

312-
$badRequest = true;
308+
$rankMatched = true;
309+
}
310+
311+
// If a route at this rank matched the method but failed routines,
312+
// don't fall through to lower-priority ranks
313+
if ($rankMatched) {
314+
return false;
313315
}
314316
}
315317

316-
return $badRequest ? false : null;
318+
return null;
317319
}
318320

319321
/**

‎src/RouteProvider.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,5 +11,8 @@ interface RouteProvider
1111
/** @return array<int, AbstractRoute> */
1212
public function getRoutes(): array;
1313

14+
/** @return array<int, AbstractRoute> */
15+
public function getSideRoutes(): array;
16+
1417
public function getBasePath(): string;
1518
}

‎src/Router.php‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,20 @@ public function errorRoute(callable $callback): Routes\Error
186186
return $route;
187187
}
188188

189+
public function statusRoute(int|null $statusCode, callable $callback): Routes\Status
190+
{
191+
$route = new Routes\Status($statusCode, $callback);
192+
$this->appendSideRoute($route);
193+
194+
return $route;
195+
}
196+
197+
/** @return array<int, Routes\AbstractRoute> */
198+
public function getSideRoutes(): array
199+
{
200+
return $this->sideRoutes;
201+
}
202+
189203
public function factoryRoute(string $method, string $path, string $className, callable $factory): Routes\Factory
190204
{
191205
$route = new Routes\Factory($method, $path, $className, $factory);

‎src/Routes/Status.php‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Respect\Rest\Routes;
6+
7+
final class Status extends Callback
8+
{
9+
/** @var callable */
10+
public $callback;
11+
12+
public function __construct(public readonly int|null $statusCode, callable $callback)
13+
{
14+
parent::__construct('ANY', '^$', $callback);
15+
}
16+
}

‎tests/DispatchEngineTest.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -136,17 +136,17 @@ public function testHandleReturnsPsr7Response(): void
136136
self::assertSame('world', (string) $response->getBody());
137137
}
138138

139-
public function testHandleReturns500OnException(): void
139+
public function testHandlePropagatesUnhandledExceptions(): void
140140
{
141141
$engine = $this->engine([
142142
new Callback('GET', '/boom', static function (): never {
143143
throw new RuntimeException('fail');
144144
}),
145145
]);
146146

147-
$response = $engine->handle(new ServerRequest('GET', '/boom'));
148-
149-
self::assertSame(500, $response->getStatusCode());
147+
$this->expectException(RuntimeException::class);
148+
$this->expectExceptionMessage('fail');
149+
$engine->handle(new ServerRequest('GET', '/boom'));
150150
}
151151

152152
public function testOnContextReadyCallbackIsInvoked(): void

‎tests/RouterTest.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1843,16 +1843,16 @@ public function testDispatchEngineHandleReturnsSameResponseAsDispatch(): void
18431843
self::assertSame((string) $dispatched->getBody(), (string) $handled->getBody());
18441844
}
18451845

1846-
public function testDispatchEngineHandleReturns500ForUncaughtExceptions(): void
1846+
public function testDispatchEngineHandlePropagatesUncaughtExceptions(): void
18471847
{
18481848
$router = self::newRouter();
18491849
$router->get('/', static function (): void {
18501850
throw new InvalidArgumentException('boom');
18511851
});
18521852

1853-
$response = $router->dispatchEngine()->handle(new ServerRequest('GET', '/'));
1854-
1855-
self::assertSame(500, $response->getStatusCode());
1853+
$this->expectException(InvalidArgumentException::class);
1854+
$this->expectExceptionMessage('boom');
1855+
$router->dispatchEngine()->handle(new ServerRequest('GET', '/'));
18561856
}
18571857

18581858
public function testDispatchEngineHandlePreservesExceptionRoutes(): void

‎tests/Routes/ExceptionTest.php‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,11 @@
66

77
use Nyholm\Psr7\Factory\Psr17Factory;
88
use Nyholm\Psr7\ServerRequest;
9+
use PDOException;
910
use PHPUnit\Framework\TestCase;
1011
use Respect\Rest\Router;
1112
use RuntimeException;
13+
use Throwable;
1214

1315
/** @covers Respect\Rest\Routes\Exception */
1416
final class ExceptionTest extends TestCase
@@ -48,4 +50,43 @@ public function testMagicConstuctorCanCreateRoutesToExceptions(): void
4850
'An exception should be caught by the router and forwarded',
4951
);
5052
}
53+
54+
public function testExceptionRouteCatchesSubclassViaInheritance(): void
55+
{
56+
$router = new Router('', new Psr17Factory());
57+
$router->exceptionRoute('RuntimeException', static fn($e) => 'caught: ' . $e->getMessage());
58+
$router->get('/', static function (): void {
59+
throw new PDOException('db error');
60+
});
61+
62+
$resp = $router->dispatch(new ServerRequest('GET', '/'))->response();
63+
self::assertNotNull($resp);
64+
self::assertSame('caught: db error', (string) $resp->getBody());
65+
}
66+
67+
public function testThrowableExceptionRouteCatchesAll(): void
68+
{
69+
$router = new Router('', new Psr17Factory());
70+
$router->exceptionRoute('Throwable', static fn(Throwable $e) => 'caught: ' . $e::class);
71+
$router->get('/', static function (): void {
72+
throw new RuntimeException('test');
73+
});
74+
75+
$resp = $router->dispatch(new ServerRequest('GET', '/'))->response();
76+
self::assertNotNull($resp);
77+
self::assertSame('caught: RuntimeException', (string) $resp->getBody());
78+
}
79+
80+
public function testExceptionRouteWorksViaHandle(): void
81+
{
82+
$router = new Router('', new Psr17Factory());
83+
$router->exceptionRoute('Throwable', static fn(Throwable $e) => 'handled: ' . $e->getMessage());
84+
$router->get('/', static function (): void {
85+
throw new RuntimeException('boom');
86+
});
87+
88+
$response = $router->handle(new ServerRequest('GET', '/'));
89+
self::assertSame(200, $response->getStatusCode());
90+
self::assertSame('handled: boom', (string) $response->getBody());
91+
}
5192
}

0 commit comments

Comments
 (0)