Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ErrorMiddleware::handleException function passes wrong number of parameters to handler #2564

Closed
lordrhodos opened this issue Jan 11, 2019 · 2 comments

Comments

@lordrhodos
Copy link

The new ErrorMiddleware currently passes 5 parameters to the handler:

$params = [
$request,
$exception,
$this->displayErrorDetails,
$this->logErrors,
$this->logErrorDetails,
];

The PR #2398 documents four parameters, one of them being a $response object:

...
$callableResolver = $app->getCallableResolver();
$errorMiddleware = new ErrorMiddleware($callableResolver, true, true, true);

$handler = function ($req, $res, $exception, $displayErrorDetails) {
    return $res->withJson(['error' => 'Caught MyNamedException']);
}
$errorMiddleware->setErrorHandler(MyNamedException::class, $handler);
$app->add($errorMiddleware);

This PR fixes several issues:

  • pass $response object as second parameter to the handler with the status code set to the exceptions code
  • update documentations

a working example looks like:

$callableResolver = $app->getCallableResolver();
$errorMiddleware = new ErrorMiddleware($callableResolver, $responseFactory, true, false, true);
$handler = function (
    ServerRequestInterface $request,
    ResponseInterface $response,
    ApiException $exception,
    bool $displayErrorDetails,
    bool $logErrors,
    bool $logErrorDetails
) {
    $data = ['code' => $exception->getErrorCode(), 'description' => $exception->getErrorDescription()];
    $response = $response->withHeader('Content-Type', 'application/json');
    $response->getBody()->write(\json_encode($data));

    return $response;
};
$errorMiddleware->setErrorHandler(ApiException::class, $handler);
$app->add($errorMiddleware);

Discussion
The phpdocs on some methods mention the return type

* The callable MUST return an instance of
* \Psr\Http\Message\ResponseInterface.

but it is not enforced when calling the handler callable. I suggest to add correct return type declaration to the ErrorMiddleware::handleException function but wanted to get you opinion about it. Should this be enforced and if how would you have this enforced?

@lordrhodos lordrhodos changed the title ErrorMiddleware::handleException function passes wrong number of parameters to handler: response is missing ErrorMiddleware::handleException function passes wrong number of parameters to handler Jan 11, 2019
@l0gicgate
Copy link
Member

The Response object needs to be created in the handler, hence why it isn't being passed. Also with the migration to PSR-15 Middleware the Response object won't be available at all via ErrorMiddleware anymore after #2555 is merged.

@lordrhodos
Copy link
Author

@l0gicgate thx for pointing this out. I missed this in your PSR-15 middleware branch I guess. I was stumbling across this in the current 4.x branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants