express crashing during authentication-CodePudding

Seems we all run into this issue at some point during our learning path. Perhaps I need to strengthen my vanilla JS skills- anyways to the point--

this is my stack trace:

Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client at new NodeError (node:internal/errors:371:5) at ServerResponse.setHeader (node:_http_outgoing:576:11)

this is my code:

router.post("/login", async (req, res) => {
    try {
        const user = await User.findOne({ username: req.body.username });
        !user && res.status(400).json("Wrong credentials.");

        const validated = await bcrypt.compare(req.body.password, user.password);
        !validated && res.status(400).json("Wrong credentials.");

        res.status(200).json(user);
    } catch(err) {
        res.status(500).json(err);
    }
});

I'm getting my response back when I incorrectly guess either my userName, or password, but it appears it's also trying to fire another error and thus is crashing my express server. Once a response is sent, why is it sending another response?

(sorry if this is a duplicate question, I've read quite a few posts of others with this issue.)

CodePudding user response：

You need to add return whenever you want the function to stop proceeding further.

router.post("/login", async (req, res) => {
    try {
        const user = await User.findOne({ username: req.body.username });
        if (!user) {
            return res.status(400).text("Wrong credentials.");
        }


        const validated = await bcrypt.compare(req.body.password, user.password);
        if (!validated) {
           return res.status(400).text("Wrong credentials.");
        }

        res.status(200).json(user);
    } catch(err) {
        res.status(500).text(err);
    }
});

Please also note there is .json only accept object, not string

CodePudding user response：

The error ERR_HTTP_HEADERS_SENT occurs when your code tries to call res.json() more than once for the same request. This is an error. You only get to send one response for each incoming request. To fix, you need to either use if/else or return after each error you send so that you stop executing the rest of your handler when you send a response so the code doesn't continue to execute the other res.json() calls.

I'd suggest this kind of implementation which centralizes the sending of responses into one place for success and one place for errors which can generally make things cleaner and is not susceptible to the problem you ran into.

class MyError {
    constructor(msg, status) {
        super(msg)
        this.status = status;
    }
}

router.post("/login", async (req, res) => {
    try {
        const user = await User.findOne({ username: req.body.username });
        if (!user) {
            throw new MyError("User not found.", 401);
        }

        const validated = await bcrypt.compare(req.body.password, user.password);
        if (!validated) {
            throw new MyError("Wrong credentials.", 401);
        }
        res.status(200).json(user);
    } catch(err) {
        const status = err.status || 500;
        res.status(status).json({message: err.message});
    }
});

Also, note that this code of yours:

res.status(500).json(err);

Also has issues because err is an Error object which does not make all of its properties enumerable and thus they don't all go into the JSON you're trying to send. For that reason, I constructed my own error object and sent that which won't have that problem.

Also, note that the http status code 400 is for a bad request. This means that the request is malformed - "The request could not be understood by the server due to malformed syntax.". This is not the correct status for a user not found or a wrong password. In both cases, the request is understood just fine by the server, it just has the wrong data in it. The wrong password should definitely be a different 4xx error. I would choose 403 (forbidden) for both the user and the password being wrong. Normally, you might think you would use 401 (unauthorized), but you have to be careful because in some circumstances, this will cause the browser to prompt on its own for the a username and password which is not what you usually want. You can read more about this http status issue here.