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
New extender for error handling #1970
Conversation
Tagging some people:
|
So taking in consideration the use case mentioned in the issue the usage of this extender would be something like: return [
(new Extend\ErrorHandling)
->handler(MyException::class, MyExceptionHandler::class)
]; Where Shouldn't there be an interface for exception handlers, perhaps? |
@matteocontrini That depends on the exact use-case. If all you're doing is configuring the status code for an exception, you should use the other methods. Handlers are really only necessary for adding more details to error output (in the JSON-API layer). #1843 removed most of our layers, and only two remain - those embed validation errors into the JSON-API response. So, assuming you're doing something simpler than that, please try out the other methods.
I thought about it and it does not help much, as that would mean you cannot type hint the argument to the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PHPDoc needed for handler
method - probably including warning that one of the other methods should work instead or something. And perhaps specify what instances it would be used and why the others don't help in those... if that makes sense.
8f0d739
to
02249fe
Compare
You're right, thanks! Added another docblock. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
This extender implements several methods for extending the new error handling stack implemented in flarum#1843. Most use-cases should be covered, but I expect some challenges for more complex setups. We can tackle those once they come up, though. Basic use-cases should be covered. Fixes flarum#1781.
Fixes #1781
This extender implements several methods for extending the new error
handling stack implemented in #1843.
Most use-cases should be covered, but I expect some challenges for more
complex setups. We can tackle those once they come up, though. Basic
use-cases should be covered.
Confirmed
composer test
).