Search code examples
expressmiddleware

Express Middleware Setting Header Error


I'm trying to implementation a fairly simple middleware function to my Express application that just adds a useCache value to the request object being passed to the main handler but for some reason, I'm getting a Can't set headers after they were sent error.

const cacheControl = (req, res, next) => {
  if (lastPulled === null) lastPulled = Date().getDay()
  req.useCache = Date().getDay() === lastPulled
  next()
}

app.use(cacheControl)
app.get('/missions', (req, res) => {
  if (req.useCache) res.status(200).json({ result: cache })

  fetch(dumpUrl)
    .then(data => data.text())
    .then(result => {
      cache = result
      res.status(200).json({ result })
    })
    .catch(e => res.status(500).json({ result: e.message }))
})

I've read that most of the time if the error is produced by the middleware it is due to multiple next() calls, but that doesn't apply here, unless I'm missing something obvious.

When I remove the cacheControl middleware from the application, there is no longer an error, but I can't figure out what in the function is causing the error! Any pointers are helpful!


Solution

  • I'm guessing it's because res.json() is firing twice:

    app.get('/missions', (req, res) => {
      if (req.useCache) res.status(200).json({ result: cache })
    
      fetch(dumpUrl)
        .then(data => data.text())
        .then(result => {
          cache = result
          res.status(200).json({ result })
        })
        .catch(e => res.status(500).json({ result: e.message }))
    })
    
    // if res.useCase is true, set headers and reply
    if (req.useCache) res.status(200).json({ result: cache })
    
    // then fetch and reply again (which generates the error)
    fetch(dumpUrl)
        .then(data => data.text())
        .then(result => {
          cache = result
          res.status(200).json({ result })
    

    Change it to this to utilize explicit return

    app.get('/missions', (req, res) => {
      if (req.useCache) return res.status(200).json({ result: cache })
    
      return fetch(dumpUrl)
        .then(data => data.text())
        .then(result => {
          cache = result
          res.status(200).json({ result })
        })
        .catch(e => res.status(500).json({ result: e.message }))
    })
    

    The nature of the error is similar to when you do this:

    problem

        function problem() {
            if (true === true) console.log('send problem')
            console.log('send garbage by accident')
        }
        console.log(problem())

    solution

        function solution() {
            if (true === true) return console.log('send solution')
            return console.log('send nothing')
        }
        console.log(solution())

    return is how you exit a function. Your issue is that your code was checking the if condition, but then continuing past it because it wasn't told to stop once it found that condition.

    The old way or less terse way to write your function would be like:

    app.get('/missions', (req, res) => {
      if (req.useCache) {
        res.status(200).json({ result: cache })
      } else {
        fetch(dumpUrl)
          .then(data => data.text())
          .then(result => {
            cache = result
            res.status(200).json({ result })
          })
          .catch(e => res.status(500).json({ result: e.message }))
      }
    })
    

    Without the else in there, it executes every if statement it comes across until it reaches the end of the function, unless you use the return keyword as the cue to exit right there.

    Keep in mind, using return inside a .then() function will resolve the promise, it won't exit from the upper scope if there are more .then()s chained on.