this._getSession is not a function while use HttpsProxyAgent
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
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?
https://github.com/mswjs/interceptors/blob/c338da8363660fbd08e794511e35678dae095670/src/interceptors/ClientRequest/agents.ts#L67-L82
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
@sebws do you mean something like:
public createConnection(options: any, callback: any) {
const createConnection =
this.customAgent.createConnection ||
super.createConnection
I think I may have, but as far as I remember it might not have been so simple
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?
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
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?
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
Hi everyone,
Are you waiting from any specific input to move on with this issue? This currently breaks nock@14 and it would be nice to resolve it, WDYT?
Thx!
David
Hi again, nock@14 is still broken without this for users of custom agents... Is anyone planning on working on it?
Thanks guys, David
Hello guys, just noticed that HttpsProxyAgent is a http.Agent, so maybe checking for instanceof http.Agent instead of https.Agent would work here? WDYT @kettanaito
Hi everyone, thanks for the investigation on this issue.
We perform an instanceof check because this.customAgent is options.agent from http.request, which can be boolean | http.Agent | undefined. The presence of that option doesn't guarantee it's actually an agent instance as it can also be false, indicating "please use the default agent" IIRC.
Looks like what happens is that this.customAgent is there, but it isn't an instance of https.Agent
This is either a bug on our end or improper construction of the HTTP request. The only instance this.customAgent || this yields the left-side expression is when this.customAgent is truthy, which is only possible if it's an instance of http.Agent (I believe true is never used as its boolean value, only false is expected there).
I am going to put the submitted reproduction (thank you!) to a test case and see what we can learn from it.
@ddolcimascolo, I think you're right:
https://github.com/TooTallNate/proxy-agents/blob/b923f5363875d8fd56477094d5ffc562840a4b76/packages/https-proxy-agent/src/index.ts#L62
https.Agent itself is a subset of http.Agent:
> new require('https').Agent() instanceof new require('http').Agent
true
But since we are performing an instance check against https.Agent (a narrower class), it might result in false for http.Agent, which is the case for https-proxy-agent. Nice find!
Opened a fix at #737. Would appreciate a review!
Released: v0.39.5 🎉
This has been released in v0.39.5!
Make sure to always update to the latest version (npm i @mswjs/interceptors@latest) to get the newest features and bug fixes.
Predictable release automation by @ossjs/release.
The fix for this has been released. I hope this unblocks everyone.
My nock example is still failing. They've pulled this update in and things still fail. Repro is here: https://github.com/nock/nock/issues/2828 but I'll repeat here (I have noted that the instanceof check that was added IS returning true).
Here's the repro with only nock and node:https raw request. No axios even: Don't even have to call nock. Just loading it.
'use strict';
const https = require('node:https');
const { HttpsProxyAgent } = require('https-proxy-agent');
const nock = require('nock');
const options = {
hostname: 'google.com',
port: 443,
path: '/',
method: 'GET',
agent: new HttpsProxyAgent('http://proxy:443'),
headers: {}
};
const req = https.request(options, incoming => {
let response = '';
incoming.on('data', chunk => response += chunk.toString() );
incoming.on('end', () => {
if (incoming.statusCode >= '400') {
reject( {
code: incoming.statusCode,
message: incoming.statusMessage,
body: response,
})
} else {
console.log(response);
}
});
});
req.on('error', err => reject(err));
req.end();
And here's the output:
/test/node_modules/agent-base/dist/index.js:154
throw new Error('No socket was returned in the `connect()` function');
^
Error: No socket was returned in the `connect()` function
at HttpsProxyAgent.createConnection (/test/node_modules/agent-base/dist/index.js:154:19)
at MockHttpSocket.passthrough (/test/node_modules/@mswjs/interceptors/lib/node/chunk-ATZKM2BZ.js:472:25)
at _ClientRequestInterceptor.onRequest (/test/node_modules/@mswjs/interceptors/lib/node/chunk-ATZKM2BZ.js:996:23)
at process.processTicksAndRejections (node:internal/process/task_queues:105:5)
Node.js v22.17.1
Was getting the _getSession error in our service, now getting this Error: No socket was returned in the connect() function one also.
@KrayzeeKev, thanks for letting me know. Looks like we'd have to add an explicit http-proxy-agent test since something else is going on here.
@silversf, do you experience this in combination with http-proxy-agent, too? If not, please submit a minimal reproduction repository. Let's make sure those issues are the same/different.
No socket was returned in the connect() function
It looks like HttpsProxyAgent relies on a custom Agent from agent-base, which expects the .createSocket() method of the agent to return a socket instance. Currently, MockHttpsAgent doesn't implement the .createSocket() method, which results in the No socket was returned in the connect() function error being throw by agent-base.
Conclusion
This issue cannot be addressed with the current implementation of Interceptors. See a more detailed explanation below.
Detailed explanation
Superficially, the error is caused by the mock agent not implementing the createSocket() method on which agent-base relies. But even if that method is implemented, supporting proxying as per https-proxy-agent is impossible.
The mock agent differentiates between two states:
- Added requests are considered mocked by default, thus a mock socket is returned for them until your request listeners (i.e. mocks) state that the outgoing request is unhandled and must be passthrough;
- Once request is determined as passthrough, it gets constructed as-is, meaning that the original (or the default) agent's flow kicks in (
customAgent.createConnection()).
It is also crucial to establish that ClientRequest is intercepted on the http module level. Requests constructed outside of that module, e.g. by direct net.connect() calls, will not be visible to the interceptor.
With such approach, proxying support requires that the proxy agent construct the proxy request using the http module as well. Sadly, that isn't the case for https-proxy-agent as it taps directly into tls.connect.
This means that even if I add the missing methods, the expected behavior will still not be fulfilled as the proxied request will always be unhandled, being invisible to the interceptor. Whereas the expectations are:
- Request A is intercepted, I have no handlers for it;
- Request A gets proxied to request B;
- Request B is intercepted, I have handlers for it, request A receives the mocked response from the handler for request B.
What now?
As I mentioned above, nothing we can do to support this as of now.
I believe the way to address this is implementing a socket-level interception, meaning intercepting not requests but socket instances, thus covering http, net, and tls simultaneously.
We've been in discussion regarding this and it seems that it would be good to draft out such a proposal. Given its complexity, however, it will take time and effort to get right. Mostly because socket-level interception implies ambiguous data packets transferred so we can no longer assume everything sent/received is HTTP messaged. We'd have to introduce HTTP parser not as a part of the socket but as a part of a interceptor-level predicate, requiring a significant rework of how interceptors work and combine (think BatchInterceptor with different parser predicates).
@kettanaito Thanks for the detailed analysis. Socket proxying is going to be quite an effort. What I am curious about is why it works fine in nock@13? It still uses this package, does it not? Or did it treat non-nocked destinations differently?
In other news: NodeJS v24.5 now supports proxies in both fetch and request via the standard agent. It relies on environment variables but you can construct the Agent to give it specific values to use for a specific instance. So https-proxy-agent would appear to be unnecessary from v24.5. It also seems like it'll get backported into v22 (and hopefully v20 as well) after sitting in v24 for a couple of weeks (LTS backporting policy). Since we run latest LTS as a rule, I'll need to wait until that happens. In the meanwhile, I continue to use nock@13.
@KrayzeeKev Nock v13 uses its implementation. Here is why v14 uses this package: https://github.com/nock/nock/pull/2517