Cannot set headers after they are sent to the client when I try to make an update request

Question

I am trying to create a CRUD. I have the UPDATE created, but when I try to request from postman, I get the following error:

Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client at ServerResponse.setHeader (_http_outgoing.js:526:11) at ServerResponse.header (E:\freebooks-core-api\node_modules\express\lib\response.js:771:10) at ServerResponse.send (E:\freebooks-core-api\node_modules\express\lib\response.js:170:12) at ServerResponse.json (E:\freebooks-core-api\node_modules\express\lib\response.js:267:15) at ServerResponse.send (E:\freebooks-core-api\node_modules\express\lib\response.js:158:21) at Object.exports.success (E:\freebooks-core-api\network\response.js:3:6) at E:\freebooks-core-api\components\book\network.js:32:18 at processTicksAndRejections (internal/process/task_queues.js:97:5) { code: 'ERR_HTTP_HEADERS_SENT' } (node:2844) UnhandledPromiseRejectionWarning: Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client at ServerResponse.setHeader (_http_outgoing.js:526:11) at ServerResponse.header (E:\freebooks-core-api\node_modules\express\lib\response.js:771:10) at ServerResponse.send (E:\freebooks-core-api\node_modules\express\lib\response.js:170:12) at ServerResponse.json (E:\freebooks-core-api\node_modules\express\lib\response.js:267:15) at ServerResponse.send (E:\freebooks-core-api\node_modules\express\lib\response.js:158:21) at Object.exports.error (E:\freebooks-core-api\network\response.js:12:6) at E:\freebooks-core-api\components\book\network.js:34:16 at processTicksAndRejections (internal/process/task_queues.js:97:5) (node:2844) 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(). To terminate the node process on unhandled promise rejection, use the CLI flag --unhandled-rejections=strict (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1) (node:2844) [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.

As I read in the error, this comes from the following files:

response.js in my network folder

exports.success = function (req, res, data, status) {
  res.status(status || 200)
    .send({
      error: '',
      data
    })
}

exports.error = function (req, res, error, status, log) {
  console.log(log)
  res.status(status || 500)
    .send({
      error,
      data: ''
    })
}

and network.js in my book (component) folder:

const router = require('express').Router()
const response = require('../../network/response')
const controller = require('./controller')

router.get('/', function (req, res) {
    controller.getBooks()
        .then( data => {
            response.success(req, res, data, 200)
        })
        .catch( err => {
            response.error(req, res, 'Unexpected Error', 500, err)
        })
})

router.post('/', function (req, res) {
  const book = req.body
  console.log(book)
  controller.addBook(book).then( data => {
    response.success(req, res, book, 201)
  })
    .catch(err => {
      response.error(req, res, 'Internal error', 500, err)
    })
})

router.patch('/:id', function(req, res) {
  const { id } = req.params
  console.log(id)
  const book = req.body
  controller.updateBook(id, book)
    .then( data => {
        response.success(req, res, data, 200)
    }).catch( err => {
      response.error(req, res, 'Internal error', 500, err)
    }) 
  res.send('Ok')
})

module.exports = router

but since I am calling the controller and the error is running on that line, this is the code:

const store = require('./store')

function getBooks () {
  return new Promise((resolve, reject) => {

    resolve(store.list())
  })
}

function addBook (book) {
  return new Promise((resolve, reject) => {
    if (!book) {
      reject('I reject')
      return false
    } else {

      resolve(book)
      store.add(book)
    }
  })
}

 function updateBook (id, book) {
  return new Promise( async (resolve, reject) => {
      if (!id || !book) {
          reject('Invalid data')
          return false
      } else {
        const result = await store.update(id, book)
        resolve(result)
      }

  })
}

function deleteBook (id) {
  return new Promise( async (resolve, reject) => {
      if (!id) {
          reject('Invalid data')
          return false
      } else {
        const result = await store.update(id)
        resolve(result)
      }

  })
}

module.exports = {
  addBook,
  getBooks,
  updateBook,
  deleteBook
}

finally the store

const db = require('mongoose')
const Model = require('./model')

db.Promise = global.Promise
db.connect(`mongodb+srv://fewtwtwfwe:efwefwecwecw@ferwrtervsefwg/test?retryWrites=true&w=majority`, {
    useNewUrlParser: true,
    useUnifiedTopology: true
}).then( () => {
    console.log(`Database connected`)
}).catch( err => {
    console.error(err)
})

function addBook(book) {
    const myBook = new Model(book)
    myBook.save()
}

async function getBooks() {
   const books =  await Model.find()
   return books
}

async function updateBook(id, book) {
    const foundBook = await Model.findOne({
        '_id':id
    })
    foundBook.book = book
    const newBook = await foundBook.save()
    return newBook
}

async function deleteBook(id) {
    const foundBook = await Model.findOne({
        '_id':id
    })
}


module.exports = {
    add: addBook,
    list: getBooks,
    update: updateBook,
    delete: deleteBook,
}

I modified the data of the connection to the database, because I am sure that the error does not lie there, I have made other requests and apparently everything is fine. Any idea?

Answer 1

You can only send one response for each incoming request. This particular error message tells you that your code is trying to send two requests. Most often, this occurs because of improper sequencing of code with asynchronous functions. That appears to be the case in this route:

router.patch('/:id', function(req, res) {
  const { id } = req.params
  console.log(id)
  const book = req.body
  controller.updateBook(id, book)
    .then( data => {
        response.success(req, res, data, 200)
    }).catch( err => {
      response.error(req, res, 'Internal error', 500, err)
    }) 
  res.send('Ok')
})

First it executes:

controller.updateBook()

and then while that asynchronous operation is running, it then executes:

res.send('Ok');

Then, sometime later, updateBook() finishes and it calls either the .then() or the .catch() handler and you then try to send another response to the same request.

It appears that you should just remove the res.send('Ok') entirely because you want the response to be sent according to the status of updateBook() in either the .then() or the .catch() handler.

---

On a completely separate topic, it's a bit of an anti-pattern to wrap a promise returning function in another promise like you are doing in updateBook(). I'd suggest you change this:

function updateBook (id, book) {
  return new Promise( async (resolve, reject) => {
      if (!id || !book) {
          reject('Invalid data')
          return false
      } else {
        const result = await store.update(id, book)
        resolve(result)
      }

  })
}

to this:

function updateBook (id, book) {
    if (!id || !book) {
        return Promise.reject('Invalid data');
    } else {
        return store.update(id, book);
    }
}

FYI, the reason it's an anti-pattern is that it's both unnecessary code and people often make mistakes in error handling which is exactly what you did. If store.update() rejected, you did not have a try/catch around the await to catch that error so it would have been lost and your promise would have never been resolved or rejected.

You should change all the similarly structured functions to fix this.