fix(ClientRequest): preserve headers type when recording raw headers

[paoloricciuti]

Closes #651

Basically when defining the header property on the original init request or the original init headers we were assigning the value of kRawHeaders but that's just a string array. So if that Request is then used again trying to read the headers with req.headers.get it will fail since there's no get method in the array of strings.

Also i've noticed

if (inferRawHeaders.length > 0) {
    defineRawHeadersSymbol(request.headers, inferredRawHeaders)
}

which is a classic TS mystake: you were checking if that function had more than one argument but that's always true so i switched to what i think you actually wanted to check.

I've added a couple of test to check this that fails before this PR.

[carpie]

I think the Response proxy needs this treatment as well. Creating HttpResponse objects after these proxies are installed will throw TypeError: init.headers.get is not a function. The following small script can demonstrate:

const { setupServer } = require('msw/node');
let s = setupServer();
s.listen();

const { HttpResponse } = require('msw');
HttpResponse.json('{}', { status: 200, statusText: 'OK' });

I was able to "fix" it locally by wrapping the right hand side of the assignment in

interceptors/src/interceptors/ClientRequest/utils/recordRawHeaders.ts

Line 248 in 8fad9cf

args[1].headers = args[1].headers[kRawHeaders]

with a new Headers() after finding this PR.

[paoloricciuti]

I think the Response proxy needs this treatment as well. Creating HttpResponse objects after these proxies are installed will throw TypeError: init.headers.get is not a function. The following small script can demonstrate:

const { setupServer } = require('msw/node');
let s = setupServer();
s.listen();

const { HttpResponse } = require('msw');
HttpResponse.json('{}', { status: 200, statusText: 'OK' });

I was able to "fix" it locally by wrapping the right hand side of the assignment in

interceptors/src/interceptors/ClientRequest/utils/recordRawHeaders.ts

Line 248 in 8fad9cf

args[1].headers = args[1].headers[kRawHeaders]

with a new Headers() after finding this PR.

You are absolutely right...i missed this because the way to trigger it is a bit weird. Pushed the fix now.

[carpie]

You are absolutely right...i missed this because the way to trigger it is a bit weird. Pushed the fix now.

Awesome! Thank you!

[kettanaito]

Maybe let's rephrase it to "preserves the original headers instance"?

[paoloricciuti]

Well it's not technically the same header instance...however if you think it's important we could rework this whole thing by pushing looping over the raw headers and set the on the original instance

[kettanaito]

Thanks for opening this, @paoloricciuti.

I believe those 3 if checks were left incorrectly. They aren't needed anymore. We don't need to modify the init in any way. The raw headers inference was achieved later on, by these blocks:

interceptors/src/interceptors/ClientRequest/utils/recordRawHeaders.ts

Lines 219 to 230 in 2067cb0

	if (typeof args[0] === 'object' && args[0].headers != null) {
	inferredRawHeaders.push(...inferRawHeaders(args[0].headers))
	}
	// Infer raw headers from the "headers" init argument.
	if (typeof args[1] === 'object' && args[1].headers != null) {
	inferredRawHeaders.push(...inferRawHeaders(args[1].headers))
	}
	if (inferRawHeaders.length > 0) {
	defineRawHeadersSymbol(request.headers, inferredRawHeaders)
	}

interceptors/src/interceptors/ClientRequest/utils/recordRawHeaders.ts

Lines 253 to 258 in 2067cb0

	if (typeof args[1] === 'object' && args[1].headers != null) {
	ensureRawHeadersSymbol(
	response.headers,
	inferRawHeaders(args[1].headers)
	)
	}

I've removed the if checks, and the tests are still passing. Let me test these changes in MSW as well, as it may have unique use cases we are not considering here.

[kettanaito]

This was a typo ✅

[kettanaito]

Here I'm using ensure to prevent the raw headers symbol from being re-defined.

[paoloricciuti]

Uh i don't remember changing this line...weird.

[kettanaito]

Both interceptors and MSW test suites have passed with these changes. Let's get this out.

[kettanaito]

@paoloricciuti, welcome to contributors! 🎉

[kettanaito]

Released: v0.36.5 🎉

This has been released in v0.36.5!

• 📄 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.