Middleware that attempts to rewrite response & headers gets `ERR_HTTP_HEADERS_SENT` error

[wbobeirne]

Bug report

I just updated my webpack-dev-server from 3.11.0 to 4.9.2 and a custom middleware I had written now causes an error. The middleware simulates a Cloudflare worker we have in production that pre-fills a <script> tag with some API information so that our initial render doesn't have to do a fetch to render the user's account information. I've dumbed it down a little, but here's the gist of it:

const expressModifyResponse = require('express-modify-response');
const fetch = require("node-fetch");
const JSDOM = require("jsdom").JSDOM;

function setupMiddlewares(middlewares, devServer) {
    const { app } = devServer;

    // Simulate cloudflare worker that pre-fetches API information and inserts into script tag.
    middlewares.unshift({
      name: 'pre-fetch-api',
      middleware: expressModifyResponse(
        (req, res) => {
          // Only modify HTML responses.
          const contentType = res.get('Content-Type') || '';
          return contentType.includes('text/html');
        },
        async (req, res, body) => {
          // Only modify if the json script is in the page.
          const html = body.toString();
          const jsdom = new JSDOM(body.toString());
          const scriptEl = jsdom.window.document.getElementById('__prefetched_json__');
          if (!scriptEl) {
            return html;
          }

          // Attempt to fetch account JSON and inject, if anything goes wrong
          // inject nothing to fall back to typical frontend API request.
          const apiRes = await fetch(`${process.env.API_URL}/api/v1/account`);
          scriptEl.innerHTML = JSON.stringify(await apiRes.json());
          const newHtml = jsdom.serialize();

          // Set headers to not cache HTML since it has dynamic content, and to update
          // the content length with the new length.
          res.setHeader("Cache-Control", "no-cache, no-store");
          res.setHeader("Content-Length", newHtml.length);

          return newHtml;
        },
      ),
    });
  }
}

It's very possible that express-modify-response is doing something abnormal here and that there may be a better way to do this, but given that I'm unshifting the middleware, I don't see a way I could act any earlier in applying my middleware. So if the response is already being sent before this middleware can act, I'm not sure how else I could possibly modify it.

Actual Behavior

Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client
    at new NodeError (node:internal/errors:371:5)
    at ServerResponse.setHeader (node:_http_outgoing:576:11)
    ...

Expected Behavior

The response body and headers should be correctly modified for HTML responses.

How Do We Reproduce?

I can try to throw together a minimal reproduction if it seems like a real issue, but I might just be doing something obviously wrong so I'll hold off on that.

Please paste the results of npx webpack-cli info here, and mention other relevant information

❯ npx webpack-cli info

  System:
    OS: macOS 12.2.1
    CPU: (10) arm64 Apple M1 Pro
    Memory: 90.16 MB / 32.00 GB
  Binaries:
    Node: 16.14.0 - ~/.nvm/versions/node/v16.14.0/bin/node
    Yarn: 1.22.11 - /opt/homebrew/bin/yarn
    npm: 8.3.1 - ~/.nvm/versions/node/v16.14.0/bin/npm
  Browsers:
    Chrome: 103.0.5060.53
    Firefox: 101.0
    Safari: 15.3
  Packages:
    babel-loader: ^8.0.6 => 8.2.2
    clean-webpack-plugin: ^3.0.0 => 3.0.0
    copy-webpack-plugin: ^9.0.1 => 9.0.1
    css-loader: ^5.0.1 => 5.0.1
    file-loader: ^6.2.0 => 6.2.0
    fork-ts-checker-webpack-plugin: ^6.0.5 => 6.0.5
    html-loader: ^3.1.2 => 3.1.2
    html-webpack-plugin: ^5.5.0 => 5.5.0
    less-loader: ^7.1.0 => 7.1.0
    react-hot-loader: ^4.12.18 => 4.13.0
    string-replace-loader: ^3.0.1 => 3.0.1
    ts-loader: ^9.3.1 => 9.3.1
    url-loader: ^4.1.1 => 4.1.1
    webpack: ^5.73.0 => 5.73.0
    webpack-bundle-analyzer: ^4.4.1 => 4.4.1
    webpack-cli: ^4.10.0 => 4.10.0
    webpack-dev-server: ^4.9.2 => 4.9.2

[alexander-akait]

Can you provide more code, sounds like body was sent, but you try to send headers after it

[RobinYang11]

hava you fixed it ?

[alexander-akait]

Please read https://github.com/webpack/webpack-dev-server/issues/4508#issuecomment-1171602497

[JSteunou]

Same issue here. We are using an old but yet very useful express mw https://github.com/axiomzen/express-interceptor

You can test it with the sample exemple.

We updated from 4.5 to latest 4.11.1, I'm trying to find the latest working version.

This is either a breaking change, or a regression.

Edit: ~~4.7 introduce this regression.~~ I think this issue happens because we are doing async things during the interception. Due to the new architecture headers are set & response is ended by the time we want to tamper the response. This was not the case with the old way.

Edit 2: I jump to fast on conclusion, I have the issue with 4.6 too. If I stick to 4.5 all work again.

[alexander-akait]

Thank you reproducible test repo, I will look soon

[alexander-akait]

@wbobeirne Please try (works fine for me):

const path = require("path");
const interceptor = require("express-interceptor");

var finalParagraphInterceptor = interceptor(function(req, res){
  return {
    // Only HTML responses will be intercepted
    isInterceptable: function(){
      console.log(res.get('Content-Type'))

      return /text\/html/.test(res.get('Content-Type'));
    },
    // Appends a paragraph at the end of the response body
    intercept: function(body, send) {
      send("Hello world");
    }
  };
})

module.exports = {
  mode: "production",
  entry: "./src/index.js",
  output: {
    filename: "main.js",
    path: path.resolve(__dirname, "dist"),
  },
  devServer: {
    setupMiddlewares: function setupMiddlewares(middlewares, _devServer) {
      const index = middlewares.findIndex((middleware) => middleware.name === "webpack-dev-middleware");

      middlewares.splice(index, 0, {
        name: 'your-name-of-middleware',
        middleware: finalParagraphInterceptor
      });

      return middlewares;
    },
  },
  devtool: false,
};

Same for the original issue

[JSteunou]

Thank you, this solution works!

I have one concerned though. Before this 4.6 & 4.7 it was easier, because thanks to the hooks one can easily plugged in logic before/after. Now we have to dive a little deeper and maintain code depending on MW order, and know the name of it. Which are not documented BTW. How can I be sure this code will never break?

[alexander-akait]

@JSteunou Based on logic about webpack-dev-middleware, it should not be broken ever, because express-interceptor should be before any middleware which return respinse with content webpack-dev-middleware is the first in list. If we will need to change the order we will add it to BREAKING CHANGE and migration guide and it will be marked as breaking change.

I want to close the issue, because solved, anyway feel free to feedback and ping me if somebody will have problems or questions.