CODEWITHSUNDEEP
expressmiddleware
m_callens
m_callens

Reputation: 6360

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!

Upvotes: 1

Views: 99

Answers (1)

agm1984
agm1984

Reputation: 17132

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.

Upvotes: 1

Related Questions