Support http.request flushHeaders

[mikicho]

flushHeaders should kickstart the request (same as end): https://nodejs.org/api/http.html#requestflushheaders
Nock test: https://github.com/nock/nock/blob/main/tests/test_intercept.js#L964

[kettanaito]

That's interesting, I didn't know you could trigger the request by .flushHeaders(). I assume this is relevant only for requests without body? I need to read more about this, see some usage examples before I'm certain how to correctly address this in the interceptor.

As of now, I'm wondering if .flushHeaders() is the equivalent of .end().

[kettanaito]

A bit of digging around.

• .flushHeaders() implementation in Node.js
• Test suite for .flushHeaders()
• Another test suite that mentions that .flushHeaders() sends a request without waiting for the body. I wonder if this means the body is ignored or written after the request has been sent.
• Also interesting that there's res.flushHeaders() as well that seems to do similar for response.

[mikicho]

A bit more context from the docs:

For efficiency reasons, Node.js normally buffers the request headers until request.end() is called or the first chunk of request data is written. It then tries to pack the request headers and data into a single TCP packet.
That's usually desired (it saves a TCP round-trip), but not when the first data is not sent until possibly much later. request.flushHeaders() bypasses the optimization and kickstarts the request

[kettanaito]

I think I understand how flushHeaders() is meant to work. Reading through the mentioned test cases from Node.js helped a lot.

• req.flushHeaders() sends a request even if its body hasn't been written. This way you can force Node.js to start a request faster, for whichever reason.
• You may still write request body after it's been started. This is illustrated well in this test. But you may not write request body at all, if it's a GET request, for example. Neither would you have to call .end() explicitly as you would otherwise.
• However, .flushHeaders() does not end the request. Nowhere in Node.js does it propagate to .end(), so that method will never be called on the request when flushing headers. It will get called if you decide to write to the request, as you'd have to end its stream once you're finished writing.

This considered, implementing a full support for .flushHeaders() is a bit problematic in the current state of Interceptors. This is related to the discussion in #400 (comment). Shortly, right now the library constructs the Request instance and emits the request event after the request has been completely written and sent.

With .flushHeaders(), it'd have to revert its event flow a little, and do this:

1. Construct a ReadableStream internally and write/end that stream based on the .write()/.end() of the underlying ClientRequest.
2. Construct a Request instance as soon as the request is constructed (or its headers are flushed, not sure what's more semantically correct here). This way the consumer would get the request listener called immediately but the request instance will have its body stream possibly in the open/writing state at that time.

As I shared in the linked comment, this is possible but will require some internal refactoring to achieve.

[kettanaito]

@mikicho, do you happen to know how Nock supports .flushHeaders()?

[mikicho]

Thanks for the info! Very insightful!
From my POV, this is insignificant; we can leave this open and tackle it later.

do you happen to know how Nock supports .flushHeaders()?

I didn't dig into it, but it seems like it starts the playback, which I THINK eventually emits an end event.

https://github.com/nock/nock/blob/feaa66fa64d24f95937ef759cdd5a7ca07646f1c/lib/intercepted_request_router.js#L221

[kettanaito]

I think you're right. Nock seems to check if the socket connection has been established, if it hasn't, it sets an internal flag to start the playback as soon as it does. If it has, it just starts the playback, which eventually calls newReq.end():

https://github.com/nock/nock/blob/feaa66fa64d24f95937ef759cdd5a7ca07646f1c/lib/intercepted_request_router.js#L343

I wonder how would Nock handled scenarios such as writing to request body after req.flushHeaders()? If it calls .end() and then the consumer tries to write to the request (which is a legal operation), Node will throw "cannot write after end".

I believe it's rather a big semantic difference between:

1. Emitting "request" when the request has been sent;
2. Emitting "request" when the request has started/been declared.

I want to lean toward the former, to be honest. But in the context of .flushHeader(), this distinction remains relevant: the request is sent with that method, its body just can be written afterward. So we can still support this method by implementing it a bit differently than .end() and keeping track of whether the request body has finished writing (the .end() has been called).

[kettanaito]

I think we can implement .flushHeaders() without any substantial refactoring by doing this:

Step 1: Emit request event on .flushHeaders()

Since this effectively means the request is sent and ready to be received by the server, emit the request event on the Interceptor to let the consumer know.

Step 2: Prepare a request body stream

This is the minimal amount of refactoring we have to do. We need to represent the request body in an internal ReadableStream (most likely will use just Readable since it's easier to write to it) and then provide that stream directly on the created Request instance during .flushHeaders().

This way the consumer will react to the request but if they decide to read its body, they'd have to wait for that request body stream to be populated, if ever.

Step 3: Adjust the .end() call

Since in the case of .flushHeaders() we have already emitted the request event, we need to keep some internal flag to indicate that and do not emit that event in the .end() method if it has already been emitted before. In the .flushHeaders() + .end() combination, the .end() call will only serve as the end of the request body stream, letting the consumer know that the last request body chunk has been written.

if (!this.requestSent) {
  // Emit "request"
}

We'd also want to push the entire emitAsync and mock response logic to .flushHeaders() (or reuse it) since .end() is not guaranteed to be called at all. This may be a bit tricky but I think the best approach here is to abstract the asyncEmit + until(mockedResponse) logic from .end() and reuse it across the two methods (.end() and .flushHeaders()).