0

I've rewritten the following function about 6 different times and am still getting a "Cannot set headers after they are sent to the client" error. I have found several posts on the topic of promises but still cant figure it out:

  1. Error: Can't set headers after they are sent to the client
  2. Cannot set headers after they are sent to the client
  3. Error: Setting header after it is sent - Help me understand why?

The following function is for a forum and is triggered when a comment is submitted. It check to see that the forum post exists, than if a parent comment exists (in the case it is a subcomment). I am using firestore.

index.js

const functions = require('firebase-functions');
const app = require('express')();
const {postOneForumComment,
} = require('./handlers/forumPosts');

app.post('/forumPost/:forumPostId/:parentId/comment', FBAuth, postOneForumComment);

exports.api = functions.https.onRequest(app);

forumPosts.js

// submit a new comment
exports.postOneForumComment = (req, res) => {
  if (req.body.body.trim() === '')
  return res.status(400).json({ comment: 'Must not be empty' });

 const newComment = {
   body: req.body.body,
   forumPostId: req.params.forumPostId,
   parentId: req.params.parentId
 };

 db.doc(`/forumPosts/${req.params.forumPostId}`)                  //check to see if the post exists
   .get()
   .then((doc) => {
     if (!doc.exists) {
       return res.status(404).json({ error: 'Post not found' });
     }
     else if (req.params.forumPostId !== req.params.parentId) {   //check to see if the comment is a subcomment
       return db.doc(`/forumComments/${req.params.parentId}`)     //check to see if the parent comment exists
         .get();
     }
     return "TopLevelComment";
   })
   .then((data) => {
     if (data === 'TopLevelComment' || data.exists) {
       return db.collection('forumComments').add(newComment);     //post the comment to the database
     }
     return res.status(500).json({ error: 'Comment not found' });
   })
   .then(() => {
     res.json(newComment);
   })
   .catch((err) => {
     console.log(err.message);
     res.status(500).json({ error: 'somethign went wrong' });
   });
 };

ERROR:

(node:29820) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1) (node:29820) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

Matt Wolfe
  • 13
  • 1
  • 4
  • The error is simple: You sent the response once and now you're again trying to send a response which isn't allowed in Express. Why is this happening? Once the statement `return res....` is executed (means you've sent the response) in any of `.then()` method before the last `.then()`, you'll get this error. – Ajay Dabas Jan 11 '20 at 18:15
  • For example, say `if (!doc.exists)` is true and `return res.status(404).json({ error: 'Post not found' });` is executed. Now you've sent the response. But in next `.then((data)=>{...})`,this `if (data === 'TopLevelComment' || data.exists)` is probably going to be false and you'll execute `return res.status(500).json({ error: 'Comment not found' });`. So, you're trying to send response again but it's already sent and hence the error. – Ajay Dabas Jan 11 '20 at 18:21
  • @AjayDabas, I don't know how I missed that! your help is greatly appreciated. If, you don'd mind me asking, is there a simple way to check if the res has been returned in the second .then()? – Matt Wolfe Jan 11 '20 at 18:27
  • Well, a simple workaround for this is to create a new variable `flag` (at the place where you've defined `newComment`) and initialize it with `false`. Now, if you want to send a response and return in `.then()`, set it to `true` so that in subsequent `.then()` methods, you can compare `flag==true` to check if the response has been already sent. Of course, this is just a quick workaround to solve your issue. A much more elegant solution is to change your code and use await/async. – Ajay Dabas Jan 11 '20 at 18:50

2 Answers2

1

There are two ways of using promises. Either you use the then/catch callbacks or you can use async/await to allow you to write them synchronously.

then/catch method

// Some code before promise

somePromise.then(() => {
    // Some code after promise action is successful
}).catch(err => {
    // Some code if promise action failed
})

// Some code after promise definition you think should run after the above code
// THIS IS WHAT IS HAPPENING WITH YOUR CODE

async/await method

// Some code before promise
await somePromise;
// Some code after promise action is successful

The latter approach was introduces to avoid the callback hell problem and it seems that's where your error is arising from.

When using callback callbacks you must make sure that nothing is defined after the promise definition else it will run before the promise resolves (Which is counter-intuitive since placing code B after code B should make A run before B)

Your error is because your callbacks are probably running AFTER the response has been sent and express does not allow you to send multiple responses for a request. You should make sure that where ever res.send or res.json is being called exist within the callback.

This article should help you understand promises much better...

Hope this helps...

Kwame Opare Asiedu
  • 2,110
  • 11
  • 13
0

For anyone who stumbles upon this here is a working solution using Promise.all to make sure all promises are fulfilled before moving on. It is not the prettiest function and I plan on going back and turning it into an async/await ordeal per @kwame and @Ajay's recommendation... but for now it works.

// post a comment
// TODO: turn into async await function
exports.postOneForumComment = (req, res) => {
  if (req.body.body.trim() === '') return res.status(400).json({ comment: 'Must not be empty' });

  const newComment = {
    body: req.body.body,
    createdAt: new Date().toISOString(),
    forumPostId: req.params.forumPostId,
    parentId: req.params.parentId,
    username: req.user.username,
    userImage: req.user.imageUrl,
    likeCount: 0
  };


  const parentPost =
    db.doc(`/forumPosts/${req.params.forumPostId}`).get()
       .then((doc) => {
         if (!doc.exists) {
           res.status(404).json({ error: 'Post not found' });
           return false;
         }
         return true;
       })
       .catch((err) => {res.status(500).json({ error: 'something went wrong while checking the post' });});

  const parentComment =
    req.params.forumPostId === req.params.parentId ? true :
      db.doc(`/forumComments/${req.params.parentId}`).get()
       .then((doc) => {
         if (!doc.exists) {
           res.status(404).json({ error: 'Comment not found' });
           return false;
         }
         if (doc.forumPostId !== req.params.forumPostId) {
           res.status(404).json({ error: 'Comment is not affiliated with this post' });
           return false;
         }
         return true;
        })
        .catch((err) => {res.status(500).json({ error: 'something went wrong while checking the comment' });});

    Promise.all([parentPost, parentComment])
    .then((values) => {
      if (values[0] && values[1]) {
        return db.collection('forumComments')
        .add(newComment)
        .then(() => {
          res.json(newComment);
        });
      }
      return console.log("there was an error");
    })
    .catch((err) => {
        res.status(500).json({ error: 'somethign went wrong with the submission' });
    });
 };
Matt Wolfe
  • 13
  • 1
  • 4