fix: log error message on socket only when there are no other handlers (#623)

Author: bliuchak

src/server.ts

@@ -249,7 +249,7 @@ export class Server extends EventEmitter {
         // See https://github.com/apify/proxy-chain/issues/53
         socket.on('error', (err) => {
             // Handle errors only if there's no other handler
-            if (this.listenerCount('error') === 1) {
+            if (socket.listenerCount('error') === 1) {
                 this.log(socket.proxyChainId, `Source socket emitted error: ${err.stack || err}`);
             }
         });

test/Dockerfile

@@ -1,22 +1,17 @@
 FROM node:18.20.8-bookworm@sha256:c6ae79e38498325db67193d391e6ec1d224d96c693a8a4d943498556716d3783
 
-RUN apt-get update && apt-get install -y --no-install-recommends chromium=142.0.7444.134-1~deb12u1 \
+RUN apt-get update && apt-get install -y --no-install-recommends chromium \
     && rm -rf /var/lib/apt/lists/*
 
 ENV PUPPETEER_SKIP_CHROMIUM_DOWNLOAD=true
 ENV PUPPETEER_EXECUTABLE_PATH=/usr/bin/chromium
 
 WORKDIR /home/node
 
-COPY .. .
+COPY --chown=node:node package*.json ./
+RUN npm --quiet set progress=false && npm install --no-optional
+COPY --chown=node:node . .
 
-RUN npm --quiet set progress=false \
-    && npm install --no-optional \
-    && echo "Installed NPM packages:" \
-    && npm list || true \
-    && echo "Node.js version:" \
-    && node --version \
-    && echo "NPM version:" \
-    && npm --version
+USER node
 
-CMD ["npm", "test"]
+ENTRYPOINT [ "npm", "test", "--" ]

test/README.md

@@ -5,9 +5,23 @@
 Since Linux and macOS handle sockets differently, please run tests in a Docker container
 to have a consistent Linux environment for running tests.
 
-```bash
-npm run test:docker
-```
+1. Run all tests
+
+    ```bash
+    npm run test:docker
+    ```
+
+2. Run a specific test file
+
+    ```bash
+    npm run test:docker test/server.js
+    ```
+
+3. Run all `direct ipv6` test cases across all tests
+
+    ```bash
+    npm run test:docker test/server.js -- --grep "direct ipv6"
+    ```
 
 Note: for test in Docker no changes in `/etc/hosts` needed.
 
@@ -39,7 +53,7 @@ Note: for test in Docker no changes in `/etc/hosts` needed.
     npm run test
     ```
 
-2. Run specific tests
+2. Run a specific test file
 
     ```bash
     npm run test test/anonymize_proxy.js

test/server.js

@@ -1647,3 +1647,63 @@ useSslVariants.forEach((useSsl) => {
         });
     });
 });
+
+describe('Socket error handler regression test', () => {
+    let server;
+    let logs = [];
+    const originalLog = console.log;
+
+    before(() => {
+        console.log = (...args) => {
+            logs.push(args.join(' '));
+            originalLog.apply(console, args);
+        };
+    });
+
+    after(() => {
+        console.log = originalLog;
+    });
+
+    beforeEach(() => {
+        logs = [];
+    });
+
+    afterEach(async () => {
+        if (server) {
+            await server.close(true);
+            server = null;
+        }
+    });
+
+    // The bug was checking `this.listenerCount('error')` (Server) instead of `socket.listenerCount('error')`.
+    // By adding an error listener to the Server, we make server.listenerCount('error') === 1.
+    // With buggy code: condition becomes TRUE (1 === 1) and incorrectly logs.
+    // With fixed code: condition stays FALSE (socket has 2 listeners, 2 !== 1) and correctly doesn't log.
+    it('does not log when server has 1 error listener but socket has multiple', (done) => {
+        server = new Server({ port: 0, verbose: true });
+
+        server.on('error', () => {});
+
+        server.server.once('connection', (serverSocket) => {
+            setImmediate(() => {
+                expect(server.listenerCount('error')).to.equal(1);
+                expect(serverSocket.listenerCount('error')).to.equal(2);
+
+                serverSocket.emit('error', new Error('Regression test error'));
+
+                setTimeout(() => {
+                    const hasLog = logs.some((log) => log.includes('Source socket emitted error') && log.includes('Regression test error'));
+
+                    expect(hasLog).to.equal(false, 'Should check socket.listenerCount, not this.listenerCount (server)');
+
+                    serverSocket.destroy();
+                    done();
+                }, 50);
+            });
+        });
+
+        server.listen().then(() => {
+            net.connect(server.port, '127.0.0.1');
+        });
+    });
+});