OZZIE
OZZIE

Reputation: 7338

Expressjs - Can't set headers after they are sent

I know there are similar questions but they seem to vary a lot in implementation, so it seems there isn't a 'one solution fits all'. I have read all of them but I cannot figure out what is wrong in my case.

server.js

const express = require('express');
const auth = require('basic-auth');

const app = express();
const fetch = require('node-fetch');

app.get('*.gz', (req, res, next) => {
  res.set('Content-Encoding', 'gzip');
  res.set('Content-Type', 'text/javascript');
  next();
});

app.use(express.static('public'));

// For AWS Fargate to check if service is OK
const helthCheck = async (req, res, next) => {
  const returnError = (message) => {
    if (!res.headersSent) {
      res.status(500);
      res.send(`Error: ${message}`);
    }
    next();
  };
  const returnErrorProductApi = (message, prefix) => returnError(`Product API - ${prefix}: ${message}`);
  const apiUrl = 'https://www.censored-fake-domain.com/graphql';
  const query = `{
    filters
  }`;
  const prefix = 'Filters';
  try {
    await fetch(apiUrl, {
      method: 'POST',
      headers: { 'Content-Type': 'application/json' },
      body: JSON.stringify({ query }),
    })
      .catch(() => returnErrorProductApi('Bad response.', prefix))
      .then((result) => result.json())
      .catch(() => returnErrorProductApi('Invalid JSON.', prefix))
      .then(() => {
        if (!res.headersSent) {
          res.send(`OK: ${prefix}`);
        }
        next();
      });
  } catch (error) {
    returnErrorProductApi('Bad response.', prefix);
  }
};

app.use((req, res, next) => {
  if (req.originalUrl === '/favicon.ico') {
    next();
    return;
  }
  if (req.originalUrl.indexOf('/health') !== -1) {
    helthCheck(req, res, next);
    return;
  }

  const credentials = auth(req);
  if (!credentials || credentials.name !== 'censored-username' || credentials.pass !== 'censored-pass') {
    res.status(401);
    res.header('WWW-Authenticate', 'Basic realm="example"');
    res.send('Access denied');
  } else {
    next();
  }
});

// we should allow sub-paths like /lorem also
// but right now we only have 1 route so this is fine
app.get('*', (request, response) => {
  response.sendFile(`${__dirname}/app/index.html`);
});

const listener = app.listen(process.env.PORT, () => {
  console.log(`Your app is listening on port ${listener.address().port}`);
});

Everything works fine except commandline node keeps outputting/spamming:

Error: Can't set headers after they are sent.
    at SendStream.headersAlreadySent (/lorem/node_modules/send/index.js:390:13)
    at SendStream.send (/lorem/node_modules/send/index.js:617:10)
    at onstat (/lorem/node_modules/send/index.js:729:10)
    at FSReqCallback.oncomplete (fs.js:166:5)

Upvotes: 0

Views: 767

Answers (2)

Hamdi Alhamoi
Hamdi Alhamoi

Reputation: 99

you are sending response twice here

if (!res.headersSent) {
      res.status(500);
      res.send('Error: ' + message);
    }

and here

if (!res.headersSent) {
          res.send('OK: ' + prefix);
        }

remove one of them and it will works correctly

Upvotes: 1

Joe Lissner
Joe Lissner

Reputation: 2462

You have more than one area where you are doing res.send(..), followed by next() which then goes on to a route which could also hit res.send(...).

You have to ensure that any time you invoke res.send(...), it doesn't happen again. The easiest way to do this would be to replace any res.send(...) with return res.send(...).

Edit for clarity:

When you call next after res.send(...) (event though you return after it) it will always go on to call the next item, which is trying to send a file.

for example, you have

if (req.originalUrl.indexOf('/health') !== -1) {
  helthCheck(req, res, next);
  return;
}

which you might think would end the request, but within healthCheck, it calls next, which prompts

app.get('*', (request, response) => {
  response.sendFile(`${__dirname}/app/index.html`);
});

to be called, even though healthCheck already sent a response.

Upvotes: 2

Related Questions