Skip to content

Conversation

@kettanaito
Copy link
Member

@kettanaito kettanaito commented Aug 2, 2025

Motivation

This finally moves the request interception to the un-opinionated net module level, allowing each interceptor to route socket packets through their respective parsers (HTTP, SMTP, etc).

Changes

  • SocketInterceptor patches net.connect() and net.createConnection() to introduce a proxy socket instance to listen to outgoing packets and mock incoming packets.
  • Other interceptors, like the new take on HttpRequestInterceptor, can now become truly client-agnostic and use the underlying socket interceptor (should probably be a singleton) to provision the interception.
  • The higher-level interceptors determine if the outgoing socket connection is relevant to them by parsing the first sent packet, which is usually the application protocol + metadata.
  • 6318aea correctly frees the request/response HTTP parsers, mimicking what Node.js does in its freeParser() internal utility.

Roadmap

  • Try reusing MockHttpSocket somehow as it handles the suppression of connection errors and passthrough properly.
  • Do not call socket.connect() in net.connect(). Instead, delegate that action to the user (i.e. the interceptor). Just in case they want to abort the connection before establishing it.
    • Do clients start writing after connect? Double-check this. If so, we must emit a preemptive connect but only in the HttpRequestInterceptor.
  • Implement HttpRequestInterceptor
  • Remove ClientRequestInterceptor
  • Update XMLHttpRequestInterceptor. Out of scope. New feature.
  • Update FetchInterceptor.
  • Check if .prependOnce() is implemented via .prepend() and if that's the case, ignore recording .prependOnce() like we do with .once().
  • Expose the recorder from the MockSocket to stop the recording after initiating a mocked response. At that point, we know we will never need to replay those recordings (they are only for passthrough). Just call something like socket[kRecorder].free() in HttpRequestInterceptor.respondWith().
  • Copy tests from fix(MockHttpSocket): deadlock with clients that wait for secureConnect before writing anything #745 (comment) to verify that these changes fix the referenced issue (secureConnect before .write()).
  • SocketController.constructor: finish the onEntry callback of the recorder for passthrough requests. This fails the tests right now.
  • Design: decide how the SocketInterceptor will be reused across multiple higher-level interceptors. Should it be used as a singleton? Or just constructed everywhere and rely on the existing symbol-base interceptor deduping? Any pros/cons here?

@kettanaito kettanaito force-pushed the feat/net-interceptor branch from 3a74846 to 6f76977 Compare August 4, 2025 18:04
@kettanaito kettanaito force-pushed the feat/net-interceptor branch from 6f76977 to 1e3b90f Compare August 4, 2025 18:05
@kettanaito kettanaito force-pushed the feat/net-interceptor branch from bb5e026 to 32fcd0f Compare August 5, 2025 13:06
@kettanaito kettanaito force-pushed the feat/net-interceptor branch from 5e9c2fe to 5429699 Compare August 5, 2025 14:13
@kettanaito kettanaito force-pushed the feat/net-interceptor branch from a149d32 to 8452422 Compare August 5, 2025 14:33
@kettanaito kettanaito force-pushed the feat/net-interceptor branch from e4ef073 to 2c7f4e9 Compare August 5, 2025 16:15
@kettanaito kettanaito changed the title feat(wip): net interceptor feat: intercept requests on the net socket level Aug 5, 2025
@kettanaito
Copy link
Member Author

kettanaito commented Aug 5, 2025

On XMLHttpRequest

Currently, we're testing XHR in JSDOM (so it's faster). That means that the global XMLHttpRequest class is a part of the test setup. In order for the net interceptor to affect it, the interceptor must be imported before the setup itself. That's quite troublesome and I'm considering if it's even worth it.

I do not encourage JSDOM or DOM-like environments, in general. We should migrate our XHR test to the real browser (we already have in-browser tests but afaik they aren't using Vitest Browser Mode; Playwright might be an overkill for our needs).

For the JSDOM users, we may provide a recipe of how to enable the interceptor as a part of their environment so they have some reference.

XMLHttpRequest interception

Despite the net interceptor, some specialized client interception must still happen on the client level. That includes XHR! We should keep XMLHttpRequestInterceptor and use the net interceptor under the hood (if we want to deviate between the browser and Node.js though).

Important

The net interceptor is intended to be used as a low-level mechanism for Node.js requests. We can and likely should keep more specialized interceptors, like XHR, both to preserve the usage-level info (e.g. .withCredentials) and, in some cases, to have the same interceptor behavior in any environment (think XHR in Node.js via JSDOM or in the browser directly). In that regard, those are more of a protocol-based interceptors.

@kettanaito kettanaito force-pushed the feat/net-interceptor branch from 988dfdd to 31fa254 Compare August 6, 2025 11:52
@kettanaito kettanaito force-pushed the feat/net-interceptor branch from 99d10ac to 4d4f851 Compare August 7, 2025 09:11
@kettanaito kettanaito force-pushed the feat/net-interceptor branch from d3faabe to e375e22 Compare August 7, 2025 13:06
@kettanaito
Copy link
Member Author

kettanaito commented Aug 8, 2025

Discovery: Missing protocol check for HTTPS ClientRequest

Turns out, our previous implementation completely disregarded the unpatched behavior of ClientRequest when providing it an HTTPS URL. We've skipped the protocol check that has to be there!

If you want to perform an HTTPS request via http.ClientRequest you MUST use the HTTPS agent so Node.js knows what's the expectedProtocol (it takes that from the agent). If you don't, you will get this exception and that's a correct behavior we should keep:

Protocol "https:" not supported. Expected "http:"

Because as far as Node.js is concerned, you are constructing an HTTP request. Nothing in your request says it has to be HTTPS.

I've updated test/modules/http/intercept/http-client-request.test.ts tests for HTTPS to now reflect this behavior.

@kettanaito kettanaito force-pushed the feat/net-interceptor branch from fe44b85 to 6318aea Compare August 8, 2025 12:39
@kettanaito
Copy link
Member Author

Note: Internal HTTP parser is not associated with the socket

The HTTP parsers we create for parsing out requests and responses are not associated with the underlying socket, mocked or passthrough, in any way. Those sockets create their own parsers per Node.js and keep using them for their own purposes.

This is important to keep in mind when we free our own parsers. No need to nuke the socket.parser association:

  1. It's not set by us;
  2. It will be cleared by freeParser() in Node.js for us.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support raw net.connection

2 participants