Skip to content

test: guard write to proxy client if proxy connection is ended #59742

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 17, 2025
+25 −15
Merged

test: guard write to proxy client if proxy connection is ended #59742

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
41eb0a6
test: guard write to proxy client if proxy connection is ended
joyeecheung Sep 3, 2025
096f459
fixup! test: guard write to proxy client if proxy connection is ended
joyeecheung Sep 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 16 additions & 10 deletions test/client-proxy/test-https-proxy-request-invalid-char-in-url.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,10 @@ const server = https.createServer({
cert: fixtures.readKey('agent8-cert.pem'),
key: fixtures.readKey('agent8-key.pem'),
}, (req, res) => {
console.log(`[Upstream server] responding to request for ${inspect(req.url)}`);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are the console.log's important to the test or just debugging output?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are just for debugging, but would be useful if it flakes in the CI again (I never see the problem of logging in tests, the lack of it is more problematic when dealing with flakes in the CI..)

requests.add(`https://localhost:${server.address().port}${req.url}`);
res.writeHead(200, { 'Content-Type': 'text/plain' });
res.end(`Response for ${req.url}`);
res.end(`Response for ${inspect(req.url)}`);
});

server.listen(0);
Expand Down Expand Up @@ -54,7 +55,7 @@ https.globalAgent = new https.Agent({

const severHost = `localhost:${server.address().port}`;

let counter = testCases.length;
let counter = 0;
const expectedUrls = new Set();
const expectedProxyLogs = new Set();
for (const testCase of testCases) {
Expand All @@ -69,15 +70,20 @@ for (const testCase of testCases) {
https.request(url, (res) => {
res.on('error', common.mustNotCall());
res.setEncoding('utf8');
res.on('data', () => {});
res.on('end', common.mustCall(() => {
console.log(`#${counter--} eneded response for: ${inspect(url)}`);
res.on('data', (data) => {
console.log(`[Proxy client] Received response from server for ${inspect(url)}: ${data.toString()}`);
});
res.on('close', common.mustCall(() => {
console.log(`[Proxy client] #${++counter} closed request for: ${inspect(url)}`);
// Finished all test cases.
if (counter === 0) {
proxy.close();
server.close();
assert.deepStrictEqual(requests, expectedUrls);
assert.deepStrictEqual(new Set(logs), expectedProxyLogs);
if (counter === testCases.length) {
setImmediate(() => {
console.log('All requests completed, shutting down.');
proxy.close();
server.close();
assert.deepStrictEqual(requests, expectedUrls);
assert.deepStrictEqual(new Set(logs), expectedProxyLogs);
});
}
}));
}).on('error', common.mustNotCall()).end();
Expand Down
14 changes: 9 additions & 5 deletions test/common/proxy-server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ exports.createProxyServer = function(options = {}) {
});

res.on('error', (err) => {
logs.push({ error: err, source: 'proxy response' });
logs.push({ error: err, source: 'client response for request' });
});

req.pipe(proxyReq, { end: true });
Expand All @@ -75,7 +75,7 @@ exports.createProxyServer = function(options = {}) {
const [hostname, port] = req.url.split(':');

res.on('error', (err) => {
logs.push({ error: err, source: 'proxy response' });
logs.push({ error: err, source: 'client response for connect' });
});

const proxyReq = net.connect(port, hostname, () => {
Expand All @@ -90,9 +90,13 @@ exports.createProxyServer = function(options = {}) {
});

proxyReq.on('error', (err) => {
logs.push({ error: err, source: 'proxy request' });
res.write('HTTP/1.1 500 Connection Error\r\n\r\n');
res.end('Proxy error: ' + err.message);
logs.push({ error: err, source: 'proxy connect' });
// The proxy client might have already closed the connection
// when the upstream connection fails.
if (!res.writableEnded) {
res.write('HTTP/1.1 500 Connection Error\r\n\r\n');
res.end('Proxy error: ' + err.message);
}
});
});

Expand Down
Loading