http-middleware icon indicating copy to clipboard operation
http-middleware copied to clipboard

Body stream is already complete and cannot be read after using http-middleware

Open editedredx opened this issue 1 year ago • 5 comments

Hi,

This problem arises in v0.10.2, v0.10.1 worked without issue. Once a PUT or POST handler has been added the body has been read and cannot be read by any other middleware anymore. This is similar to what the express bodyParser middleware does but that would place the body inside the req object to be used but that's not the case here.

Simple testcase:

import express from 'express';
import { createMiddleware } from '@mswjs/http-middleware';
import * as msw from 'msw';

const app = express();

app.use(
  createMiddleware([
    msw.http.post('something', function () {
      return new Response(JSON.stringify({ foo: 123 }), {
        headers: {
          'Content-Type': 'application/json',
        },
      });
    }),
  ])
);

app.use((req, res) => {
  console.assert(req.readable === true);
  console.assert(req.readableDidRead === false);
  console.assert(req.readableEnded === false);
  res.sendStatus(200);
});

app.listen(9090, () => console.log('Ready at http://localhost:9090'));

Test by making a POST request to http://localhost:9090, path doesn't have to be /something

The assertions succeed on 0.10.1 and fail on 0.10.2. They do succeed if there are only GET handlers passed to createMiddleware.

editedredx avatar Oct 31 '24 12:10 editedredx

Hi! Thanks for raising this. I believe this is happening because this library consumes the request as long as it's readable:

https://github.com/mswjs/http-middleware/blob/4b71edb58f26e6d21ff33a7895f6e063c4e73b6b/src/middleware.ts#L31

I'm not that familiar with the best practices in Express as to how to approach this properly. I can propose we tee the request Readable into a Passthrough stream and that read that. That should technically leave the original req intact and allow for any further readings down the middleware chain.

Would that work?

kettanaito avatar Jun 11 '25 12:06 kettanaito

My suggestion from above won't work. Teeing works if we control both consumers. While we control the internal consumer, we don't control how Express consumes the stream so we cannot provide it with the passthrough version of the stream to be used for the subsequent middleware

const one = new PassThrough()
const two = new PassThrough()
request.pipe(one)
request.pipe(two)

This will allow for independent reads of the same request stream through one and two, but I don't see how we can make Express use two instead of request in its handlers from this point on.

I've tried some hackery too, like creating a lazy version of ReadableStream, but that API doesn't provide you any means to know when it's being consumed so we could defer the request reading until then. Simply converting Readable to ReadableStream consumes the stream.

@editedredx, you've mentioned Express sets request.body. Do you happen to know if there's convention about it? I'd still hate to make you adjust your middleware because you've added handlers but I want to know about all the options.

kettanaito avatar Jun 11 '25 14:06 kettanaito

Same problem here, after a deep search I've found this issue describing exactly what I was trying to identify. I'm trying to have a separate mocking server using express and msw that can be consumed by two different aplications at the same time but i'm unable to access the body in POST requests inside the handlers.

Looking forward to a solution while looking for workarounds

Thanks a lot

EDIT:

I just realized my problem can be solved using bodyParser just before the middleware is used, this way the middleware directly reads the body as a javascript object and can be correctly accesed by the handlers:

// ① Parse JSON _and_ capture the raw bytes
app.use(
  bodyParser.json({
    type: 'application/json',
    verify: (req, res, buf) => {
      console.log('[express ▶ raw body]', buf.toString());
    },
  })
);

// ② Now MSW can consume the parsed body without draining a second stream
app.use(httpServer);

// ③ Your CORS, your static‐assets, etc.
app.use((req, res, next) => {
  // …CORS headers…
  next();
});
app.use('/', express.static(mocksPath));

app.listen(3001, () => {
  console.log('MSW mock server + static assets listening at http://localhost:3001');
});

TomasMoratoPerezPorro avatar Jul 30 '25 09:07 TomasMoratoPerezPorro

I don't believe this can be solved given the architecture of Node.js streams. MSW constructs a fetch representation of outgoing requests, which includes the body stream, even before it checks for any matching request handlers. That is done so you could use the request's body as a predicate for deciding whether to intercept a request or not.

Constructing a web stream out of Readable locks that stream. In other words, it consumes it for good. No other layer of your app would be able to read the request stream after

Readable.toWeb(request)

This limitation is evident when you use body parsers in Express:

  • Cannot use more than 1 body parser;
  • Once you use a body parser, request is locked and cannot be operated on. You must consume req.body that contains the result of the parsing.

MSW is similar in this regard, with the exception that we don't modify the original req and would love for it remain as-is, which is impossible due to the nature of Node.js stream.

No forking/teeing can circumvent that because stream teeing techniques rely on a creating two passthrough streams and then exposing one as a "clean" stream. We cannot override the existing req, which is IncomingMessage, which is a subset of Readable (stream + extras).

What now?

The best way forward is to delegate the request stream reading to the consumer. So you must use a body parser of some shape or form, and then MSW can look up req.body and turn that into a web stream.

Upsides:

  • You control the body parsing.
  • MSW middleware will work without locking the request stream.

Downsides:

  • Actual request streaming is gone. Since MSW will recreate a readable with the entire request stream already parsed by the body parser of your choice, there's no streaming per-se. All the request buffer is known.

kettanaito avatar Aug 23 '25 13:08 kettanaito

Alternative solution

Deprecate the middleware mode. I frankly don't know relied on the middleware mode is. The primary use-case behind this package is to give you a standalone server from your request handlers. I can imagine spawning a brand-new HTTP server would be what most developers are doing. You can share how you're using this package below to help me decide.

Removing the middleware mode means the package controls the server behavior end-to-end, resulting in fewer compatibility issues and bugs arising from those.

What I'm proposing with this is that the usage of this package becomes this:

import { createServer } from '@mswjs/http-middleware'

const server = createServer(...handlers)
// ...

kettanaito avatar Aug 23 '25 13:08 kettanaito