http: add ability to pass response bodies in base_exception
#15
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Since an exception carries some text for the response body text, the raising site might like to specify the content type if it's e.g. json. Signed-off-by: John Spray <jcs@vectorized.io>
This enables throwing a base_exception from a json request handler with a json payload inside it. Signed-off-by: John Spray <jcs@vectorized.io>
Signed-off-by: John Spray <jcs@vectorized.io>
dotnwat
reviewed
Dec 20, 2021
| @@ -82,7 +82,12 @@ std::unique_ptr<reply> routes::exception_reply(std::exception_ptr eptr) { | |||
| } | |||
| std::rethrow_exception(eptr); | |||
| } catch (const base_exception& e) { | |||
| rep->set_status(e.status(), json_exception(e).to_json()); | |||
| if (e.content_type().size()) { | |||
| rep->set_status(e.status(), e.str()); | |||
There was a problem hiding this comment.
why aren't both conditional cases the same w.r.t. set_status (ie: json_exception(e).to_json())? I was thinking the only thing happening was that the content type was being plumbed through for context.
There was a problem hiding this comment.
json_exception wraps the string up in a message like {"error": , "status": 400}
When content_type is set, the caller is indicating that they want to send the whole request body themselves (i.e. e.str() is the whole body, not a string to go within a json object)
We have a bit of an accumulation of smallish http changes that at some point should go upstream. |
dotnwat
approved these changes
Dec 21, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In JSON request handlers, the string carried in the exception would be wrapped in a {"message": , "status":
} body. That prevented sending back a full response body to provide structured errors, e.g. dicts of request attributes to validation issues.In this PR, a content_type field is added to base_exception, and if this content type is set then the exception's string is used as the full response body, and the response's content type header is set.