Skip to content

Middleware never observe the response for a thrown exception on 0.3, so MetricMiddleware labels every non-HttpException as 500 #482

Description

@andrew-nuwber

PHP Version

8.4.23

Swoole Extension Version

6.2.2

Database Driver & Version

N/A — not database related, the repro below needs no database.

Description

On the 0.3 line the exception handler runs in Kernel::onRequest(), outside the whole middleware pipeline. A throwable propagates past every middleware, so no middleware can ever see the status the client actually receives.

src/foundation/src/Http/Kernel.php:

$response = $this->dispatcher->dispatch(
    $request,
    $this->getMiddlewareForRequest($request),
    $this->coreMiddleware
);
} catch (Throwable $throwable) {
    $response = $this->getResponseForException($throwable);   // <- outside the pipeline
}

Hypervel\Dispatcher\Pipeline::carry() (src/dispatcher/src/Pipeline.php) overrides Laravel's carry() but, unlike Illuminate\Routing\Pipeline, has no try/catch and no handleException(). Hyperf\Pipeline\Pipeline has neither either, so nothing in the chain turns a throwable into a response before it leaves the pipeline.

Impact. Hyperf\Metric\Middleware\MetricMiddleware is the clearest case:

} catch (Throwable $exception) {
    if ($exception instanceof HttpException) {
        $labels['request_status'] = (string) $exception->getStatusCode();
    }
    throw $exception;
}

request_status stays at its '500' default for anything that is not a Hyperf HttpException. ModelNotFoundException is exactly that case: Handler::prepareException() maps it to NotFoundHttpException, so the client gets a 404 while the metric records a 500. Every 404 in an application that uses findOrFail() is counted as a server error, which makes error-rate alerting on request_status unusable.

The same blindness affects any middleware that inspects the response — request logging, timing by status, error-path header handling.

This is already fixed on 0.4 by Hypervel\Routing\Pipeline::handleException() plus the try/catch in Hypervel\Pipeline\Pipeline::carry(). Since 0.4 is not released, I am asking whether a 0.3 backport is in scope.

Suggested backport, mirroring the 0.4 split:

  1. Hypervel\Dispatcher\Pipeline::carry() — wrap the slice body in try/catch delegating to a new protected function handleException(mixed $passable, Throwable $e): mixed whose default implementation rethrows. Self-contained, no new dependencies, no behaviour change on its own.
  2. A Hypervel\Foundation\Http\Pipeline subclass overriding handleException() to report() + render() through ExceptionHandler, bound as Hypervel\Dispatcher\Pipeline::class => Hypervel\Foundation\Http\Pipeline::class in the foundation ConfigProvider.

The subclass has to live in hypervel/foundation, because hypervel/dispatcher does not depend on the exception-handler contract and adding that dependency would invert the layering.

Two 0.3-specific details that differ from 0.4:

  • The passable in HttpRequestHandler::handle() is a Hyperf\HttpMessage\Server\Request, not a Hypervel\Http\Request. 0.4's ! $passable instanceof Request guard would rethrow on every request and make the backport a silent no-op. Handler::handle() resolves the request from the container instead, and the backport should do the same.
  • withoutDuplicates defaults to false, so double reporting matters. Once the pipeline returns a response, Kernel::onRequest()'s catch no longer fires for pipeline throwables, so the exception is still reported once.

Worth a release note either way: middleware that currently catch domain exceptions stop seeing them, because the exception is already a response by the time it reaches them. That is Laravel's behaviour, so 0.4 already carries the same change.

Steps To Reproduce

Tested on hypervel/framework v0.3.14 with hyperf/metric 3.1.65. The same code is present in v0.3.25.

  1. Install hyperf/metric and leave config/autoload/metric.php at its defaults (prometheus driver, SCRAPE_MODE, scrape port 9502).

  2. Register the metric middleware globally in app/Http/Kernel.php:

    protected array $middleware = [
        \Hyperf\Metric\Middleware\MetricMiddleware::class,
    ];
  3. Add a route that throws a ModelNotFoundException. No model or database is needed:

    Route::get('/boom', fn () => throw new \Hypervel\Database\Eloquent\ModelNotFoundException());
  4. Call the route, then scrape the metrics endpoint:

    $ curl -s -o /dev/null -w '%{http_code}\n' http://127.0.0.1:9501/boom
    404
    
    $ curl -s http://127.0.0.1:9502/metrics | grep http_requests_count
    hyperf_http_requests_count{request_status="500",request_path="/boom",request_method="GET"} 1
    

Expected: request_status="404", the status the client received.
Actual: request_status="500".

For reference, the workaround we run in production subclasses the middleware and applies the mapping inside the parent's own try/catch:

class MetricMiddleware extends BaseMetricMiddleware
{
    public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
    {
        return parent::process($request, new class($handler) implements RequestHandlerInterface {
            public function __construct(private RequestHandlerInterface $inner) {}

            public function handle(ServerRequestInterface $request): ResponseInterface
            {
                try {
                    return $this->inner->handle($request);
                } catch (ModelNotFoundException $e) {
                    throw new NotFoundHttpException($e->getMessage(), 0, $e);
                }
            }
        });
    }
}

It works, but it duplicates one arm of Handler::prepareException() and only fixes this one middleware.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions