1

I have recently been developing a MERN application and I have recently came into the trouble that express is saying that I am setting headers after they are sent.

I am using mongo db and trying to update a user profile.

I have tried to comment out my res.send points to find the issue but I have failed to do so.

Here is my post method for updating the user profile:

app.post("/api/account/update", (req, res) => {
    const { body } = req;
    // Validating and Checking Email
    if (body.email) {
      var email = body.email;
      email = email.toLowerCase();
      email = email.trim();
      body.email = email;
      User.find(
        {
          email: body.email
        },
        (err, previousUsers) => {
          if (previousUsers.length > 0) {
            return res.send({
              success: false,
              message:
                "Error: There is already another account with that email address"
            });
          } else {
          }
        }
      );
    }
    // Validating Names Function
    function checkName(name) {
      var alphaExp = /^[a-zA-Z]+$/;
      if (!name.match(alphaExp)) {
        return res.send({
          success: false,
          message: "Error: Names cannot contain special characters or numbers"
        });
      }
    }
    checkName(body.firstName);
    checkName(body.lastName);

    // Making sure that all fields cannot be empty
    if (!body.email && !body.firstName && !body.lastName) {
      return res.send({
        success: false,
        message: "Error: You cannot submit nothing"
      });
    }
    // Getting User ID from the current session
    UserSession.findById(body.tokenID, function(err, userData) {
      // Finding User ID using the current users session token
      if (userData.isDeleted) {
        return res.send({
          success: false,
          message:
            "Error: Session token is no longer valid, please login to recieve a new one"
        });
      }
      // Deleting the token ID from the body object as user table entry doesnt store tokens
      delete body.tokenID;
      // Finding the user profile and updating fields that are present
      User.findByIdAndUpdate(userData.userId, body, function(err, userInfo) {
        if (!err) {
          return res.send({
            success: true,
            message: "Success: User was updated successfully"
          });
        }
      });
    });
  });

This is the call that I am doing to the backend of the site: 

onUpdateProfile: function(fieldsObj) {
    return new Promise(function(resolve, reject) {
      // Get Session Token
      const obj = getFromStorage("the_main_app");
      // Defining what fields are getting updated
      fieldsObj.tokenID = obj.token;
      // Post request to backend
      fetch("/api/account/update", {
        method: "POST",
        headers: {
          "Content-Type": "application/json"
        },
        body: JSON.stringify(fieldsObj)
      })
        .then(res => {
          console.log("Verify Token - Res");
          return res.json();
        })
        .then(json => {
          console.log("Verify Token JSON", json);
          if (json.success) {
            window.location.href = `/manage-account?success=${json.success}`;
          } else {
            window.location.href = `/manage-account?success=${json.success}`;
          }
        });
    });
  }

Here is my error message that I am getting: 

Error: Can't set headers after they are sent.
    at validateHeader (_http_outgoing.js:491:11)
    at ServerResponse.setHeader (_http_outgoing.js:498:3)
    at ServerResponse.header (C:\Users\kieran.corkin\Desktop\Projects\Mern Template Final\mern-cra-and-server\server\node_modules\express\lib\response.js:767:10)
    at ServerResponse.send (C:\Users\kieran.corkin\Desktop\Projects\Mern Template Final\mern-cra-and-server\server\node_modules\express\lib\response.js:170:12)
    at ServerResponse.json (C:\Users\kieran.corkin\Desktop\Projects\Mern Template Final\mern-cra-and-server\server\node_modules\express\lib\response.js:267:15)
    at ServerResponse.send (C:\Users\kieran.corkin\Desktop\Projects\Mern Template Final\mern-cra-and-server\server\node_modules\express\lib\response.js:158:21)
    at C:\Users\kieran.corkin\Desktop\Projects\Mern Template Final\mern-cra-and-server\server\routes\api\account.js:270:22
    at C:\Users\kieran.corkin\Desktop\Projects\Mern Template Final\mern-cra-and-server\server\node_modules\mongoose\lib\model.js:4641:16
    at process.nextTick (C:\Users\kieran.corkin\Desktop\Projects\Mern Template Final\mern-cra-and-server\server\node_modules\mongoose\lib\query.js:2624:28)
    at _combinedTickCallback (internal/process/next_tick.js:131:7)
    at process._tickCallback (internal/process/next_tick.js:180:9)
[nodemon] app crashed - waiting for file changes before starting...

Can anyone help me with this?

EDIT

I have changed my code, this seems to now work however I feel like its a little messy when put together. Any refactoring tips?

Code: 

app.post("/api/account/update", (req, res) => {
    // Preform checks on data that is passed through
    const { body } = req;
    var messages = {
      ExistedUser:
        "Error: There is already another account with that email address",
      NameFormat: "Error: Names cannot contain special characters or numbers",
      BlankInputs: "Error: You cannot submit nothing",
      accountLoggedOut:
        "Error: Session token is no longer valid, please login to recieve a new one",
      successfullyUpdated: "Success: User was updated successfully"
    };
    var usersFound;
    if (body.email) {
      var email = body.email;
      email = email.toLowerCase();
      email = email.trim();
      body.email = email;
      User.find(
        {
          email: body.email
        },
        (err, UserCount) => {
          usersFound = UserCount;
        }
      );
    }
    function capitalize(text) {
      return text.replace(/\b\w/g, function(m) {
        return m.toUpperCase();
      });
    }
    if (body.firstName) {
      body.firstName = capitalize(body.firstName);
    }
    if (body.lastName) {
      body.lastName = capitalize(body.lastName);
    }

    //Making sure that all fields cannot be empty
    if (!body.email && !body.firstName && !body.lastName) {
      return res.send({
        success: false,
        message: messages.BlankInputs
      });
    }
    // Getting User ID from the current session
    UserSession.findById(body.tokenID, function(err, userData) {
      // Finding User ID using the current users session token
      if (userData.isDeleted) {
        return res.end({
          success: false,
          message: messages.accountLoggedOut
        });
      }
      if (userData) {
        // Deleting the token ID from the body object as user table entry doesnt store tokens
        delete body.tokenID;
        // Finding the user profile and updating fields that are present
        User.findByIdAndUpdate(userData.userId, body, function(err, userInfo) {
          if (userInfo) {
            if (!usersFound.length > 0) {
              return res.send({
                success: true,
                message: messages.successfullyUpdated
              });
            } else {
              return res.send({
                success: false,
                message: messages.ExistedUser
              });
            }
          }
        });
      }
    });
  });
Kieran Corkin
  • 41
  • 6
  • The problem that your returns are not grouped, when the callback is calling within the first call to the bank it does not end the request, on the contrary the node continues to transpilando the lines of code below and this generates the error, the same goes for the others calls to the database, my advice is to you refactor all calls to database in promises or group all the calls within the callback of each one. – Chance Jan 15 '19 at 14:55
  • I have just edited my post with the updated version of the code which seems to work a little better. My issue now is that it looks messy and I dont like messy code. Anyone able to help refactor? – Kieran Corkin Jan 15 '19 at 14:57

3 Answers3

0

You're calling res.send() twice. res.send() ends the process. You ought to refactor such that you call res.write() and only call res.send() when you're done.

This StackOverflow link describes the difference in more detail. What is the difference between res.send and res.write in express?

Chris Adams
  • 1,376
  • 1
  • 8
  • 15
  • Where would you say that I am calling res.send twice? Is it the point that Andrew Birks pointed out? – Kieran Corkin Jan 15 '19 at 13:58
  • In short, yes, I think so. To test, stick a `console.log` type message inside the `checkName` function. It looks to me you call that function with two different values. However, once it has run once you've closed the req/res loop because you used res.send(). Then when the function is called a second time, it throws the exception. – Chris Adams Jan 15 '19 at 14:09
  • Hi, I have now removed the checkName function and instead of checking and then ending, i just change their input so there is no need to send an error. I am still recieving this error ```function capitalize(text) { return text.replace(/\b\w/g, function(m) { return m.toUpperCase(); }); } if (body.firstName) { body.firstName = capitalize(body.firstName); console.log(body.firstName); } if (body.lastName) { body.lastName = capitalize(body.lastName); } ``` – Kieran Corkin Jan 15 '19 at 14:18
  • The same one - Cant send headers one – Kieran Corkin Jan 15 '19 at 14:39
  • OK, then there is still a flaw in the logic of the function. You must still be calling res.send() more than once. There are lots of res.send() calls, but what I cannot easily work out is where the path/problem is in your logic to see where subsequent calls to res.send() would occur. – Chris Adams Jan 15 '19 at 14:49
  • Once you have called res.send(), you cannot call it again. – Chris Adams Jan 15 '19 at 14:49
  • I have posted the updated code in my post - Seems to be working but doesnt seem to be the "right way" for it to be working – Kieran Corkin Jan 15 '19 at 15:03
  • I'm glad you have it working. Personally, and this is no criticism, I'd look to refactor that `app.post("/api/account/update", (req, res) => {` function. There's a lot going on in there, but I'd look to build up an object that is passed through those checking functions and once you get to the end, call a single `res.send(myObj);`. Alternatively, have one `res.send()` at the end of the function and elsewhere use `res.write()`. – Chris Adams Jan 15 '19 at 15:13
