Terminal Intercepting crashes when using NodeJS worker threads

[leumasme]

HTTP Toolkit Terminal Interceptor currently crashes when executing a script that uses Worker threads

Exception

Worker error: Error [InvalidArgumentError]: Proxy opts.uri is mandatory
    at buildProxyOptions (S:\Apps\Scoop\apps\httptoolkit\1.13.0\resources\httptoolkit-server\overrides\js\node_modules\undici\lib\proxy-agent.js:28:11)
    at new ProxyAgent (S:\Apps\Scoop\apps\httptoolkit\1.13.0\resources\httptoolkit-server\overrides\js\node_modules\undici\lib\proxy-agent.js:40:20)
    at Object.wrapUndici [as wrap] (S:\Apps\Scoop\apps\httptoolkit\1.13.0\resources\httptoolkit-server\overrides\js\prepend-node.js:72:9)
    at fixModule (S:\Apps\Scoop\apps\httptoolkit\1.13.0\resources\httptoolkit-server\overrides\js\wrap-require.js:32:37)
    at mod._load (S:\Apps\Scoop\apps\httptoolkit\1.13.0\resources\httptoolkit-server\overrides\js\wrap-require.js:67:24)
    at Module.require (node:internal/modules/cjs/loader:1143:19)
    at require (node:internal/modules/cjs/helpers:110:18)
    at Object.<anonymous> (S:\Apps\Scoop\apps\httptoolkit\1.13.0\resources\httptoolkit-server\overrides\js\prepend-node.js:126:5)
    at Module._compile (node:internal/modules/cjs/loader:1256:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1310:10) {
  code: 'UND_ERR_INVALID_ARG'
}

The error originates here:

httptoolkit-server/overrides/js/prepend-node.js

Lines 61 to 71 in 89ba2ec

	wrapModule('undici', function wrapUndici (loadedModule) {
	const ProxyAgent = loadedModule.ProxyAgent;
	const setGlobalDispatcher = loadedModule.setGlobalDispatcher;
	// Old Undici release, which can't be intercepted:
	if (!ProxyAgent || !setGlobalDispatcher) return;
	setGlobalDispatcher(
	new ProxyAgent(process.env.HTTP_PROXY)
	);
	});

After some investigation, it turns out that you are setting the envoirement variable http_proxy, but in code attempt to access it as HTTP_PROXY. In a regular NodeJS context, this works fine since the process.env object does some getter magic to make property accessing case-insensitive. This is correct and expected, as envoirement variables on windows are intended to be case-insensitive.
Documented here: On Windows operating systems, environment variables are case-insensitive.

In worker threads, however, this behavior does not apply, and process.env.HTTP_PROXY returns undefined when the variable is called http_proxy, which causes the ProxyAgent to get called without an uri, causing the crash as seen above.

On new node versions this happens even without importing undici within the worker, since HTTP Toolkit automatically imports undici to proxy the global fetch object.

httptoolkit-server/overrides/js/prepend-node.js

Lines 117 to 124 in 89ba2ec

	if (MAJOR_NODEJS_VERSION >= 18 || global.fetch) {
	// Node 18 enables fetch by default (available previously behind a flag). This does not use
	// the existing agent API, so is not intercepted by global-agent. Instead, the only way to
	// set the HTTP proxy is to separately import Undici (used internally by Node) and configure
	// Undici's global proxy agent. We bundle our own Undici dep so we can do this reliably,
	// and here we import it to trigger the Undici setup hook defined above.
	require('undici');
	}

The worker threads documentation doesn't explicitly state wether process.env in workers should be case-sensitive or -insensitive, but it seems reasonable to assume that this detail was simply forgotten.
I'm thus going to create an issue in the NodeJS repo as well, however HTTP Toolkit should still implement a workaround for this (perhaps trying to get both the fully-uppercase and the fully-lowercase variant, or getting the keys of process.env and finding the one that case-insensitively matches the env var name to even find variables that are matched inconsistently, e.g. Http-Proxy)

Here's a repro of the issue:

Reproduction code

// main.js
const { Worker } = require("worker_threads");
const path = require("path");

const worker = new Worker(path.join(__dirname, "worker.js"));

console.log("in parent:")
console.log("http_proxy:",process.env.http_proxy)
console.log("HTTP_PROXY:",process.env.HTTP_PROXY)

worker.on("message", (data) => {
    console.log("in worker:")
    console.log("http_proxy:", data.http_proxy);
    console.log("HTTP_PROXY:", data.HTTP_PROXY);
});

// ---------

// worker.js
const { parentPort } = require('worker_threads');

parentPort.postMessage({
  http_proxy: process.env.http_proxy,
  HTTP_PROXY: process.env.HTTP_PROXY,
});

logs:

in parent:
http_proxy: test
HTTP_PROXY: test
in worker:
http_proxy: test
HTTP_PROXY: undefined

TLDR: process.env isn't case insensitive in NodeJS Worker Threads, which breaks the Terminal Interceptor. Make the terminal interceptor manually search for the "http_toolkit" variable in different casings as a workaround.

[pimterry]

After some investigation, it turns out that you are setting the envoirement variable http_proxy, but in code attempt to access it as HTTP_PROXY.

Technically, we're setting both, here:

httptoolkit-server/src/interceptors/terminal/terminal-env-overrides.ts

Lines 81 to 82 in 89ba2ec

	'http_proxy': proxyUrl,
	'HTTP_PROXY': proxyUrl,

That said, it totally makes sense to me that Windows collapses these into a single variable when it ignores case, and then that Node bug (thanks for filing that!) makes it case sensitive to the selected result.

I don't want to overcomplicate this (e.g. to handle all possible casings) since in theory the current approach is 'correct' (set both vars, read one of them, expect that to be set) and it seems there's a proper fix en route for the specific node bug anyway. For now, I've just added the simplest fix, which should resolve this for your specific Undici issue and also various other cases (e.g. global-agent reads HTTP_PROXY internally I think, which is presumably broken as well, though not actually crashing).

I've pushed that here: 0a3d0af. Can you give that a quick test and confirm it works for you?

[leumasme]

Yup, that patch fixes the issue for me!

[leumasme]

and it seems there's a proper fix en route for the specific node bug anyway.

Sadly it seems that the NodeJS team has decided to add this behavior to the docs instead of fixing it.
nodejs/node#49008
nodejs/node#48976 (review)