Skip to content

Commit 849eec5

Browse files
committed
Improve JS interception when using ESM imports (e.g. node-fetch)
This is not a proper fix. We don't yet use loaded hooks, because they're experimental, apparently _extremely_ likely to change in future, and setting the option would cause issues with older node versions. We can look at this in future once the API has stabilised (by doing version detection in the PATH wrapper) but for now we can solve the immediate issue by just enabling global-agent 100% of the time regardless, which avoids issues with hooks not firing for ESM node-fetch. I think ESM usage in Node is still not widespread, so this should be enough to keep things usable in the short term.
1 parent 73f6538 commit 849eec5

File tree

3 files changed

+22
-25
lines changed

3 files changed

+22
-25
lines changed

overrides/js/prepend-node.js

Lines changed: 16 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -7,33 +7,11 @@
77
* plus tweaks various other HTTP clients that need nudges, so they
88
* all correctly pick up the proxy from the environment.
99
*
10-
* Tested against Node 6, 8, 10, 12 and 14.
11-
*/
10+
* Tested against Node 6, 8, 10, 12, 14, 16 & 18.
11+
*/
1212

1313
const wrapModule = require('./wrap-require');
1414

15-
let httpAlreadyIntercepted = false;
16-
17-
function interceptAllHttp() {
18-
if (httpAlreadyIntercepted) return;
19-
httpAlreadyIntercepted = true;
20-
21-
const MAJOR_NODEJS_VERSION = parseInt(process.version.slice(1).split('.')[0], 10);
22-
23-
if (MAJOR_NODEJS_VERSION >= 10) {
24-
// `global-agent` works with Node.js v10 and above.
25-
const globalAgent = require('global-agent');
26-
globalAgent.bootstrap();
27-
} else {
28-
// `global-tunnel-ng` works only with Node.js v10 and below.
29-
const globalTunnel = require('global-tunnel-ng');
30-
globalTunnel.initialize();
31-
}
32-
}
33-
34-
wrapModule('http', interceptAllHttp, true);
35-
wrapModule('https', interceptAllHttp, true);
36-
3715
wrapModule('axios', function wrapAxios (loadedModule) {
3816
// Global agent handles this automatically, if used (i.e. Node >= 10)
3917
if (global.GLOBAL_AGENT) return;
@@ -121,4 +99,17 @@ wrapModule('stripe', function wrapStripe (loadedModule) {
12199
loadedModule,
122100
{ INTERCEPTED_BY_HTTPTOOLKIT: true }
123101
);
124-
});
102+
});
103+
104+
// We always install a global HTTP agent, to ensure that everything using the base HTTP module is intercepted
105+
// by default. This avoids issues where hooks don't fire on ESM imports by just enabling this in all cases.
106+
const MAJOR_NODEJS_VERSION = parseInt(process.version.slice(1).split('.')[0], 10);
107+
if (MAJOR_NODEJS_VERSION >= 10) {
108+
// `global-agent` works with Node.js v10 and above.
109+
const globalAgent = require('global-agent');
110+
globalAgent.bootstrap();
111+
} else {
112+
// `global-tunnel-ng` works only with Node.js v10 and below.
113+
const globalTunnel = require('global-tunnel-ng');
114+
globalTunnel.initialize();
115+
}

overrides/js/wrap-require.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@
44
* This modules intercepts all require calls. For all modules previously
55
* registered via wrapModule, it runs the registered wrapper on the loaded
66
* module before it is returned to the original require() call.
7+
*
8+
* This only works for require(), not for ESM Import. Node.js import hooks
9+
* are very experimental and not yet usable, but we should extend this to
10+
* support those in future: https://github.com/nodejs/modules/issues/351
711
*/
812

913
// Grab the built-in module loader that we're going to intercept

test/fixtures/terminal/js-test-script.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
// Test built-ins against their specific protocol
2+
require('node:http').get('http://example.test/js/http');
3+
require('node:https').get('https://example.test/js/https');
24
require('http').get('http://example.test/js/http');
35
require('https').get('https://example.test/js/https');
46

0 commit comments

Comments
 (0)