-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
[HttpKernel] 15874 framework exceptions #15916
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
Conversation
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.
Thanks. I let few comments
Thanks for the review, should i make it one commit again? Or is it okay like this? |
I was wondering if it uses an instance of the exception class. I checked the code and it does, maybe this should be documented? Also, the order is important. |
@kbond So it would be nice to have 2 notices 1 mentioning the exception should extend from |
This isn't what I meant. I meant that the first exception class that passes an I think one notice is sufficient as it is related: framework:
exceptions:
Exception:
log_level: debug
status_code: 404
RuntimeException: # this will never be used as \RuntimeException extends \Exception
log_level: debug
status_code: 422 The fix would be to reverse the above: framework:
exceptions:
RuntimeException:
log_level: debug
status_code: 422
Exception:
log_level: debug
status_code: 404 Hope that makes sense. |
Thanks Johan! This is now merged. |
Fixes #15874