Better JSON handling in requests and responses #45
Conversation
…nd with 400 if none found.
| def define_routes(self): | ||
| @self.app.route('/') | ||
| def manifest(): | ||
| return json.dumps(dict( | ||
| return jsonify(dict( |
There was a problem hiding this comment.
Flask's jsonify() is a helpful replacement for json.dumps() that wraps its parameter in a Flask Response object and sets the MIME type to application/json.
| return wrapped | ||
|
|
||
| def get_json_or_none_if_invalid(request): | ||
| return request.get_json(force=True, silent=True) |
There was a problem hiding this comment.
This is a thin wrapper around Flask's request.get_json() but it ensures that we are calling it such that it will sniff JSON even if the content-type: application/json MIME type is not set, and will return None if the sniff fails instead of throwing an error.
We can then use this util function elsewhere to make sure that we are parsing JSON from Flask response objects in a consistent way.
| else: | ||
| err_msg = 'The body of all POST requests must contain JSON' | ||
| return jsonify(dict(error=err_msg)), 400 | ||
| return wrapped |
There was a problem hiding this comment.
A decorator to wrap around POST endpoints to ensure that requests that are sent to them contain JSON bodies. If they don't we immediately return a JSON error message and a 400 error code.
Codecov Report
@@ Coverage Diff @@
## master #45 +/- ##
=========================================
+ Coverage 91.98% 94.09% +2.1%
=========================================
Files 5 5
Lines 424 440 +16
=========================================
+ Hits 390 414 +24
+ Misses 34 26 -8 |
content-type: application/jsonas response MIME typecontent-type: application/jsonis not specified. If the sniff is successful, allow the request. If it fails return a nicer 400 explaining the body must be JSON instead of a 500 internal server error.My hope is that this gets us into the territory where the model server now only speaks JSON as responses. I have yet to get an
html/textsince implementing these changes but we should be on the lookout in case we missed something.Closes #1
Closes #6