Skip to content

Conversation

@juicemia
Copy link

I opened mswjs/msw#2569 because I noticed some behavior with an HTTP client that waits for the secureConnect event to happen before writing anything, which causes a deadlock because the MSW interceptor waits for write before firing secureConnect.

I've tried my best to also implement a solution given that I'm not an expert on how NodeJS sockets work by any means, but I wanted to at least try to be helpful. Any feedback on this PR is welcome. It seems to me like the code is a bit brittle. The implementation of the once method is the main thing that fixes that deadlock, but it caused other tests to fail so I went trying to handle each case one by one.

I also confirmed that it fixes the issue in my reproduction repository. I tested it by building the client on my Mac and linking it like this:

    "pnpm": {
      "overrides": {
      "@mswjs/interceptors": "file:/path/to/local/projects/juicemia/mswjs-interceptors"
    }

Then I removed my node_modules and re-ran pnpm i, and sure enough when running the tests none of them time out anymore.

One thing I would like is to add a test for this in this repo that reproduces the issue in the reproduction repo exactly. On my first iteration when I got the test in test/modules/http/regressions/http-socket-secure-connect-passthrough.test.ts working, all the tests in this repository were passing, but using the built package in my reproduction repository still didn't make it work. It was making that work that then caused all the regressions that I had to go fixing one by one. I'd love any tips on how I can make a test like that, I was thinking of copying how msw sets up ClientRequestInterceptor but it seemed like it would be very complicated to do.

@juicemia juicemia changed the title Fix deadlock with clients that wait for secureConnect before writing anything fix(MockHttpSocket): deadlock with clients that wait for secureConnect before writing anything Aug 22, 2025
@kettanaito
Copy link
Member

kettanaito commented Oct 19, 2025

Hi, @juicemia. Appreciate your proposal and a fix here! I wonder how viable it is to proceed with MockHttpSocket given a total architecture overhaul at #741. We would be just adding more things to the plug that way.

The new architecture taps into net.* methods and then routes the sent packets through the interceptor-scoped parsers (think HTTP parser, SMTP parser, etc). That way, we aren't even working on the socket level anymore. All the socket events are emitted as-is for passthrough request. For mocked HTTPS/HTTP, we will just emit respective events, like so. That's similar to what we're doing now.

Todos

I should definitely take the test cases you added and add them to #741 before releasing it. Would be a nice way to confirm if those changes fix the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants