fix(setupWorker): correctly delete internal accept header on passthrough

[smouillour]

• Related to fix: prevent infinite loop when bypassing sendBeacon() requests #2353
• https://developer.mozilla.org/en-US/docs/Web/API/Headers/delete

the function delete for Headers manage only one parameter.
what we try to do there it's to remove the value msw/passthrough to the header accept.

change will check content of accept and:

• if the header contains only msw/passthrough then it will be removed
• if the header contains multiple value than it will update the header to remove msw/passthrough inside
• if the header not contains the value msw/passthrough then nothing is done

[kettanaito]

Hi, @smouillour. Thanks for spotting this. That's a good catch. In Node.js, you can use Headers.prototype.delete to delete a particular header value (and only that value). That's not the case for the Web API though.

Your proposal is okay if the accept header was used only by MSW, but it won't work if that internal header value was appended to an already existing accept header.

To properly solve this, we need to parse the accept header in case it's multi-valued, then remove the internal MSW header value, then stitch the remaining value back. If it's not an empty string after these transformations, rewrite the accept header to whichever string remains. If it is an empty string, delete the accept header altogether.

How does that sound?

[smouillour]

Hi @kettanaito,
It's clear. I will update my PR following this.

[kettanaito]

This looks great, thank you!

One thing remaining is to add a test for this since we didn't catch this before. Would you like to give it a try? If not that's okay, I can add a test when I have some time this/next week.

[smouillour]

I added tests.
I had to export getResponse to do that as I used it as starting point for the test.
Hoping that everything will be okay and that will be enough

[kettanaito]

Thanks, @smouillour. We usually take a different approach to testing worker-related things, that being relying on e2d tests (build -> run in the browser). I've reverted your test and added the in-browser test instead. If you are curious, take a look to see the difference (mostly that it's implementation-agnostic now).

[kettanaito]

Looks great! 🚀

[kettanaito]

Welcome to contributors, @smouillour 👏

[kettanaito]

Released: v2.6.7 🎉

This has been released in v2.6.7!

• 📄 Release notes
• 📦 npm package

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

---

Predictable release automation by @ossjs/release.

[mckelveygreg]

Oh my goodness, thank you for finding this! I've been trying to figure this one out for a few months now. We use testcontainers, and I think this logic was messing with a bunch of stuff I have to passthrough in order to get to the system's docker process.

Our tests finally passed again! Thanks!