-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[HttpKernel][Debug] Get exception content according to request format #30851
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
You're right, this is for master. |
3d66ef3
to
aa6bc3d
Compare
Do we follow any RFC regarding the formatting of the json/xml/etc payloads? |
Not really, I just copied the content format from TwigBundle exceptions.
Good idea! I'll check the RFC for each response format. Later, I guess we should update the TwigBundle errors/exceptions as well. |
Status: Needs Work |
aa6bc3d
to
94167b0
Compare
Status: Needs Review |
94167b0
to
259fa20
Compare
This is ready so far, because we don't have more info from HttpException, but if we wanted to complete the spec details...
... we might need a meta argument containing standard (e.g. Ideally something like this: throw new ProblemDetailsHttpException(400, 'Your request parameters didn't validate.', null, ['Content-Type' => 'application/problem+json'], 0, [
'type' => 'https://example.net/validation-error',
'invalid-params' => [
[
'name' => 'age',
'reason' => 'must be a positive integer'
],
[
'name' => 'color',
'reason' => 'must be green, red or blue'
],
],
]); then, the last argument (named e.g. meta) could be handled by |
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.
Nice, thanks.
About your last comment, I wouldn't go further for a generic Debug component - for now at least.
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.
👍
} elseif (class_exists(Response::class) && isset(Response::$statusTexts[$statusCode])) { | ||
$title = Response::$statusTexts[$statusCode]; | ||
} else { | ||
$title = 'Internal Server Error'; |
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.
so a 403 error without symfony/http-foundation
installed would lead to this title/message? Might be a bit confusing 😄
$exception = FlattenException::create($exception); | ||
} | ||
|
||
if (404 === $statusCode = $exception->getStatusCode()) { |
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.
this blocks seems repeated for all content-types? maybe move it to a private method?
@yceruto Thank you for working on this topic, which we should tackle. But I don't think this is the right approach. Let's talk about this this week-end during the Hackaton. |
@fabpot then I guess should I close here, right? |
No, we should talk first |
As discussed, this code should be moved to a new component (maybe Later, the |
See #31065 |
Mainly for API-based apps that don't require TwigBundle to get the correct exception response according to the request format (aka
_format
attribute).Let me know if this classify as a bugfix for 4.2, but IMHO this was never supported without TwigBundle, hence
master
branch.