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

joyeecheung · targos · commit 8f32746142ff · 2025-09-24T09:03:38.000+02:00
In the testing proxy server for proxy client tests, the proxy client might have already closed the connection when the upstream connection fails. In that case, there's no need for the proxy server to inform the proxy client about the error. PR-URL: #59742 Fixes: #59741 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
diff --git a/test/client-proxy/test-https-proxy-request-invalid-char-in-url.mjs b/test/client-proxy/test-https-proxy-request-invalid-char-in-url.mjs
@@ -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)}`);
   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);
@@ -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) {
@@ -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();
diff --git a/test/common/proxy-server.js b/test/common/proxy-server.js
@@ -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 });
@@ -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, () => {
@@ -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);
+      }
     });
   });
 
