fix: MockHttpSocket: if passthrough: dont flush write buffer on stream read #659
Conversation
|
Hi, @thecopy. Thanks for opening this. If we don't flush request chunks on Note that it's the handler that decides whether the request is mocked or passthrough, not the other way around. In other words, you cannot know if the request is passthrough until you flushed the request buffer on read so the handler can read the request body and decide on how to handle the request. What you are describing sounds like a bug in how we are handling the write buffer. We should still flush, but if the passthrough is established, we should send those chunks via the original socket instance as well to make a proper request. Does that sound right to you? Wdyt? |
| @@ -63,6 +63,7 @@ export class MockHttpSocket extends MockSocket { | |||
| private responseType: 'mock' | 'bypassed' = 'bypassed' | |||
| private responseParser: HTTPParser<1> | |||
| private responseStream?: Readable | |||
| private isPassthrough: boolean | |||
There was a problem hiding this comment.
Cannot this be represented via responseType === 'bypassed? We already have a flag to track this.
| @@ -472,7 +477,9 @@ export class MockHttpSocket extends MockSocket { | |||
| // flush the write buffer to trigger the callbacks. | |||
| // This way, if the request stream ends in the write callback, | |||
| // it will indeed end correctly. | |||
| this.flushWriteBuffer() | |||
| if (!this.isPassthrough) { | |||
| this.flushWriteBuffer() | |||
There was a problem hiding this comment.
As I've mentioned, this introduces an impossible state. You won't know if the request is passthrough until the handler is finished running. For the handler to finish running, it must be able to read the request's body, for which we must flush these first chunks.
resolves mswjs/msw#2309
Underlying issue
As i understand the code: if consumer attempts to start reading stream before buffer is sent, the items will be removed from the writeBuffer. If
passthrough()is called after, it will miss those chunks.This change disables calling
flushWriteBufferonstream.readif current request is a passthrough request.