feat: json parsing error #2042
feat: json parsing error #2042
Conversation
|
This takes over from #2030 |
src/server/index.ts
Outdated
| err instanceof SyntaxError && | ||
| err.statusCode === 400 && | ||
| 'body' in err && | ||
| err.type === 'entity.parse.failed' |
There was a problem hiding this comment.
Thanks for contributing to our code!
This error message is specific to the body-parser library we use. I suggest that it'd be nice for future reference either to 1) leave an inline comment above to note that this catches body-parser errors and returns a 400 error message (similar like how we did it with Joi errors above), or 2) wrap an error handler around the bodyParser.json() parsing in apiMiddleware like so, so that it's clear what this piece of code is meant to be catching.
(req, res, next) => {
bodyParser.json()(req, res, err => {
if (err) {
console.error(err);
return res.sendStatus(400); // Bad request
}
next();
});
Taken from https://stackoverflow.com/questions/53048642/node-js-handle-body-parser-invalid-json-error
There was a problem hiding this comment.
Hi @gweiying, have added a comment similar to the Joi errors to reflect that this catches body-parser json errors and returns error 400 accordingly
There was a problem hiding this comment.
Thanks for the quick fix.
Pulled your code locally to run - just a couple of small issues.
- missing return statement
- small issue with Typescript - since
statusandtypedoes not exist onSyntaxError, the code cannot be compiled. A quick solution for this is to just drop theerr instanceof SyntaxErrorcheck since the other errors should be specific enough to catch the body parser error, or possibly to create our own error type - the latter might be more involved.
Problem
What problem are you trying to solve? What issue does this close?
Closes #1961
Solution
How did you solve the problem?
Check the incoming error code, type and syntax error. If 400 is received, return 400 back
Features:
Improvements:
Bug Fixes:
Before & After Screenshots
BEFORE:
[insert screenshot here]
AFTER:
[insert screenshot here]
Tests
What tests should be run to confirm functionality?
Deploy Notes
Notes regarding deployment of the contained body of work. These should note any
new dependencies, new scripts, etc.
New environment variables:
env var: env var detailsNew scripts:
script: script detailsNew dependencies:
dependency: dependency detailsNew dev dependencies:
dependency: dependency details