0

I believe this is happening, as you're trying to send a response after the first / initial response has already been sent to the browser. For example:

checkName(body.firstName);
checkName(body.lastName);

Running this function twice is going to try and yield 2 different "response" messages.

The product of a single route, should ultimately be a single response.

Andrew Birks
  • 792
  • 9
  • 26
0

Thanks for all your help on this issue.

Here is my final code that allowed it to work.

I have also tried to "refactor" it too. Let me know if you'd do something else.

app.post("/api/account/update", (req, res) => {
    const { body } = req;
    console.log(body, "Logged body");
    // Defining objects to be used at the end of request
    var updateUserInfo = {
      userInfo: {},
      sessionToken: body.tokenID
    };
    var hasErrors = {
      errors: {}
    };

    // Checking that there is at least one value to update
    if (!body.email && !body.firstName && !body.lastName) {
      var blankError = {
        success: false,
        message: "Error: You cannot change your details to nothing"
      };
      hasErrors.errors = { ...hasErrors.errors, ...blankError };
    } else {
      console.log("Normal Body", body);
      clean(body);
      console.log("Cleaned Body", body);
      updateUserInfo.userInfo = body;
      delete updateUserInfo.userInfo.tokenID;
    }
    // Function to check if object is empty
    function isEmpty(obj) {
      if (Object.keys(obj).length === 0) {
        return true;
      } else {
        return false;
      }
    }

    // Function to remove objects from body if blank
    function clean(obj) {
      for (var propName in obj) {
        if (obj[propName] === "" || obj[propName] === null) {
          delete obj[propName];
        }
      }
    }

    // Checking and Formatting Names Given
    function capitalize(text) {
      return text.replace(/\b\w/g, function(m) {
        return m.toUpperCase();
      });
    }
    if (body.firstName) {
      body.firstName = capitalize(body.firstName);
    }
    if (body.lastName) {
      body.lastName = capitalize(body.lastName);
    }

    // Checking and formatting email
    if (body.email) {
      body.email = body.email.toLowerCase();
      body.email = body.email.trim();

      // Checking for email in database
      User.find({ email: body.email }, (err, EmailsFound) => {
        if (EmailsFound.length > 0) {
          var EmailsFoundErr = {
            success: false,
            message: "There is already an account with that email address"
          };
          hasErrors.errors = { ...hasErrors.errors, ...EmailsFoundErr };
        }
      });
    }
    // Getting User Session Token
    UserSession.findById(updateUserInfo.sessionToken, function(err, userData) {
      // Finding User ID using the current users session token
      if (userData.isDeleted) {
        var userDeletedError = {
          success: false,
          message:
            "Your account is currently logged out, you must login to change account details"
        };
        hasErrors.errors = { ...hasErrors.errors, ...userDeletedError };
      } else {
        // Finding the user profile and updating fields that are present
        User.findByIdAndUpdate(
          userData.userId,
          updateUserInfo.userInfo,
          function(err, userInfo) {
            // userInfo varable contains user db entry
            if (err) {
              var updateUserError = {
                success: false,
                message: "Error: Server Error"
              };
              hasErrors.errors = {
                ...hasErrors.errors,
                ...updateUserError
              };
            }
            if (isEmpty(hasErrors.errors)) {
              res.send({
                success: true,
                message: "Success: You have updated your profile!"
              });
            } else {
              res.send({
                success: false,
                message: hasErrors.errors
              });
            }
          }
        );
      }
    });
  });
Kieran Corkin
  • 41
  • 6