fix(ClientRequest): inherit the connection _handle for passthrough requests#706
Conversation
|
I wonder if this could be the underlying cause of mswjs/msw#2405? |
|
Yes.. it might fix this as well. but I'm not sure about it |
| it('does ', async () => { | ||
| const url = httpServer.http.url('/') | ||
| const warningListener = vi.fn() | ||
| process.on('warning', warningListener) |
There was a problem hiding this comment.
This is an indirect assertion. I think you are onto something with that connect event. Can we somehow add a listener to that instead and assert that it's being called once? That would make a nice compliance test.
There was a problem hiding this comment.
Delightful comment! Fixed.
kettanaito
left a comment
kettanaito
left a comment
There was a problem hiding this comment.
This is a great discovery! It needs more discussion before we proceed with the fix. Would be grateful for your opinion on the comments I left.
|
@kettanaito WDYT? |
kettanaito
changed the title
MockSocket does not have _handle in passthrough
kettanaito
changed the title
MockSocket does not have _handle in passthrough_handle for passthrough requests
kettanaito
changed the title
_handle for passthrough requests_handle for passthrough requests
kettanaito
left a comment
kettanaito
left a comment
There was a problem hiding this comment.
A few tiny adjustments. This looks great. Let's see what the tests tell us.
| Object.defineProperty(this, '_handle', { | ||
| value: socket._handle, | ||
| enumerable: true, | ||
| writable: true, |
There was a problem hiding this comment.
_handle has to be writable. Node overrides it themselves a few times during the request's life cycle.
Released: v0.38.7 🎉This has been released in v0.38.7! Make sure to always update to the latest version ( Predictable release automation by @ossjs/release. |
|
Hello @kettanaito @mikicho I use I locally patched the files to comment these changes and it worked. |
|
@saikumarrs Thank you for reporting this! Could you please open an issue with a minimal reproduction? |
4 participants
MockHttpSocketandMockHttpAgentare not being garbage collected msw#2405Since the
MockSocketnever actually connects to a server, it does not initialize its_handleproperty. As a result, this push creates a connect listener with every push.@kettanaito Please guide me in the leftovers:
this._handleis private; there is an undocumentedhandleoption in the Socket constructor. How do you think we should handle this?Fixes: nock/nock#2830