body-parser: Difficult to determine that an error is the result of bad JSON
Having SyntaxErrors consistently returned (#119) and a status code provided means we can fairly reliably detect errors in express middleware that are the result of bad JSON.
app.use(function(err, req, res, next) {
if (err instanceof SyntaxError && err.status === 400) {
console.error('Bad JSON');
}
});
This test could theoretically give false positives though, if some other middleware were to throw something similar. Imagine we’re using body-parser first and then something else to validate the body has the structure we’re expecting, for example.
Is there any way to detect JSON parsing error with 100% reliability with the library as it is now? Is there something you could (perhaps in 2.0) to make this a little easier?
About this issue
- Original URL
- State: closed
- Created 9 years ago
- Reactions: 6
- Comments: 26 (9 by maintainers)
Links to this issue
Commits related to this issue
- Include the "type" property on all generated errors closes #122 — committed to expressjs/body-parser by dougwilson 7 years ago
- Include the "type" property on all generated errors closes #122 — committed to expressjs/body-parser by dougwilson 7 years ago
- bug: body-parser failures would throw bare exceptions/500s. now 400. * upgrade body-parser to actually be able to detect its failure signature. https://github.com/expressjs/body-parser/issues/122 *... — committed to getodk/central-backend by issa-tseng 7 years ago
- Handle malformed JSON body Based on https://github.com/expressjs/body-parser/issues/122 — committed to EMBL-EBI-SUBS/json-schema-validator by deleted user 6 years ago
All errors include a
typeproperty from 1.18.0 release onwards. For parse failure,err.type === 'entity.parse.failed'.That’s fine, I trust your judgement 😃
I’ll stick with
Just adding another option here: instead of using the JSON body parser, you can parse the body as text:
And then parse the text as JSON in your request handlers. Then you can handle errors however you like:
Probably the best, though without access, you cannot do
instanceofagainst it.We already do this, we just call the property
bodyright now, and you didn’t like that above, so weird to propose it anyway 😃That wouldn’t make any sense, because it’s not how Express middleware works. You can easily wrap the body parsing middleware if you really want to know the error came from this middleware, and is the recommend way to determine error source:
Any other suggestions 😃 ? I don’t see anything to act on currently from that list.
True. Still seems messy though.
AFAIK:
This should shut up the typescript 😃