(minor issue) Should bodyParser.json() throw SyntaxError on invalid json?

[MeLlamoPablo]

Consider the following example:

"use strict";

const express = require("express");
const bodyParser = require("body-parser");

const app = express();

app.use(bodyParser.json());

app.post("/", (req, res) => {
    let response = {
        "message": `Your parameter is ${req.body.myParameter}`
    };

    res.status(200).send(JSON.stringify(response));
});

app.use((err, req, res, next) => {
    if (err instanceof SyntaxError) return res.status(400).send(JSON.stringify({
        error: "The body of your request is not valid json!"
    }))

    console.error(err);
    res.status(500).send();
});

app.listen(3000, () => console.log("ready!"));

This app expects JSON as input, and since bodyParser.json() throws a SyntaxError on invalid json, the app would send 500 if the user sent invalid json if it wasn't for the conditional sentence checking if the error is in fact a SyntaxError.

However, isn't there a better way to handle this situation? At first, I thought that this error handler would catch any SyntaxError on my code, but then I remembered that SyntaxErrors are not produced at runitme and that my code wouldn't even run with one.

But the way I see it, this is potentially bad, as another module could theoretically throw a SyntaxError as well, that would also be treated as "invalid json" even though that wouldn't be the cause. I can't really think of other sources of SyntaxError other than eval("shitty code"), but if anyone's using eval, at that point they're asking for it.

However, the way I see it, something like BodyParseError would be more elegant and less confusing.

[dougwilson]

Hi! This is a duplicate of #122 . If you can, please have a read through the entire conversation and feel free to expand it with additional discussion or a proposal; a custom class to instanceof is not going to happen (and was discussed there).

Ultimately you'll notice that if you remove your custom error handler completely, you'll see Express will already return 400 for a bad JSON parse, but a 500 on your eval example. This is because it looks at the statusCode property on the error object. This module thows a lot of errors (listed in the errors section in the readme), several of which are not 400.