this._getSession is not a function while use HttpsProxyAgent

[WhoisUnknown]

Hi, I encounter a bug when I use proxy for requests

TypeError: this._getSession is not a function
    at HttpsProxyAgent.createConnection (node:https:140:26)
    at MockHttpSocket.passthrough (example/node_modules/@mswjs/interceptors/src/interceptors/ClientRequest/MockHttpSocket.ts:160:25)
    at _ClientRequestInterceptor.onRequest (example/node_modules/@mswjs/interceptors/src/interceptors/ClientRequest/index.ts:151:21)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)

Example repo https://github.com/WhoisUnknown/example-interseptor

I used version 0.36.10

[fionawhim]

Seeing this as well, with 0.37.1 and Node v20.12.0. My use case is MSW running in a Vitest test, and trying to access (unmocked) HTTPS endpoints.

I think the code in the Node repo is here: https://github.com/nodejs/node/blob/v20.x/lib/https.js#L163

Possibly a situation in the way that the createConnection method is used here? It gets bound to something without a _getSession method, but an _agentKey is still being passed into options?

[sebws]

interceptors/src/interceptors/ClientRequest/agents.ts

Lines 67 to 82 in c338da8

	public createConnection(options: any, callback: any) {
	const createConnection =
	(this.customAgent instanceof https.Agent &&
	this.customAgent.createConnection) ||
	super.createConnection
	const socket = new MockHttpSocket({
	connectionOptions: options,
	createConnection: createConnection.bind(
	this.customAgent || this,
	options,
	callback
	),
	onRequest: this.onRequest.bind(this),
	onResponse: this.onResponse.bind(this),
	})

Looks like what happens is that this.customAgent is there, but it isn't an instance of https.Agent, so createConnection is taken from super.createConnection. Then when we go to bind at L75-L79, it binds super.createConnection to this.customAgent. No good!

Given that createConnection does exist on the HttpsProxyAgent (this.customAgent), I think it shouldn't be that super.createConnection is now bound to this instead of this.customAgent, instead that the check isn't if this.customAgent is an instance of https.Agent but something else?

I'm not sure of the intent here. Hopefully somebody who knows better what's intended to happen here, might just be @kettanaito

[mikicho]

@sebws do you mean something like:

public createConnection(options: any, callback: any) { 
   const createConnection = 
     this.customAgent.createConnection || 
     super.createConnection 

[sebws]

I think I may have, but as far as I remember it might not have been so simple

[mikicho]

I think we should let users use whatever agent they want:

   public createConnection(options: any, callback: any): net.Socket {
-    const createConnection =
-      this.customAgent instanceof https.Agent
-        ? this.customAgent.createConnection
-        : super.createConnection
+    let _createConnection = super.createConnection
+    let _options = options
 
-    const createConnectionOptions =
-      this.customAgent instanceof https.Agent
-        ? {
-            ...options,
-            ...this.customAgent.options,
-          }
-        : options
+    if (typeof this.customAgent !== 'boolean' && this.customAgent?.createConnection) {
+      _createConnection = this.customAgent.createConnection
+      _options = { ...options, ...this.customAgent.options }
+    }
 
     const socket = new MockHttpSocket({
       connectionOptions: options,
-      createConnection: createConnection.bind(
+      createConnection: _createConnection.bind(
         this.customAgent || this,
-        createConnectionOptions,
+        _options,
         callback
       ),
       onRequest: this.onRequest.bind(this),

@kettanaito WDYT?

[sebws]

I don't remember, is there a reason it's not as simple as doing the instanceof check in the .bind call as well?

It seems at least like a starting point. Some very quick testing to me seems to show it fixing the example repo for me

[mikicho]

is there a reason it's not as simple as doing the instanceof check in the .bind call as well?

Could you provide a code example to clarify your point?

[sebws]

public createConnection(options: any, callback: any) {
    const createConnection =
      (this.customAgent instanceof https.Agent &&
        this.customAgent.createConnection) ||
      super.createConnection

    const socket = new MockHttpSocket({
      connectionOptions: options,
      createConnection: createConnection.bind(
        (this.customAgent instanceof https.Agent && this.customAgent) || this,
        options,
        callback
      ),
      onRequest: this.onRequest.bind(this),
      onResponse: this.onResponse.bind(this),
    })

    return socket
  }

The condition could be reused but just to demonstrate where there's a difference between the conditions possibly causing an issue