Hi, @mikicho 👋 I've tried to create a reproduction scenario for this issue but failed at that. The problem is that the body isn't being handled by the HTTP parser at all in this case. It reaches this:

But this never fires:

The intention behind the current test is to throw with the same error as in nock/nock#2826:

Uncaught Invariant Violation: Failed to write to a request stream: stream does not exist

Could you please take a look at this and let me know if you spot anything I missed?

Once we get the test failing correctly, we can proceed with the fix.

@kettanaito I added content-length to the request, and now it throws the error.

Regarding the solution, we need the request body in the interceptor for interception and recording.
While I really admire your efforts to keep the API clear, there are edge cases (like this and/or the rawHeaders) that I'm not sure are worth going the extra mile for, as they're edge cases and, IMO, should be handled with a more straightforward solution.

@mikicho, thanks for looking into this one.

There's a lot of difference between the raw request/response instances and the Fetch API representation that we are constructing. I understand how this can bite us back sometimes. But this alignment is still highly worth it!

I am not a big fan of rawHeaders myself, but I don't see this stream change as a related issue. In fact, it's just a bug that we should fix (you can write to HTTP GET request streams, in the end).

In the very early version of Interceptors, we also exposed the raw client request instance on the interceptor. I don't know if I like it, but we technically can do this:

interceptor.on('request', ({ request, rawRequest }) => {
  request // Request
  rawRequest // ClientRequest OR XMLHttpRequest OR Request
})

If I was to do this today, I'd probably do request[kRawRequest] instead. Lock it behind a symbol to make it abundantly clear that you shouldn't be accessing that value normally.

This kind of API was removed because it introduces significant overhead at the usage layer. You become in charge to differentiate different request types, which kills the purpose of a single interception library.

I also agree that all these use cases so far were edge cases. You normally don't need raw headers, for example, and not all request clients even support thats as a concept. I can also argue that HTTP headers are case-insensitive by design so the concept of recording and exposing raw headers is also flawed by design. I may be wrong on this so feel free to point that out.

@kettanaito We see eye to eye here.
The thing I want to point out is that we may want to enable an "escape hatch" for legacy or non-standard behavior. New edge cases may arise on any platform, tool, library, etc. So, instead of trying to fit them into our main solution, we might develop a robust approach that allows us to expose these quirks in each client in a maintainable and convenient way. For example (not necessarily a good one that I recommend), instead of exposing the entire rawRequest, ' which may be confusing and cumbersome, as you mentioned, we can expose the extras property, which includes specific properties like rawHeaders or body.
I totally agree that we need to be cautious with these escape hatches and introduce them only when there is no other option or when the cost of embedding a solution into the main flow is too high.

instead of exposing the entire rawRequest, ' which may be confusing and cumbersome, as you mentioned, we can expose the extras property, which includes specific properties like rawHeaders or body.

There's quite an important distinction between these two approaches, though. Exposing kRawRequest means "here's the keys, do your custom logic as you see fit". Exposing kRawHeaders or kBody means "here's an API we sort of maintain but it's not public.

Given how we are largely discussing edge cases, I think we should not build more logic around them than necessary (which is exactly the point you bring up and I agree).

I wonder if we could remove the raw headers recording entirely and replace it with kRawRequest + custom logic on Nock's side (a matter of request[kRawRequest].rawHeaders or something)? From the back of your head, would that be possible? I don't think we rely on raw headers anywhere else but I may be wrong.

The symbol also isn't the only way to expose that API. In fact, perhaps we need something like:

import { getRawRequest } from '@mswjs/interceptors'

interceptor.on('request', ({ request }) => {
  const rawRequest = getRawRequest(request)
})

Released: v0.38.2 🎉

This has been released in v0.38.2!

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

I wonder if we could remove the raw headers recording entirely and replace it with kRawRequest + custom logic on Nock's side

Yes. Definitely into this kind of thing.