Calling server.closeConnections() or server.close(true) prevents stats collection

[marcplouhinec]

I’m using proxy-chain to monitor proxy traffic consumption, but I noticed that I was undercounting the number of transmitted bytes.

Before starting to use the proxy, I register a listener on the connectionClosed event like this:

server.on('connectionClosed', ({ connectionId, stats }) => {
    if (stats) {
        // Process statistics
    }
});

After finishing with the proxy, I call server.close(true). However, when the connectionClosed events are fired, the stats are always undefined.

Upon investigating the implementation of server.close(), I found that it calls server.closeConnections() when the first parameter is true.

The issue appears to be caused by this line in server.ts, line 636:

    closeConnections(): void {
        this.log(null, 'Closing pending sockets');

        for (const socket of this.connections.values()) {
            socket.destroy();
        }

        this.connections.clear(); // <-- The problem

        this.log(null, `Destroyed ${this.connections.size} pending sockets`);
    }

Unlike the closeConnection(connectionId: number) method just above in the same file, this function clears the this.connections map. I’m not sure why this line was added, or if removing it has any negative side effects, but typically each entry in this map is removed by the socket's close event listener, as seen in server.registerConnection() line 215:

    registerConnection(socket: Socket): void {
        // ...

        socket.on('close', () => {
            this.emit('connectionClosed', {
                connectionId: unique,
                stats: this.getConnectionStats(unique), // <--  Stats are obtained here
            });

            this.connections.delete(unique); // <-- Connection is deleted after emitting stats
        });
        // ..
    }

It’s critical that this.connections is modified after server.getConnectionStats() is called:

    getConnectionStats(connectionId: number): ConnectionStats | undefined {
        const socket = this.connections.get(connectionId); // <-- Needs the entry here; otherwise, stats are undefined
        if (!socket) return undefined;

        const targetStats = getTargetStats(socket);

        const result = {
            srcTxBytes: socket.bytesWritten,
            srcRxBytes: socket.bytesRead,
            trgTxBytes: targetStats.bytesWritten,
            trgRxBytes: targetStats.bytesRead,
        };

        return result;
    }

In summary, calling server.closeConnections() or server.close(true) prevents stats collection by clearing this.connections before server.getConnectionStats() can retrieve the necessary information.

Workaround

Currently, I avoid the issue by manually closing connections like this:

const connectionIds = server.getConnectionIds();
for (const connectionId of connectionIds) {
    server.closeConnection(connectionId);
}
await server.close(false);

Proposed Fix

I suggest removing this line in server.ts. If this solution is acceptable, I can submit a pull request with the fix. What do you think?

[jancurn]

Hey @marcplouhinec, thank you for your report and suggested fix. Your proposed fix makes sense - please can you prepare a pull request?

[marcplouhinec]

Hi @jancurn , ok, please wait

[marcplouhinec]

@jancurn Here is my pull request: #556

[jirimoravcik]

Hello, the PR has been merged and a new version 2.5.4 has been released. Thanks for your contribution!