-
Notifications
You must be signed in to change notification settings - Fork 24.3k
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
Handle exceptions thrown from RestCompatibleVersionHelper #80253
Handle exceptions thrown from RestCompatibleVersionHelper #80253
Conversation
Pinging @elastic/es-core-infra (Team:Core/Infra) |
// e.getMessage(), | ||
// equalTo("Unknown media type Accept=wrong/type;compatible-with=") | ||
// ); | ||
// } |
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 part right here seems off. Perhaps accidentally committed? WIP that was meant to be removed?
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.
ah yes, initially I thought I can include validation for unknown media types before parsing versions, but this is far beyond the scope of this PR.
super(cause); | ||
this.failedHeaderName = failedHeaderName; | ||
this.failedHeaderNames = Set.of(failedHeaderNames); | ||
this.message = "Invalid media-type value on header " + this.failedHeaderNames; |
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.
Little wording/formatting nit -- maybe "on headers [HeaderA, HeaderB, HeaderC]"? What do you think?
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.
agree - plural looks better. I guess for single header that failed it is fine too?
example: Invalid media-type value on headers [Content-Type]
I rely on AbstractCollection.toString here when it comes to set formatting
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
💔 Backport failed
You can use sqren/backport to manually backport by running |
) RestCompatibleVersionHelper is used to validate versions on Accept and Content-Type headers When a validation fails, an exception is being thrown indicating which headers are incorrect. That exception was not handled in RestRequest where the helper was used. This commit gracefully handles the exception from validation failure. A bad request response is returned to a user. closes elastic#79060 closes elastic#78214
…80605) RestCompatibleVersionHelper is used to validate versions on Accept and Content-Type headers When a validation fails, an exception is being thrown indicating which headers are incorrect. That exception was not handled in RestRequest where the helper was used. This commit gracefully handles the exception from validation failure. A bad request response is returned to a user. closes #79060 closes #78214
RestCompatibleVersionHelper is used to validate versions on Accept and Content-Type headers
When a validation fails, an exception is being thrown indicating which headers are incorrect.
That exception was not handled in RestRequest where the helper was used.
This commit gracefully handles the exception from validation failure. A bad request response is returned to a user.
closes #79060
closes #78214