0

I am using the Model.findOne method from mongoose to find and add some new data into a document. What kind of data is added is based on conditional if statements. See below:

companyTotal.findOne({companyName: "xyz"}, function (err, doc) {
        if (err) {
            sendJsonResponse(res, 400, err)
        } else if (doc) {

                if (req.body.q1 === "poor") {
                    doc.poor += 1;
                } else if (req.body.q1 === "okay") {
                    doc.okay += 1;
                } else if (req.body.q1 === "well") {
                    doc.well += 1;
                } else if (req.body.q1 === "very well") {
                    doc.veryWell += 1;
                } else {
                    sendJsonResponse(res, 401, {"message": "Wrong data entry."})
                }
        }
        doc.save(function (err, data) {
            if (err) {
                sendJsonResponse(res, 400, err)
            } else {
                sendJsonResponse(res, 200, data);
            }
        });

    });

If none of the conditional statements are met I want to end the request and send an error message back. I am using the sendJsonResponse function to handle any kind responses that need to be sent to the user along with the status codes.

function sendJsonResponse(res, status, content) {
  res.status(status);
  res.json(content);
}

Based on my knowledge res.json calls res.end(); which then ends the request. But everytime the condition is not met it displays the error message BUT still runs the doc.save() method and adds the data into the document.

What am I doing wrong? How can I end a request within if statements if conditions are not met?

Skywalker
  • 4,984
  • 16
  • 57
  • 122
  • @AJS its already inside it. Its placed right before the closing brackets for `companyTotal.findOne().` – Skywalker Dec 02 '16 at 10:05
  • 1
    Try add "return" before res.json(content); – Tan Le Dec 02 '16 at 10:08
  • Try this: `function sendJsonResponse(res, status, content) { return res.status(status).json(content); }` – Niezborala Dec 02 '16 at 10:09
  • @TanLe I tried the return statement. It still adds the data and gives me `can't set headers after they are sent` error. – Skywalker Dec 02 '16 at 10:15
  • @Niezborala I tried the return statement. It still adds the data and gives me `can't set headers after they are sent` error. – Skywalker Dec 02 '16 at 10:16
  • @Skywalker I think you can try with this `return sendJsonResponse(res, 400, err)` or `sendJsonResponse(res, 400, err); return ;` – Tan Le Dec 02 '16 at 10:26
  • @Skywalker because in your function `sendJsonResponse` you send two responses. One with status, second one with json. Try to change this two lines to one: `return res.status(status).json(content);` – Niezborala Dec 02 '16 at 10:31

2 Answers2

0

define flag variable as

flag=0 ; 

Then

else {
        flag++;
        sendJsonResponse(res, 401, {"message": "Wrong data entry."})
      }

Then

if (flag ==0 ){  
doc.save(function (err, data) {
            if (err) {
                sendJsonResponse(res, 400, err)
            } else {
                sendJsonResponse(res, 200, data);
            }
        });
 } 
AJS
  • 953
  • 7
  • 21
  • Thank you so much for the answer. I will try this and let you know but I really want to know why calling `sendJsonResponse` is not ending the request. In theory it should as its calling the `res.json`. Any thoughts on this? Thanks again. – Skywalker Dec 02 '16 at 10:06
  • @Skywalker about the reason , have a question about that in here, you can have a look : http://stackoverflow.com/questions/37314598/in-express-js-why-does-code-after-res-json-still-execute – Tan Le Dec 02 '16 at 10:13
  • 1
    @Skywalker it is ending the request but due to node asynchronous nature doc.save() getting called before response end. Btw does that worked ? – AJS Dec 02 '16 at 10:35
  • Still having issue ? – AJS Dec 02 '16 at 10:45
  • 1
    @AJS I tried your approach and its giving me the same issue. – Skywalker Dec 02 '16 at 10:48
  • 1
    ohhh thats quiet viyard. Do u have team viewer ? – AJS Dec 02 '16 at 10:50
0

In my opinion it should look like this:

function sendJsonResponse(res, status, content) {
  return res.status(status).json(content); // you can send response only one 
}

and:

companyTotal.findOne({companyName: "xyz"}, function (err, doc) {
    if (err) {
        return sendJsonResponse(res, 400, err);
    } else if (doc) {
            if (req.body.q1 === "poor") {
                doc.poor += 1;
            } else if (req.body.q1 === "okay") {
                doc.okay += 1;
            } else if (req.body.q1 === "well") {
                doc.well += 1;
            } else if (req.body.q1 === "very well") {
                doc.veryWell += 1;
            } else {
                return sendJsonResponse(res, 401, {"message": "Wrong data entry."});
            }
    }
    doc.save(function (err, data) {
        if (err) {
            return sendJsonResponse(res, 400, err);
        } else {
            return sendJsonResponse(res, 200, data);
        }
    });
});
Niezborala
  • 1,857
  • 1
  • 18
  • 27
  • Thank you for the answer. I tried it but it still gives the same issue. I think the issue is that I use `sendJsonResponse` when my condition is not met but somehow the request does not stop there and goes onto `doc.save` which also runs `sendJsonResponse`` and thats what causing the 2nd response to be sent. The request should end when the first `sendJsonResponse` runs. – Skywalker Dec 02 '16 at 10:38
  • @Skywalker thats why I add `return` statements before all `sendJsonResponse` call. – Niezborala Dec 02 '16 at 10:40
  • I copy pasted your answer and tried it. It still giving the same error and issues. – Skywalker Dec 02 '16 at 10:49