Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`);
}
});
Expand Down
17 changes: 6 additions & 11 deletions test/Dockerfile
Original file line number Diff line number Diff line change
@@ -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", "--" ]
22 changes: 18 additions & 4 deletions test/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -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
Expand Down
60 changes: 60 additions & 0 deletions test/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});
});
Loading