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

Request object received in ErrorHandler is not in context #3250

Closed
odahcam opened this issue Feb 20, 2023 · 5 comments
Closed

Request object received in ErrorHandler is not in context #3250

odahcam opened this issue Feb 20, 2023 · 5 comments

Comments

@odahcam
Copy link

odahcam commented Feb 20, 2023

In the error handler, I expect the ServerRequestInterface to be the most recent request value passed down in the request pipeline before the error happened. Instead, the actual error handler receives the first request version given to App::handle method (for most applications created from globals), which in some cases might not be useful specially since you can't get the route context from it since this request version comes from before the routing middleware determining the current route.

Actually I know it's kinda difficult to capture the request from the middleware stack and pass it to the handler, my solution itself (with Slim 3) was kinda unpleasant, but worked very well; I've introduced a try-catch block in the "dispatcher" that would listen for Errors/Exceptions and in the catch would grab the request and add both the throwable and the request snapshot into a custom app exception and threw it so it could then be captured and handled by the app that would extract the original throwable and the request snapshot and then pass it to the error handler.

Anyway, I'm looking for a cleaner solution for achieving that functionality.

@odan
Copy link
Contributor

odan commented Feb 20, 2023

You could do the "same" within a custom middleware.

@odahcam
Copy link
Author

odahcam commented Feb 21, 2023

Yeah I'm doing it, even though it's cumbersome in Slim 4. Ultimately I think this would be a helpful thing to have by default in the framework.

@l0gicgate
Copy link
Member

l0gicgate commented Feb 21, 2023

The most up to date request object is stored in the exception.

Use $exception->getRequest()

https://github.com/slimphp/Slim/blob/4.x/Slim/Exception/HttpException.php#L38

When you throw exceptions within your pipeline, you should throw instances of HttpException with the most up to date request so it automatically passes them back to the handler:
https://github.com/slimphp/Slim/blob/4.x/Slim/Middleware/ErrorMiddleware.php#L85

$app->get('/hello/{name}', function (Request $request, Response $response, array $args) {
    $request = $request->withAttribute(‘key’, ‘value’);

    throw new HttpBadRequestException($request, ‘Something went wrong’);
});

You may need to use try/catch and rethrow exceptions coming from your domain/infrastructure as HttpException

@odahcam
Copy link
Author

odahcam commented Feb 21, 2023

Well that's true expect you're talking about a completely different scenario. It is not exceptions thrown by my code that I'm struggling with, it is exceptions (Throwables, TypeErrors, etc.) comming from either PHP or 3rd-party libraries that are the problem. Stuff like TypeError are not catch anywhere so they do not have the request in context, that would be useful for handling the error since during the request pipeline we do store things like request scoped services as attributes in the request.

That said I don't think this should be closed since we don't have a solution nor a conclusion for a good way to handle this.

Continuing, if you (@l0gicgate) believe that this behavior I described should be the way Slim works (has context in HTTP exceptions and nothing with other errors) how would you prepare request-scoped services (such as session handlers or template engines) for using during the error handling/rendering? I don't like the idea of configuring them before the error middleware nor having to instantiate and build everything again inside the error handling, instead I'd much rather prefer to access the ones already present in the request, if I could have access to it.

I also did think of having two separate error boundaries using two error middleware so I could have some context on the request in the second and the first one exists only for emergencies, but doesn't seem much clean since I'd have to throw some errors from the "inner boundary".

@odahcam
Copy link
Author

odahcam commented Feb 23, 2023

@l0gicgate @odan no considerations?

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

3 participants