fix(xhr): clone the request before calculating its body size

[robbtraister]

• Fixes XHR is broken in 0.34.3 #630

The root cause of the issue is that cloning the Request to calculate the Content-Length header occurs too late. At the point in which clone() is called, the body buffer has already been consumed, so cloning fails.

Since this kFetchRequest value is only used for this specific purpose of calculating content-length, there is no harm in creating and storing a clone immediately upon creation of the Request object.

[ddolcimascolo]

Hey all, can we get this merged and msw released?

Thx,
David

[kettanaito]

Golden 🥇 Thank you for adding this fix, @robbtraister!

What we are missing is an automated test for this. Would you have some time to add one? You can add it to XHR regression tests.

If not, let me know, I will add it once I have a moment.

[robbtraister]

@kettanaito I tried adding a test, but I can't seem to duplicate the behavior. In the test environment, reading the request buffer does not seem to lock it from future attempts to read/clone.

This is a simplification of what I tried:

  interceptor.once('request', async ({ controller, request }) => {
    await request.arrayBuffer() // <-- this is the addition to the existing test, along with changing to POST
    controller.respondWith(
      new UndiciResponse('Hello world') as unknown as Response
    )
  })

[robbtraister]

I think the issue is that the vite environment has its own Request implementation that does not block cloning after consumption.

I was able to figure out a hacky workaround by importing Request from "undici" and setting it to global.Request in the beforeAll hook, then reverting in the afterAll hook. Does that seem reasonable?

[ddolcimascolo]

@robbtraister I'm impacted by this one and I'm using Vite + Vitest. Please see mswjs/msw#2273 (comment)

[kettanaito]

I was able to figure out a hacky workaround by importing Request from "undici" and setting it to global.Request in the beforeAll hook, then reverting in the afterAll hook. Does that seem reasonable?

Hmm, this seems a bit too much. We are using the Undici's Request. Let me take a look at what we can do to improve these tests, and how to drive out this problematic scenario.

[kettanaito]

I've added an isolated test and confirm it failing with the following error without the proposed fix:

TypeError: unusable
 ❯ Request.clone ../node:internal/deps/undici/undici:7289:17
 ❯ XMLHttpRequestController.respondWith ../src/interceptors/XMLHttpRequest/XMLHttpRequestController.ts:299:29
    297|     if (this[kFetchRequest]) {
    298|       const totalRequestBodyLength = await getBodyByteLength(
    299|         this[kFetchRequest].clone()
       |                             ^
    300|       )
    301| 
 ❯ Object.onResponse ../src/interceptors/XMLHttpRequest/XMLHttpRequestProxy.ts:70:24
 ❯ handleResponse ../src/utils/handleRequest.ts:55:21
 ❯ Module.handleRequest ../src/utils/handleRequest.ts:207:12
 ❯ processTicksAndRejections ../node:internal/process/task_queues:95:5
 ❯ XMLHttpRequestController.xhrRequestController.onRequest ../src/interceptors/XMLHttpRequest/XMLHttpRequestProxy.ts:64:34

With the fix, the test is passing.

[kettanaito]

This looks great. Waiting for the tests before merging.

[robbtraister]

It looks like the key difference is using the jsdom environment in lieu of happy-dom. Either way, thanks for getting it working.

[kettanaito]

Released: v0.35.3 🎉

This has been released in v0.35.3!

• 📄 Release notes
• 📦 npm package

Make sure to always update to the latest version (npm i @mswjs/interceptors@latest) to get the newest features and bug fixes.

---

Predictable release automation by @ossjs/release.