push.rocks/smartproxy
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Add tests for connect-disconnect and error handling in SmartProxy

Browse Source
This commit is contained in:
Philipp Kunz
2025-06-01 14:41:19 +00:00
parent 2a75a86d73
commit 5b40e82c41
4 changed files with 654 additions and 107 deletions
Download Patch File Download Diff File Expand all files Collapse all files

61
readme.hints.md
View File

@ -523,4 +523,63 @@ if (!record) {
- **Consideration**: More frequent cleanup operations, but prevents queue backlog
### Migration Notes
No configuration changes needed. The improvements are automatic and backward compatible.
No configuration changes needed. The improvements are automatic and backward compatible.
## Early Client Disconnect Handling (v19.5.13+)
### Issue
Connections were accumulating when clients connected but disconnected before sending data or during routing. This occurred in two scenarios:
1. **TLS Path**: Clients connecting and disconnecting before sending initial TLS handshake data
2. **Non-TLS Immediate Routing**: Clients disconnecting while backend connection was being established
### Root Cause
1. **Missing Cleanup Handlers**: During initial data wait and immediate routing, no close/end handlers were attached to catch early disconnections
2. **Race Condition**: Backend connection attempts continued even after client disconnected, causing unhandled errors
3. **Timing Window**: Between accepting connection and establishing full bidirectional flow, disconnections weren't properly handled
### Solution
1. **TLS Path Fix**: Added close/end handlers during initial data wait (lines 224-253 in route-connection-handler.ts)
2. **Immediate Routing Fix**: Used `setupSocketHandlers` for proper handler attachment (lines 180-205)
3. **Backend Error Handling**: Check if connection already closed before handling backend errors (line 1144)
### Changes Made
```typescript
// 1. TLS path - handle disconnect before initial data
socket.once('close', () => {
if (!initialDataReceived) {
this.connectionManager.cleanupConnection(record, 'closed_before_data');
}
});
// 2. Immediate routing path - proper handler setup
setupSocketHandlers(socket, (reason) => {
if (!record.outgoing || record.outgoing.readyState !== 'open') {
if (record.outgoing && !record.outgoing.destroyed) {
record.outgoing.destroy(); // Abort pending backend connection
}
this.connectionManager.cleanupConnection(record, reason);
}
}, undefined, 'immediate-route-client');
// 3. Backend connection error handling
onError: (error) => {
if (record.connectionClosed) {
logger.log('debug', 'Backend connection failed but client already disconnected');
return; // Client already gone, nothing to clean up
}
// ... normal error handling
}
```
### Test Coverage
- `test/test.connect-disconnect-cleanup.node.ts` - Comprehensive test for early disconnect scenarios
- Tests verify connection count stays at 0 even with rapid connect/disconnect patterns
- Covers immediate disconnect, delayed disconnect, and mixed patterns
### Performance Impact
- **Positive**: No more connection accumulation from early disconnects
- **Positive**: Immediate cleanup reduces memory usage
- **Positive**: Prevents resource exhaustion from rapid reconnection attempts
### Migration Notes
No configuration changes needed. The fix is automatic and backward compatible.

242
test/test.connect-disconnect-cleanup.node.ts Normal file
View File

@ -0,0 +1,242 @@
import { tap, expect } from '@git.zone/tstest/tapbundle';
import * as net from 'net';
import * as plugins from '../ts/plugins.js';
// Import SmartProxy and configurations
import { SmartProxy } from '../ts/index.js';
tap.test('should handle clients that connect and immediately disconnect without sending data', async () => {
console.log('\n=== Testing Connect-Disconnect Cleanup ===');
// Create a SmartProxy instance
const proxy = new SmartProxy({
ports: [8560],
enableDetailedLogging: false,
initialDataTimeout: 5000, // 5 second timeout for initial data
routes: [{
name: 'test-route',
match: { ports: 8560 },
action: {
type: 'forward',
target: {
host: 'localhost',
port: 9999 // Non-existent port
}
}
}]
});
// Start the proxy
await proxy.start();
console.log('✓ Proxy started on port 8560');
// Helper to get active connection count
const getActiveConnections = () => {
const connectionManager = (proxy as any).connectionManager;
return connectionManager ? connectionManager.getConnectionCount() : 0;
};
const initialCount = getActiveConnections();
console.log(`Initial connection count: ${initialCount}`);
// Test 1: Connect and immediately disconnect without sending data
console.log('\n--- Test 1: Immediate disconnect ---');
const connectionCounts: number[] = [];
for (let i = 0; i < 10; i++) {
const client = new net.Socket();
// Connect and immediately destroy
client.connect(8560, 'localhost', () => {
// Connected - immediately destroy without sending data
client.destroy();
});
// Wait a tiny bit
await new Promise(resolve => setTimeout(resolve, 10));
const count = getActiveConnections();
connectionCounts.push(count);
if ((i + 1) % 5 === 0) {
console.log(`After ${i + 1} connect/disconnect cycles: ${count} active connections`);
}
}
// Wait a bit for cleanup
await new Promise(resolve => setTimeout(resolve, 500));
const afterImmediateDisconnect = getActiveConnections();
console.log(`After immediate disconnect test: ${afterImmediateDisconnect} active connections`);
// Test 2: Connect, wait a bit, then disconnect without sending data
console.log('\n--- Test 2: Delayed disconnect ---');
for (let i = 0; i < 5; i++) {
const client = new net.Socket();
client.on('error', () => {
// Ignore errors
});
client.connect(8560, 'localhost', () => {
// Wait 100ms then disconnect without sending data
setTimeout(() => {
if (!client.destroyed) {
client.destroy();
}
}, 100);
});
}
// Check count immediately
const duringDelayed = getActiveConnections();
console.log(`During delayed disconnect test: ${duringDelayed} active connections`);
// Wait for cleanup
await new Promise(resolve => setTimeout(resolve, 1000));
const afterDelayedDisconnect = getActiveConnections();
console.log(`After delayed disconnect test: ${afterDelayedDisconnect} active connections`);
// Test 3: Mix of immediate and delayed disconnects
console.log('\n--- Test 3: Mixed disconnect patterns ---');
const promises = [];
for (let i = 0; i < 20; i++) {
promises.push(new Promise<void>((resolve) => {
const client = new net.Socket();
client.on('error', () => {
resolve();
});
client.on('close', () => {
resolve();
});
client.connect(8560, 'localhost', () => {
if (i % 2 === 0) {
// Half disconnect immediately
client.destroy();
} else {
// Half wait 50ms
setTimeout(() => {
if (!client.destroyed) {
client.destroy();
}
}, 50);
}
});
// Failsafe timeout
setTimeout(() => resolve(), 200);
}));
}
// Wait for all to complete
await Promise.all(promises);
const duringMixed = getActiveConnections();
console.log(`During mixed test: ${duringMixed} active connections`);
// Final cleanup wait
await new Promise(resolve => setTimeout(resolve, 1000));
const finalCount = getActiveConnections();
console.log(`\nFinal connection count: ${finalCount}`);
// Stop the proxy
await proxy.stop();
console.log('✓ Proxy stopped');
// Verify all connections were cleaned up
expect(finalCount).toEqual(initialCount);
expect(afterImmediateDisconnect).toEqual(initialCount);
expect(afterDelayedDisconnect).toEqual(initialCount);
// Check that connections didn't accumulate during the test
const maxCount = Math.max(...connectionCounts);
console.log(`\nMax connection count during immediate disconnect test: ${maxCount}`);
expect(maxCount).toBeLessThan(3); // Should stay very low
console.log('\n✅ PASS: Connect-disconnect cleanup working correctly!');
});
tap.test('should handle clients that error during connection', async () => {
console.log('\n=== Testing Connection Error Cleanup ===');
const proxy = new SmartProxy({
ports: [8561],
enableDetailedLogging: false,
routes: [{
name: 'test-route',
match: { ports: 8561 },
action: {
type: 'forward',
target: {
host: 'localhost',
port: 9999
}
}
}]
});
await proxy.start();
console.log('✓ Proxy started on port 8561');
const getActiveConnections = () => {
const connectionManager = (proxy as any).connectionManager;
return connectionManager ? connectionManager.getConnectionCount() : 0;
};
const initialCount = getActiveConnections();
console.log(`Initial connection count: ${initialCount}`);
// Create connections that will error
const promises = [];
for (let i = 0; i < 10; i++) {
promises.push(new Promise<void>((resolve) => {
const client = new net.Socket();
client.on('error', () => {
resolve();
});
client.on('close', () => {
resolve();
});
// Connect to proxy
client.connect(8561, 'localhost', () => {
// Force an error by writing invalid data then destroying
try {
client.write(Buffer.alloc(1024 * 1024)); // Large write
client.destroy();
} catch (e) {
// Ignore
}
});
// Timeout
setTimeout(() => resolve(), 500);
}));
}
await Promise.all(promises);
console.log('✓ All error connections completed');
// Wait for cleanup
await new Promise(resolve => setTimeout(resolve, 500));
const finalCount = getActiveConnections();
console.log(`Final connection count: ${finalCount}`);
await proxy.stop();
console.log('✓ Proxy stopped');
expect(finalCount).toEqual(initialCount);
console.log('\n✅ PASS: Connection error cleanup working correctly!');
});
tap.start();

279
test/test.connection-cleanup-comprehensive.node.ts Normal file
View File

@ -0,0 +1,279 @@
import { tap, expect } from '@git.zone/tstest/tapbundle';
import * as net from 'net';
import * as plugins from '../ts/plugins.js';
// Import SmartProxy and configurations
import { SmartProxy } from '../ts/index.js';
tap.test('comprehensive connection cleanup test - all scenarios', async () => {
console.log('\n=== Comprehensive Connection Cleanup Test ===');
// Create a SmartProxy instance
const proxy = new SmartProxy({
ports: [8570, 8571], // One for immediate routing, one for TLS
enableDetailedLogging: false,
initialDataTimeout: 2000,
socketTimeout: 5000,
routes: [
{
name: 'non-tls-route',
match: { ports: 8570 },
action: {
type: 'forward',
target: {
host: 'localhost',
port: 9999 // Non-existent port
}
}
},
{
name: 'tls-route',
match: { ports: 8571 },
action: {
type: 'forward',
target: {
host: 'localhost',
port: 9999 // Non-existent port
},
tls: {
mode: 'passthrough'
}
}
}
]
});
// Start the proxy
await proxy.start();
console.log('✓ Proxy started on ports 8570 (non-TLS) and 8571 (TLS)');
// Helper to get active connection count
const getActiveConnections = () => {
const connectionManager = (proxy as any).connectionManager;
return connectionManager ? connectionManager.getConnectionCount() : 0;
};
const initialCount = getActiveConnections();
console.log(`Initial connection count: ${initialCount}`);
// Test 1: Rapid ECONNREFUSED retries (from original issue)
console.log('\n--- Test 1: Rapid ECONNREFUSED retries ---');
for (let i = 0; i < 10; i++) {
await new Promise<void>((resolve) => {
const client = new net.Socket();
client.on('error', () => {
client.destroy();
resolve();
});
client.on('close', () => {
resolve();
});
client.connect(8570, 'localhost', () => {
// Send data to trigger routing
client.write('GET / HTTP/1.1\r\nHost: test.com\r\n\r\n');
});
setTimeout(() => {
if (!client.destroyed) {
client.destroy();
}
resolve();
}, 100);
});
if ((i + 1) % 5 === 0) {
const count = getActiveConnections();
console.log(`After ${i + 1} ECONNREFUSED retries: ${count} active connections`);
}
}
// Test 2: Connect without sending data (immediate disconnect)
console.log('\n--- Test 2: Connect without sending data ---');
for (let i = 0; i < 10; i++) {
const client = new net.Socket();
client.on('error', () => {
// Ignore
});
// Connect to non-TLS port and immediately disconnect
client.connect(8570, 'localhost', () => {
client.destroy();
});
await new Promise(resolve => setTimeout(resolve, 10));
}
const afterNoData = getActiveConnections();
console.log(`After connect-without-data test: ${afterNoData} active connections`);
// Test 3: TLS connections that disconnect before handshake
console.log('\n--- Test 3: TLS early disconnect ---');
for (let i = 0; i < 10; i++) {
const client = new net.Socket();
client.on('error', () => {
// Ignore
});
// Connect to TLS port but disconnect before sending handshake
client.connect(8571, 'localhost', () => {
// Wait 50ms then disconnect (before initial data timeout)
setTimeout(() => {
client.destroy();
}, 50);
});
await new Promise(resolve => setTimeout(resolve, 100));
}
const afterTlsEarly = getActiveConnections();
console.log(`After TLS early disconnect test: ${afterTlsEarly} active connections`);
// Test 4: Mixed pattern - simulating real-world chaos
console.log('\n--- Test 4: Mixed chaos pattern ---');
const promises = [];
for (let i = 0; i < 30; i++) {
promises.push(new Promise<void>((resolve) => {
const client = new net.Socket();
const port = i % 2 === 0 ? 8570 : 8571;
client.on('error', () => {
resolve();
});
client.on('close', () => {
resolve();
});
client.connect(port, 'localhost', () => {
const scenario = i % 5;
switch (scenario) {
case 0:
// Immediate disconnect
client.destroy();
break;
case 1:
// Send data then disconnect
client.write('GET / HTTP/1.1\r\nHost: test.com\r\n\r\n');
setTimeout(() => client.destroy(), 20);
break;
case 2:
// Disconnect after delay
setTimeout(() => client.destroy(), 100);
break;
case 3:
// Send partial TLS handshake
if (port === 8571) {
client.write(Buffer.from([0x16, 0x03, 0x01])); // Partial TLS
}
setTimeout(() => client.destroy(), 50);
break;
case 4:
// Just let it timeout
break;
}
});
// Failsafe
setTimeout(() => {
if (!client.destroyed) {
client.destroy();
}
resolve();
}, 500);
}));
// Small delay between connections
if (i % 5 === 0) {
await new Promise(resolve => setTimeout(resolve, 10));
}
}
await Promise.all(promises);
console.log('✓ Chaos test completed');
// Wait for any cleanup
await new Promise(resolve => setTimeout(resolve, 1000));
const afterChaos = getActiveConnections();
console.log(`After chaos test: ${afterChaos} active connections`);
// Test 5: NFTables route (should cleanup properly)
console.log('\n--- Test 5: NFTables route cleanup ---');
const nftProxy = new SmartProxy({
ports: [8572],
enableDetailedLogging: false,
routes: [{
name: 'nftables-route',
match: { ports: 8572 },
action: {
type: 'forward',
forwardingEngine: 'nftables',
target: {
host: 'localhost',
port: 9999
}
}
}]
});
await nftProxy.start();
const getNftConnections = () => {
const connectionManager = (nftProxy as any).connectionManager;
return connectionManager ? connectionManager.getConnectionCount() : 0;
};
// Create NFTables connections
for (let i = 0; i < 5; i++) {
const client = new net.Socket();
client.on('error', () => {
// Ignore
});
client.connect(8572, 'localhost', () => {
setTimeout(() => client.destroy(), 50);
});
await new Promise(resolve => setTimeout(resolve, 100));
}
await new Promise(resolve => setTimeout(resolve, 500));
const nftFinal = getNftConnections();
console.log(`NFTables connections after test: ${nftFinal}`);
await nftProxy.stop();
// Final check on main proxy
const finalCount = getActiveConnections();
console.log(`\nFinal connection count: ${finalCount}`);
// Stop the proxy
await proxy.stop();
console.log('✓ Proxy stopped');
// Verify all connections were cleaned up
expect(finalCount).toEqual(initialCount);
expect(afterNoData).toEqual(initialCount);
expect(afterTlsEarly).toEqual(initialCount);
expect(afterChaos).toEqual(initialCount);
expect(nftFinal).toEqual(0);
console.log('\n✅ PASS: Comprehensive connection cleanup test passed!');
console.log('All connection scenarios properly cleaned up:');
console.log('- ECONNREFUSED rapid retries');
console.log('- Connect without sending data');
console.log('- TLS early disconnect');
console.log('- Mixed chaos patterns');
console.log('- NFTables connections');
});
tap.start();

179
ts/proxies/smart-proxy/route-connection-handler.ts
View File

@ -176,8 +176,33 @@ export class RouteConnectionHandler {
// If no routes require TLS handling and it's not port 443, route immediately
if (!needsTlsHandling && localPort !== 443) {
// Set up error handler
socket.on('error', this.connectionManager.handleError('incoming', record));
// Set up proper socket handlers for immediate routing
setupSocketHandlers(
socket,
(reason) => {
// Only cleanup if connection hasn't been fully established
// Check if outgoing connection exists and is connected
if (!record.outgoing || record.outgoing.readyState !== 'open') {
logger.log('debug', `Connection ${connectionId} closed during immediate routing: ${reason}`, {
connectionId,
remoteIP: record.remoteIP,
reason,
hasOutgoing: !!record.outgoing,
outgoingState: record.outgoing?.readyState,
component: 'route-handler'
});
// If there's a pending outgoing connection, destroy it
if (record.outgoing && !record.outgoing.destroyed) {
record.outgoing.destroy();
}
this.connectionManager.cleanupConnection(record, reason);
}
},
undefined, // Use default timeout handler
'immediate-route-client'
);
// Route immediately for non-TLS connections
this.routeConnection(socket, record, '', undefined);
@ -221,6 +246,37 @@ export class RouteConnectionHandler {
// Set up error handler
socket.on('error', this.connectionManager.handleError('incoming', record));
// Add close/end handlers to catch immediate disconnections
socket.once('close', () => {
if (!initialDataReceived) {
logger.log('warn', `Connection ${connectionId} closed before sending initial data`, {
connectionId,
remoteIP: record.remoteIP,
component: 'route-handler'
});
if (initialTimeout) {
clearTimeout(initialTimeout);
initialTimeout = null;
}
this.connectionManager.cleanupConnection(record, 'closed_before_data');
}
});
socket.once('end', () => {
if (!initialDataReceived) {
logger.log('debug', `Connection ${connectionId} ended before sending initial data`, {
connectionId,
remoteIP: record.remoteIP,
component: 'route-handler'
});
if (initialTimeout) {
clearTimeout(initialTimeout);
initialTimeout = null;
}
// Don't cleanup on 'end' - wait for 'close'
}
});
// First data handler to capture initial TLS handshake
socket.once('data', (chunk: Buffer) => {
// Clear the initial timeout since we've received data
@ -927,107 +983,6 @@ export class RouteConnectionHandler {
}
}
/**
* Setup improved error handling for the outgoing connection
* @deprecated This method is no longer used - error handling is done in createSocketWithErrorHandler
*/
private setupOutgoingErrorHandler(
connectionId: string,
targetSocket: plugins.net.Socket,
record: IConnectionRecord,
socket: plugins.net.Socket,
finalTargetHost: string,
finalTargetPort: number
): void {
targetSocket.once('error', (err) => {
// This handler runs only once during the initial connection phase
const code = (err as any).code;
logger.log('error',
`Connection setup error for ${connectionId} to ${finalTargetHost}:${finalTargetPort}: ${err.message} (${code})`,
{
connectionId,
targetHost: finalTargetHost,
targetPort: finalTargetPort,
errorMessage: err.message,
errorCode: code,
component: 'route-handler'
}
);
// Resume the incoming socket to prevent it from hanging
socket.resume();
// Log specific error types for easier debugging
if (code === 'ECONNREFUSED') {
logger.log('error',
`Connection ${connectionId}: Target ${finalTargetHost}:${finalTargetPort} refused connection. Check if the target service is running and listening on that port.`,
{
connectionId,
targetHost: finalTargetHost,
targetPort: finalTargetPort,
recommendation: 'Check if the target service is running and listening on that port.',
component: 'route-handler'
}
);
} else if (code === 'ETIMEDOUT') {
logger.log('error',
`Connection ${connectionId} to ${finalTargetHost}:${finalTargetPort} timed out. Check network conditions, firewall rules, or if the target is too far away.`,
{
connectionId,
targetHost: finalTargetHost,
targetPort: finalTargetPort,
recommendation: 'Check network conditions, firewall rules, or if the target is too far away.',
component: 'route-handler'
}
);
} else if (code === 'ECONNRESET') {
logger.log('error',
`Connection ${connectionId} to ${finalTargetHost}:${finalTargetPort} was reset. The target might have closed the connection abruptly.`,
{
connectionId,
targetHost: finalTargetHost,
targetPort: finalTargetPort,
recommendation: 'The target might have closed the connection abruptly.',
component: 'route-handler'
}
);
} else if (code === 'EHOSTUNREACH') {
logger.log('error',
`Connection ${connectionId}: Host ${finalTargetHost} is unreachable. Check DNS settings, network routing, or firewall rules.`,
{
connectionId,
targetHost: finalTargetHost,
recommendation: 'Check DNS settings, network routing, or firewall rules.',
component: 'route-handler'
}
);
} else if (code === 'ENOTFOUND') {
logger.log('error',
`Connection ${connectionId}: DNS lookup failed for ${finalTargetHost}. Check your DNS settings or if the hostname is correct.`,
{
connectionId,
targetHost: finalTargetHost,
recommendation: 'Check your DNS settings or if the hostname is correct.',
component: 'route-handler'
}
);
}
// Clear any existing error handler after connection phase
targetSocket.removeAllListeners('error');
// Re-add the normal error handler for established connections
targetSocket.on('error', this.connectionManager.handleError('outgoing', record));
if (record.outgoingTerminationReason === null) {
record.outgoingTerminationReason = 'connection_failed';
this.connectionManager.incrementTerminationStat('outgoing', 'connection_failed');
}
// Clean up the connection
this.connectionManager.initiateCleanupOnce(record, `connection_failed_${code}`);
});
}
/**
* Sets up a direct connection to the target
@ -1090,6 +1045,16 @@ export class RouteConnectionHandler {
host: finalTargetHost,
onError: (error) => {
// Connection failed - clean up everything immediately
// Check if connection record is still valid (client might have disconnected)
if (record.connectionClosed) {
logger.log('debug', `Backend connection failed but client already disconnected for ${connectionId}`, {
connectionId,
errorCode: (error as any).code,
component: 'route-handler'
});
return;
}
logger.log('error',
`Connection setup error for ${connectionId} to ${finalTargetHost}:${finalTargetPort}: ${error.message} (${(error as any).code})`,
{
@ -1117,10 +1082,12 @@ export class RouteConnectionHandler {
}
// Resume the incoming socket to prevent it from hanging
socket.resume();
if (socket && !socket.destroyed) {
socket.resume();
}
// Clean up the incoming socket
if (!socket.destroyed) {
if (socket && !socket.destroyed) {
socket.destroy();
}
@ -1294,7 +1261,7 @@ export class RouteConnectionHandler {
}
});
// Only set up basic properties - everything else happens in onConnect
// Set outgoing socket immediately so it can be cleaned up if client disconnects
record.outgoing = targetSocket;
record.outgoingStartTime = Date.now();