response.complete is false in end listener

[mikicho]

We set the this.response.complete after the response.push(null), which emits the end event.
Therefore, I think the response.complete is always false in the end event.

IIUC the docs, it should be true:
https://nodejs.org/api/http.html#messagecomplete

I think we should switch these lines: https://github.com/mswjs/interceptors/blob/main/src/interceptors/ClientRequest/NodeClientRequest.ts#L489-L490

[mikicho]

Maybe I'm wrong. I'll reopen if needed

[mikicho]

@kettanaito

1. I guess I'm doing something wrong, but this script doesn't print true/false:

const { ClientRequestInterceptor } = require('@mswjs/interceptors/ClientRequest')
const http = require('http')

const interceptor = new ClientRequestInterceptor({
  name: 'my-interceptor',
})
interceptor.apply();
interceptor.on('request', ({request}) => {
  request.respondWith(new Response('hey', { status: 200 }))
});

http.get('http://example.test', res => {
  res.on('end', () => {
    console.log(res.complete)
  })
})

Do you spot my error here?

2. I didn't have time to find the root cause now, but I think the complete property is always false because we clone the response: https://github.com/mswjs/interceptors/blob/main/src/interceptors/ClientRequest/NodeClientRequest.ts#L330-L332

3. I'll try to reproduce it with a test later

[kettanaito]

Do you spot my error here?

Afaik, you need to read the response body if you want for it to emit the end event. Simply add the res.on('data', () => {}) listener and the end will be emitted as you expect.

[mikicho]

thanks!

const { ClientRequestInterceptor } = require('@mswjs/interceptors/ClientRequest')
const http = require('http')

const interceptor = new ClientRequestInterceptor({
  name: 'my-interceptor',
})
interceptor.apply();
interceptor.on('request', ({request}) => {
  request.respondWith(new Response('hey', { status: 200 }))
});

http.get('http://example.test', res => {
  res.on('data', () => {})
  res.on('end', () => {
    console.log(res.complete) // prints false
  })
})

res.complete prints false. Is this expected?

[kettanaito]

I think you're right, res.complete should be true by the time the end event is emitted. Will open a fix.

[kettanaito]

Right now, when using Interceptors, the res.complete property always remains false. That's due to us cloning the original emitted response instance right here:

interceptors/src/interceptors/ClientRequest/NodeClientRequest.ts

Lines 330 to 332 in 01aa28d

	const response = data[0] as IncomingMessage
	const firstClone = cloneIncomingMessage(response)
	const secondClone = cloneIncomingMessage(response)

We do this cloning so then the received response can be read multiple times:

1. Internally by Interceptors to construct a Response instance out of it.
2. Externally by the developer via data and end events.

And while both use cases are working right now, it seems that something is off in regards to the complete and maybe even the end event.

[kettanaito]

In Node.js, it's the stream module that sets the complete property to true when the message is complete:

https://github.com/nodejs/node/blob/67546529c6e8181aa3f93aca5cbbada29bfd29e9/lib/_http_common.js#L142

https://github.com/nodejs/node/blob/67546529c6e8181aa3f93aca5cbbada29bfd29e9/lib/_http_common.js#L168

Perhaps, our cloneIncomingMessage doesn't propagate that [kOnMessageComplete] symbol to the cloned messages.

[kettanaito]

I think my initial implementation of IncomingMessage cloning was a bit too radical and mainly based on my lack of knowledge of streams in Node.js. I can't say it's much better now, but I think what we should do instead is to pipe the original response through a PipeThrough() stream that would allow us to react to incoming chunks, and then pipe those chunks to a Readable that we'd pass to the createResponse().

I think if we pipe it to a Readable directly, it would still error if we try to read them twice but I can verify.

[mikicho]

From a quick look, I think this is what nock does: https://github.com/nock/nock/blob/main/lib/playback_interceptor.js#L275-L291

[kettanaito]

Thanks for the reference! In Interceptors, we consume the same response body multiple times so we have to have some mechanism in place not to block further consumers if we read the response body internally (for example, in order to construct a Response representation out of IncomingMessage). Nothing in the linked code would account for that, as it's more about converting the body to a stream and reading it than allowing it to read the same stream multiple times. I think that's due to the API difference between the libraries.

[saurabh-asurion]

hey @kettanaito & @mikicho, I am running into the same issue (i.e. response.complete is false) even when using mswjs/interceptors to just observe the response. I can confirm that I am not mocking the response in interceptor.('request', ({.....}))

Is there a workaround I can go with in the short term? Thanks in advance for the help!

[mikicho]

@saurabh-asurion
Move to Nock 😉 (Just kidding, we are working hard to replace the Nock underlining interception engine with @mswjs/interceptors)

I don't think there is a workaround, but I have a feeling we will solve it soon :) 🤞