From 02603c3b0729567cee3241b975a6c3eb3cb3a235 Mon Sep 17 00:00:00 2001 From: Philipp Kunz Date: Sat, 31 May 2025 17:14:15 +0000 Subject: [PATCH] fix(performance): start with planning performance optimizations --- package.json | 2 +- pnpm-lock.yaml | 69 +-- readme.hints.md | 26 +- readme.plan.md | 1455 +++++++++++++++++++++++++++++++++++--------- readme.plan2.md | 764 ----------------------- readme.problems.md | 220 ++++--- 6 files changed, 1397 insertions(+), 1139 deletions(-) delete mode 100644 readme.plan2.md diff --git a/package.json b/package.json index 95058e3..4c39fb4 100644 --- a/package.json +++ b/package.json @@ -18,7 +18,7 @@ "@git.zone/tsbuild": "^2.6.4", "@git.zone/tsrun": "^1.2.44", "@git.zone/tstest": "^2.3.1", - "@types/node": "^22.15.24", + "@types/node": "^22.15.29", "typescript": "^5.8.3" }, "dependencies": { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 09ef281..e8e0048 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -70,8 +70,8 @@ importers: specifier: ^2.3.1 version: 2.3.1(@aws-sdk/credential-providers@3.798.0)(socks@2.8.4)(typescript@5.8.3) '@types/node': - specifier: ^22.15.24 - version: 22.15.24 + specifier: ^22.15.29 + version: 22.15.29 typescript: specifier: ^5.8.3 version: 5.8.3 @@ -1635,11 +1635,11 @@ packages: '@types/node-forge@1.3.11': resolution: {integrity: sha512-FQx220y22OKNTqaByeBGqHWYz4cl94tpcxeFdvBo3wjG6XPBuZ0BNgNZRV5J5TFmmcsJ4IzsLkmGRiQbnYsBEQ==} - '@types/node@18.19.105': - resolution: {integrity: sha512-a+DrwD2VyzqQR2W0EVF8EaCh6Em4ilQAYLEPZnMNkQHXR7ziWW7RUhZMWZAgRpkDDAdUIcJOXSPJT/zBEwz3sA==} + '@types/node@18.19.110': + resolution: {integrity: sha512-WW2o4gTmREtSnqKty9nhqF/vA0GKd0V/rbC0OyjSk9Bz6bzlsXKT+i7WDdS/a0z74rfT2PO4dArVCSnapNLA5Q==} - '@types/node@22.15.24': - resolution: {integrity: sha512-w9CZGm9RDjzTh/D+hFwlBJ3ziUaVw7oufKA3vOFSOZlzmW9AkZnfjPb+DLnrV6qtgL/LNmP0/2zBNCFHL3F0ng==} + '@types/node@22.15.29': + resolution: {integrity: sha512-LNdjOkUDlU1RZb8e1kOIUpN1qQUlzGkEtbVNo53vbrwDg5om6oduhm4SiUaPW5ASTXhAiP0jInWG8Qx9fVlOeQ==} '@types/ping@0.4.4': resolution: {integrity: sha512-ifvo6w2f5eJYlXm+HiVx67iJe8WZp87sfa683nlqED5Vnt9Z93onkokNoWqOG21EaE8fMxyKPobE+mkPEyxsdw==} @@ -5708,6 +5708,7 @@ snapshots: - '@aws-sdk/credential-providers' - '@mongodb-js/zstd' - '@nuxt/kit' + - aws-crt - encoding - gcp-metadata - kerberos @@ -7097,27 +7098,27 @@ snapshots: '@types/bn.js@5.1.6': dependencies: - '@types/node': 22.15.24 + '@types/node': 22.15.29 '@types/body-parser@1.19.5': dependencies: '@types/connect': 3.4.38 - '@types/node': 22.15.24 + '@types/node': 22.15.29 '@types/buffer-json@2.0.3': {} '@types/clean-css@4.2.11': dependencies: - '@types/node': 22.15.24 + '@types/node': 22.15.29 source-map: 0.6.1 '@types/connect@3.4.38': dependencies: - '@types/node': 22.15.24 + '@types/node': 22.15.29 '@types/cors@2.8.18': dependencies: - '@types/node': 22.15.24 + '@types/node': 22.15.29 '@types/debug@4.1.12': dependencies: @@ -7129,7 +7130,7 @@ snapshots: '@types/dns-packet@5.6.5': dependencies: - '@types/node': 22.15.24 + '@types/node': 22.15.29 '@types/elliptic@6.4.18': dependencies: @@ -7137,7 +7138,7 @@ snapshots: '@types/express-serve-static-core@5.0.6': dependencies: - '@types/node': 22.15.24 + '@types/node': 22.15.29 '@types/qs': 6.9.18 '@types/range-parser': 1.2.7 '@types/send': 0.17.4 @@ -7154,30 +7155,30 @@ snapshots: '@types/from2@2.3.5': dependencies: - '@types/node': 22.15.24 + '@types/node': 22.15.29 '@types/fs-extra@11.0.4': dependencies: '@types/jsonfile': 6.1.4 - '@types/node': 22.15.24 + '@types/node': 22.15.29 '@types/fs-extra@9.0.13': dependencies: - '@types/node': 22.15.24 + '@types/node': 22.15.29 '@types/glob@7.2.0': dependencies: '@types/minimatch': 5.1.2 - '@types/node': 22.15.24 + '@types/node': 22.15.29 '@types/glob@8.1.0': dependencies: '@types/minimatch': 5.1.2 - '@types/node': 22.15.24 + '@types/node': 22.15.29 '@types/gunzip-maybe@1.4.2': dependencies: - '@types/node': 22.15.24 + '@types/node': 22.15.29 '@types/hast@3.0.4': dependencies: @@ -7199,7 +7200,7 @@ snapshots: '@types/jsonfile@6.1.4': dependencies: - '@types/node': 22.15.24 + '@types/node': 22.15.29 '@types/mdast@4.0.4': dependencies: @@ -7217,18 +7218,18 @@ snapshots: '@types/node-fetch@2.6.12': dependencies: - '@types/node': 22.15.24 + '@types/node': 22.15.29 form-data: 4.0.2 '@types/node-forge@1.3.11': dependencies: - '@types/node': 22.15.24 + '@types/node': 22.15.29 - '@types/node@18.19.105': + '@types/node@18.19.110': dependencies: undici-types: 5.26.5 - '@types/node@22.15.24': + '@types/node@22.15.29': dependencies: undici-types: 6.21.0 @@ -7244,30 +7245,30 @@ snapshots: '@types/s3rver@3.7.4': dependencies: - '@types/node': 22.15.24 + '@types/node': 22.15.29 '@types/semver@7.7.0': {} '@types/send@0.17.4': dependencies: '@types/mime': 1.3.5 - '@types/node': 22.15.24 + '@types/node': 22.15.29 '@types/serve-static@1.15.7': dependencies: '@types/http-errors': 2.0.4 - '@types/node': 22.15.24 + '@types/node': 22.15.29 '@types/send': 0.17.4 '@types/symbol-tree@3.2.5': {} '@types/tar-stream@2.2.3': dependencies: - '@types/node': 22.15.24 + '@types/node': 22.15.29 '@types/through2@2.0.41': dependencies: - '@types/node': 22.15.24 + '@types/node': 22.15.29 '@types/triple-beam@1.3.5': {} @@ -7291,18 +7292,18 @@ snapshots: '@types/whatwg-url@8.2.2': dependencies: - '@types/node': 22.15.24 + '@types/node': 22.15.29 '@types/webidl-conversions': 7.0.3 '@types/which@3.0.4': {} '@types/ws@8.18.1': dependencies: - '@types/node': 22.15.24 + '@types/node': 22.15.29 '@types/yauzl@2.10.3': dependencies: - '@types/node': 22.15.24 + '@types/node': 22.15.29 optional: true '@ungap/structured-clone@1.3.0': {} @@ -7582,7 +7583,7 @@ snapshots: cloudflare@4.2.0: dependencies: - '@types/node': 18.19.105 + '@types/node': 18.19.110 '@types/node-fetch': 2.6.12 abort-controller: 3.0.0 agentkeepalive: 4.6.0 @@ -7835,7 +7836,7 @@ snapshots: engine.io@6.6.4: dependencies: '@types/cors': 2.8.18 - '@types/node': 22.15.24 + '@types/node': 22.15.29 accepts: 1.3.8 base64id: 2.0.0 cookie: 0.7.2 diff --git a/readme.hints.md b/readme.hints.md index a1ae2be..b595d0c 100644 --- a/readme.hints.md +++ b/readme.hints.md @@ -318,4 +318,28 @@ const routes: IRouteConfig[] = [{ - Authentication requires TLS termination (cannot be enforced on passthrough/direct connections) - Per-route connection limits are not yet implemented - Security is defined at the route level (route.security), not in the action -- Route matching is based solely on match criteria; security is enforced after matching \ No newline at end of file +- Route matching is based solely on match criteria; security is enforced after matching + +## Performance Issues Investigation (v19.5.3+) + +### Critical Blocking Operations Found +1. **Busy Wait Loop** in `ts/proxies/nftables-proxy/nftables-proxy.ts:235-238` + - Blocks entire event loop with `while (Date.now() < waitUntil) {}` + - Should use `await new Promise(resolve => setTimeout(resolve, delay))` + +2. **Synchronous Filesystem Operations** + - Certificate management uses `fs.existsSync()`, `fs.mkdirSync()`, `fs.readFileSync()` + - NFTables proxy uses `execSync()` for system commands + - Certificate store uses `ensureDirSync()`, `fileExistsSync()`, `removeManySync()` + +3. **Memory Leak Risks** + - Several `setInterval()` calls without storing references for cleanup + - Event listeners added without proper cleanup in error paths + - Missing `removeAllListeners()` calls in some connection cleanup scenarios + +### Performance Recommendations +- Replace all sync filesystem operations with async alternatives +- Fix the busy wait loop immediately (critical event loop blocker) +- Add proper cleanup for all timers and event listeners +- Consider worker threads for CPU-intensive operations +- See `readme.problems.md` for detailed analysis and recommendations \ No newline at end of file diff --git a/readme.plan.md b/readme.plan.md index 672b5e0..ed2b11f 100644 --- a/readme.plan.md +++ b/readme.plan.md @@ -1,316 +1,1229 @@ -# SmartProxy Development Plan +# SmartProxy Performance Optimization Plan -## Implementation Plan: Socket Handler Function Support (Simplified) ✅ COMPLETED +## Executive Summary -### Overview -Add support for custom socket handler functions with the simplest possible API - just pass a function that receives the socket. +This plan addresses critical performance issues in SmartProxy that impact scalability, responsiveness, and stability. The approach is phased, starting with critical event loop blockers and progressing to long-term architectural improvements. -### User Experience Goal +## Phase 1: Critical Issues (Week 1) + +### 1.1 Eliminate Busy Wait Loop + +**Issue**: `ts/proxies/nftables-proxy/nftables-proxy.ts:235-238` blocks the entire event loop + +**Solution**: ```typescript -const proxy = new SmartProxy({ - routes: [{ - name: 'my-custom-protocol', - match: { ports: 9000, domains: 'custom.example.com' }, - action: { - type: 'socket-handler', - socketHandler: (socket) => { - // User has full control of the socket - socket.write('Welcome!\n'); - socket.on('data', (data) => { - socket.write(`Echo: ${data}`); - }); - } - } - }] -}); -``` - -That's it. Simple and powerful. - ---- - -## Phase 1: Minimal Type Changes - -### 1.1 Add Socket Handler Action Type -**File:** `ts/proxies/smart-proxy/models/route-types.ts` - -```typescript -// Update action type -export type TRouteActionType = 'forward' | 'redirect' | 'block' | 'static' | 'socket-handler'; - -// Add simple socket handler type -export type TSocketHandler = (socket: net.Socket) => void | Promise; - -// Extend IRouteAction -export interface IRouteAction { - // ... existing properties - - // Socket handler function (when type is 'socket-handler') - socketHandler?: TSocketHandler; +// Create utility function in ts/core/utils/async-utils.ts +export async function delay(ms: number): Promise { + return new Promise(resolve => setTimeout(resolve, ms)); } + +// Replace busy wait in nftables-proxy.ts +// OLD: +const waitUntil = Date.now() + retryDelayMs; +while (Date.now() < waitUntil) { } + +// NEW: +await delay(retryDelayMs); ``` ---- +**Implementation**: +1. Create `async-utils.ts` with common async utilities +2. Replace all synchronous sleeps with async delay +3. Ensure all calling functions are async -## Phase 2: Simple Implementation +### 1.2 Async Filesystem Operations -### 2.1 Update Route Connection Handler -**File:** `ts/proxies/smart-proxy/route-connection-handler.ts` - -In the `handleConnection` method, add handling for socket-handler: +**Issue**: Multiple synchronous filesystem operations blocking the event loop +**Solution Architecture**: ```typescript -// After route matching... -if (matchedRoute) { - const action = matchedRoute.action; - - if (action.type === 'socket-handler') { - if (!action.socketHandler) { - logger.error('socket-handler action missing socketHandler function'); - socket.destroy(); - return; - } - +// Create ts/core/utils/fs-utils.ts +import * as plugins from '../../plugins.js'; + +export class AsyncFileSystem { + static async exists(path: string): Promise { try { - // Simply call the handler with the socket - const result = action.socketHandler(socket); + await plugins.fs.promises.access(path); + return true; + } catch { + return false; + } + } + + static async ensureDir(path: string): Promise { + await plugins.fs.promises.mkdir(path, { recursive: true }); + } + + static async readFile(path: string): Promise { + return plugins.fs.promises.readFile(path, 'utf8'); + } + + static async writeFile(path: string, data: string): Promise { + // Ensure directory exists + const dir = plugins.path.dirname(path); + await this.ensureDir(dir); + await plugins.fs.promises.writeFile(path, data); + } + + static async remove(path: string): Promise { + try { + await plugins.fs.promises.unlink(path); + } catch (error: any) { + if (error.code !== 'ENOENT') throw error; + } + } + + static async readJSON(path: string): Promise { + const content = await this.readFile(path); + return JSON.parse(content); + } + + static async writeJSON(path: string, data: any): Promise { + await this.writeFile(path, JSON.stringify(data, null, 2)); + } +} +``` + +**Migration Strategy**: + +1. **Certificate Manager** (`ts/proxies/http-proxy/certificate-manager.ts`) + ```typescript + // OLD: + constructor(private options: IHttpProxyOptions) { + if (!fs.existsSync(this.certDir)) { + fs.mkdirSync(this.certDir, { recursive: true }); + } + } + + // NEW: + private initialized = false; + + constructor(private options: IHttpProxyOptions) {} + + async initialize(): Promise { + if (this.initialized) return; + await AsyncFileSystem.ensureDir(this.certDir); + this.initialized = true; + } + + async getCertificate(domain: string): Promise<{ cert: string; key: string } | null> { + await this.initialize(); + const certPath = path.join(this.certDir, `${domain}.crt`); + const keyPath = path.join(this.certDir, `${domain}.key`); + + if (await AsyncFileSystem.exists(certPath) && await AsyncFileSystem.exists(keyPath)) { + const [cert, key] = await Promise.all([ + AsyncFileSystem.readFile(certPath), + AsyncFileSystem.readFile(keyPath) + ]); + return { cert, key }; + } + return null; + } + ``` + +2. **Certificate Store** (`ts/proxies/smart-proxy/cert-store.ts`) + ```typescript + // Convert all methods to async + export class CertStore { + constructor(private storePath: string) {} + + async init(): Promise { + await AsyncFileSystem.ensureDir(this.storePath); + } + + async hasCertificate(domain: string): Promise { + const certPath = this.getCertificatePath(domain); + return AsyncFileSystem.exists(certPath); + } + + async getCertificate(domain: string): Promise { + if (!await this.hasCertificate(domain)) return null; + + const metaPath = path.join(this.getCertificatePath(domain), 'meta.json'); + return AsyncFileSystem.readJSON(metaPath); + } + } + ``` + +3. **NFTables Proxy** (`ts/proxies/nftables-proxy/nftables-proxy.ts`) + ```typescript + // Replace execSync with execAsync + private async execNftCommand(command: string): Promise { + const maxRetries = 3; + let lastError: Error | null = null; + + for (let i = 0; i < maxRetries; i++) { + try { + const { stdout } = await this.execAsync(command); + return stdout; + } catch (err: any) { + lastError = err; + if (i < maxRetries - 1) { + await delay(this.retryDelayMs); + } + } + } + + throw new NftExecutionError(`Failed after ${maxRetries} attempts: ${lastError?.message}`); + } + ``` + +## Dependencies Between Phases + +### Critical Path +``` +Phase 1.1 (Busy Wait) ─┐ + ├─> Phase 2.1 (Timer Management) ─> Phase 3.2 (Worker Threads) +Phase 1.2 (Async FS) ──┘ │ + ├─> Phase 4.1 (Monitoring) +Phase 2.2 (Connection Pool) ────────────────────────────┘ +``` + +### Phase Dependencies +- **Phase 1** must complete before Phase 2 (foundation for async operations) +- **Phase 2.1** enables proper cleanup for Phase 3.2 worker threads +- **Phase 3** optimizations depend on stable async foundation +- **Phase 4** monitoring requires all components to be instrumented + +## Phase 2: Resource Management (Week 2) + +### 2.1 Timer Lifecycle Management + +**Issue**: Timers created without cleanup references causing memory leaks + +**Solution Pattern**: +```typescript +// Create base class in ts/core/utils/lifecycle-component.ts +export abstract class LifecycleComponent { + private timers: Set = new Set(); + private listeners: Array<{ target: any, event: string, handler: Function }> = []; + protected isShuttingDown = false; + + protected setInterval(handler: Function, timeout: number): NodeJS.Timeout { + const timer = setInterval(() => { + if (!this.isShuttingDown) { + handler(); + } + }, timeout); + this.timers.add(timer); + return timer; + } + + protected setTimeout(handler: Function, timeout: number): NodeJS.Timeout { + const timer = setTimeout(() => { + this.timers.delete(timer); + if (!this.isShuttingDown) { + handler(); + } + }, timeout); + this.timers.add(timer); + return timer; + } + + protected addEventListener(target: any, event: string, handler: Function): void { + target.on(event, handler); + this.listeners.push({ target, event, handler }); + } + + protected async cleanup(): Promise { + this.isShuttingDown = true; + + // Clear all timers + for (const timer of this.timers) { + clearInterval(timer); + clearTimeout(timer); + } + this.timers.clear(); + + // Remove all listeners + for (const { target, event, handler } of this.listeners) { + target.removeListener(event, handler); + } + this.listeners = []; + } +} +``` + +**Implementation**: +1. Extend LifecycleComponent in: + - `HttpProxy` + - `SmartProxy` + - `ConnectionManager` + - `RequestHandler` + - `SharedSecurityManager` + +2. Replace direct timer/listener usage with lifecycle methods + +### 2.2 Connection Pool Enhancement + +**Issue**: No backpressure mechanism and synchronous operations + +**Solution**: +```typescript +// First, implement efficient BinaryHeap for O(log n) operations +// Create ts/core/utils/binary-heap.ts +export class BinaryHeap { + private heap: T[] = []; + + constructor( + private compareFn: (a: T, b: T) => number, + private extractKey?: (item: T) => string + ) {} + + insert(item: T): void { + this.heap.push(item); + this.bubbleUp(this.heap.length - 1); + } + + extract(): T | undefined { + if (this.heap.length === 0) return undefined; + if (this.heap.length === 1) return this.heap.pop(); + + const result = this.heap[0]; + this.heap[0] = this.heap.pop()!; + this.bubbleDown(0); + return result; + } + + extractIf(predicate: (item: T) => boolean): T | undefined { + const index = this.heap.findIndex(predicate); + if (index === -1) return undefined; + + if (index === this.heap.length - 1) return this.heap.pop(); + + const result = this.heap[index]; + this.heap[index] = this.heap.pop()!; + + // Restore heap property + this.bubbleUp(index); + this.bubbleDown(index); + return result; + } + + sizeFor(key: string): number { + if (!this.extractKey) return this.heap.length; + return this.heap.filter(item => this.extractKey!(item) === key).length; + } + + private bubbleUp(index: number): void { + while (index > 0) { + const parentIndex = Math.floor((index - 1) / 2); + if (this.compareFn(this.heap[index], this.heap[parentIndex]) >= 0) break; - // If it returns a promise, handle errors - if (result instanceof Promise) { - result.catch(error => { - logger.error('Socket handler error:', error); - if (!socket.destroyed) { - socket.destroy(); - } - }); + [this.heap[index], this.heap[parentIndex]] = + [this.heap[parentIndex], this.heap[index]]; + index = parentIndex; + } + } + + private bubbleDown(index: number): void { + while (true) { + let minIndex = index; + const leftChild = 2 * index + 1; + const rightChild = 2 * index + 2; + + if (leftChild < this.heap.length && + this.compareFn(this.heap[leftChild], this.heap[minIndex]) < 0) { + minIndex = leftChild; } - } catch (error) { - logger.error('Socket handler error:', error); - if (!socket.destroyed) { - socket.destroy(); + + if (rightChild < this.heap.length && + this.compareFn(this.heap[rightChild], this.heap[minIndex]) < 0) { + minIndex = rightChild; } + + if (minIndex === index) break; + + [this.heap[index], this.heap[minIndex]] = + [this.heap[minIndex], this.heap[index]]; + index = minIndex; + } + } +} + +// Enhanced connection pool with queue and heap +export class EnhancedConnectionPool extends LifecycleComponent { + private connectionQueue: Array<{ + resolve: (socket: net.Socket) => void; + reject: (error: Error) => void; + host: string; + port: number; + timestamp: number; + }> = []; + + private connectionHeap: BinaryHeap; + private metricsCollector: ConnectionMetrics; + + constructor(options: IConnectionPoolOptions) { + super(); + + // Priority: least recently used connections first + this.connectionHeap = new BinaryHeap( + (a, b) => a.lastUsed - b.lastUsed, + (item) => item.poolKey + ); + + this.metricsCollector = new ConnectionMetrics(); + this.startQueueProcessor(); + } + + private startQueueProcessor(): void { + // Process queue periodically to handle timeouts and retries + this.setInterval(() => { + const now = Date.now(); + const timeout = this.options.connectionQueueTimeout || 30000; + + // Remove timed out requests + this.connectionQueue = this.connectionQueue.filter(item => { + if (now - item.timestamp > timeout) { + item.reject(new Error(`Connection pool timeout for ${item.host}:${item.port}`)); + this.metricsCollector.recordTimeout(); + return false; + } + return true; + }); + + // Try to fulfill queued requests + this.processQueue(); + }, 1000); + } + + private processQueue(): void { + if (this.connectionQueue.length === 0) return; + + // Group by destination + const grouped = new Map(); + + for (const item of this.connectionQueue) { + const key = `${item.host}:${item.port}`; + if (!grouped.has(key)) grouped.set(key, []); + grouped.get(key)!.push(item); } - return; // Done - user has control now - } - - // ... rest of existing action handling -} -``` - ---- - -## Phase 3: Optional Context (If Needed) - -If users need more info, we can optionally pass a minimal context as a second parameter: - -```typescript -export type TSocketHandler = ( - socket: net.Socket, - context?: { - route: IRouteConfig; - clientIp: string; - localPort: number; - } -) => void | Promise; -``` - -Usage: -```typescript -socketHandler: (socket, context) => { - console.log(`Connection from ${context.clientIp} to port ${context.localPort}`); - // Handle socket... -} -``` - ---- - -## Phase 4: Helper Utilities (Optional) - -### 4.1 Common Patterns -**File:** `ts/proxies/smart-proxy/utils/route-helpers.ts` - -```typescript -// Simple helper to create socket handler routes -export function createSocketHandlerRoute( - domains: string | string[], - ports: TPortRange, - handler: TSocketHandler, - options?: { name?: string; priority?: number } -): IRouteConfig { - return { - name: options?.name || 'socket-handler-route', - priority: options?.priority || 50, - match: { domains, ports }, - action: { - type: 'socket-handler', - socketHandler: handler - } - }; -} - -// Pre-built handlers for common cases -export const SocketHandlers = { - // Simple echo server - echo: (socket: net.Socket) => { - socket.on('data', data => socket.write(data)); - }, - - // TCP proxy - proxy: (targetHost: string, targetPort: number) => (socket: net.Socket) => { - const target = net.connect(targetPort, targetHost); - socket.pipe(target); - target.pipe(socket); - socket.on('close', () => target.destroy()); - target.on('close', () => socket.destroy()); - }, - - // Line-based protocol - lineProtocol: (handler: (line: string, socket: net.Socket) => void) => (socket: net.Socket) => { - let buffer = ''; - socket.on('data', (data) => { - buffer += data.toString(); - const lines = buffer.split('\n'); - buffer = lines.pop() || ''; - lines.forEach(line => handler(line, socket)); - }); - } -}; -``` - ---- - -## Usage Examples - -### Example 1: Custom Protocol -```typescript -{ - name: 'custom-protocol', - match: { ports: 9000 }, - action: { - type: 'socket-handler', - socketHandler: (socket) => { - socket.write('READY\n'); - socket.on('data', (data) => { - const cmd = data.toString().trim(); - if (cmd === 'PING') socket.write('PONG\n'); - else if (cmd === 'QUIT') socket.end(); - else socket.write('ERROR: Unknown command\n'); - }); - } - } -} -``` - -### Example 2: Simple TCP Proxy -```typescript -{ - name: 'tcp-proxy', - match: { ports: 8080, domains: 'proxy.example.com' }, - action: { - type: 'socket-handler', - socketHandler: SocketHandlers.proxy('backend.local', 3000) - } -} -``` - -### Example 3: WebSocket with Custom Auth -```typescript -{ - name: 'custom-websocket', - match: { ports: [80, 443], path: '/ws' }, - action: { - type: 'socket-handler', - socketHandler: async (socket) => { - // Read HTTP headers - const headers = await readHttpHeaders(socket); + // Try to fulfill requests for each destination + for (const [poolKey, requests] of grouped) { + const available = this.connectionHeap.extractIf( + conn => conn.poolKey === poolKey && conn.isIdle && !conn.socket.destroyed + ); - // Custom auth check - if (!headers.authorization || !validateToken(headers.authorization)) { - socket.write('HTTP/1.1 401 Unauthorized\r\n\r\n'); - socket.end(); + if (available) { + const request = requests.shift()!; + this.connectionQueue = this.connectionQueue.filter(item => item !== request); + + available.isIdle = false; + available.lastUsed = Date.now(); + request.resolve(available.socket); + + this.metricsCollector.recordReuse(); + } + } + } + + async getConnection(host: string, port: number): Promise { + const poolKey = `${host}:${port}`; + + // Try to get existing connection + let connection = this.connectionHeap.extractIf( + conn => conn.poolKey === poolKey && conn.isIdle && !conn.socket.destroyed + ); + + if (connection) { + connection.isIdle = false; + connection.lastUsed = Date.now(); + this.metricsCollector.recordReuse(); + + // Validate connection is still alive + if (await this.validateConnection(connection.socket)) { + return connection.socket; + } + + // Connection is dead, try another + connection.socket.destroy(); + return this.getConnection(host, port); + } + + // Check pool size + const poolSize = this.connectionHeap.sizeFor(poolKey); + if (poolSize < this.options.connectionPoolSize) { + return this.createConnection(host, port); + } + + // Queue the request + return this.queueConnectionRequest(host, port); + } + + private async validateConnection(socket: net.Socket): Promise { + return new Promise((resolve) => { + if (socket.destroyed || !socket.readable || !socket.writable) { + resolve(false); return; } - // Proceed with WebSocket upgrade - const ws = new WebSocket(socket, headers); - // ... handle WebSocket + // Try to write a TCP keepalive probe + const originalWrite = socket.write; + let writeError = false; + + socket.write = function(data: any, encoding?: any, cb?: any) { + writeError = true; + return false; + }; + + socket.setNoDelay(true); + socket.setNoDelay(false); + + socket.write = originalWrite; + + resolve(!writeError); + }); + } + + returnConnection(socket: net.Socket, host: string, port: number): void { + const poolKey = `${host}:${port}`; + + // Check for queued requests first + const queuedIndex = this.connectionQueue.findIndex( + item => item.host === host && item.port === port + ); + + if (queuedIndex >= 0) { + const queued = this.connectionQueue.splice(queuedIndex, 1)[0]; + queued.resolve(socket); + this.metricsCollector.recordDirectHandoff(); + return; + } + + // Return to pool + this.connectionHeap.insert({ + socket, + poolKey, + lastUsed: Date.now(), + isIdle: true, + created: Date.now() + }); + } + + getMetrics(): IConnectionPoolMetrics { + return { + ...this.metricsCollector.getMetrics(), + poolSize: this.connectionHeap.size(), + queueLength: this.connectionQueue.length + }; + } +} +``` + +## Phase 3: Performance Optimizations (Week 3) + +### 3.1 JSON Operations Optimization + +**Issue**: Frequent JSON.stringify for cache keys + +**Solution**: +```typescript +// Create ts/core/utils/hash-utils.ts +import * as crypto from 'crypto'; + +export class HashUtils { + private static readonly objectCache = new WeakMap(); + + static hashObject(obj: any): string { + // Check cache first + if (typeof obj === 'object' && obj !== null) { + const cached = this.objectCache.get(obj); + if (cached) return cached; + } + + // Create stable string representation + const str = this.stableStringify(obj); + const hash = crypto.createHash('sha256').update(str).digest('hex').slice(0, 16); + + // Cache if object + if (typeof obj === 'object' && obj !== null) { + this.objectCache.set(obj, hash); + } + + return hash; + } + + private static stableStringify(obj: any): string { + if (obj === null || typeof obj !== 'object') { + return JSON.stringify(obj); + } + + if (Array.isArray(obj)) { + return '[' + obj.map(item => this.stableStringify(item)).join(',') + ']'; + } + + const keys = Object.keys(obj).sort(); + const pairs = keys.map(key => `"${key}":${this.stableStringify(obj[key])}`); + return '{' + pairs.join(',') + '}'; + } +} + +// Update function-cache.ts +private computeContextHash(context: IRouteContext): string { + return HashUtils.hashObject({ + domain: context.domain, + path: context.path, + clientIp: context.clientIp + }); +} +``` + +### 3.2 Worker Thread Integration + +**Issue**: CPU-intensive operations blocking event loop + +**Solution Architecture**: +```typescript +// Create ts/core/workers/worker-pool.ts +import { Worker } from 'worker_threads'; + +export class WorkerPool { + private workers: Worker[] = []; + private queue: Array<{ + task: any; + resolve: Function; + reject: Function; + }> = []; + private busyWorkers = new Set(); + + constructor( + private workerScript: string, + private poolSize: number = 4 + ) { + this.initializeWorkers(); + } + + async execute(task: any): Promise { + const worker = await this.getAvailableWorker(); + + return new Promise((resolve, reject) => { + const messageHandler = (result: any) => { + worker.off('message', messageHandler); + worker.off('error', errorHandler); + this.releaseWorker(worker); + resolve(result); + }; + + const errorHandler = (error: Error) => { + worker.off('message', messageHandler); + worker.off('error', errorHandler); + this.releaseWorker(worker); + reject(error); + }; + + worker.on('message', messageHandler); + worker.on('error', errorHandler); + worker.postMessage(task); + }); + } +} + +// Create ts/core/workers/nftables-worker.ts +import { parentPort } from 'worker_threads'; +import { exec } from 'child_process'; +import { promisify } from 'util'; + +const execAsync = promisify(exec); + +parentPort?.on('message', async (task) => { + try { + const result = await execAsync(task.command); + parentPort?.postMessage({ success: true, result }); + } catch (error) { + parentPort?.postMessage({ success: false, error: error.message }); + } +}); +``` + +## Phase 4: Monitoring & Metrics (Week 4) + +### 4.1 Event Loop Monitoring + +```typescript +// Create ts/core/monitoring/performance-monitor.ts +export class PerformanceMonitor extends LifecycleComponent { + private metrics = { + eventLoopLag: [] as number[], + activeConnections: 0, + memoryUsage: {} as NodeJS.MemoryUsage, + cpuUsage: {} as NodeJS.CpuUsage + }; + + start() { + // Monitor event loop lag + let lastCheck = process.hrtime.bigint(); + + this.setInterval(() => { + const now = process.hrtime.bigint(); + const expectedInterval = 100n * 1000000n; // 100ms in nanoseconds + const actualInterval = now - lastCheck; + const lag = Number(actualInterval - expectedInterval) / 1000000; // Convert to ms + + this.metrics.eventLoopLag.push(lag); + if (this.metrics.eventLoopLag.length > 100) { + this.metrics.eventLoopLag.shift(); + } + + lastCheck = now; + }, 100); + + // Monitor system resources + this.setInterval(() => { + this.metrics.memoryUsage = process.memoryUsage(); + this.metrics.cpuUsage = process.cpuUsage(); + }, 5000); + } + + getMetrics() { + const avgLag = this.metrics.eventLoopLag.reduce((a, b) => a + b, 0) + / this.metrics.eventLoopLag.length; + + return { + eventLoopLag: { + current: this.metrics.eventLoopLag[this.metrics.eventLoopLag.length - 1], + average: avgLag, + max: Math.max(...this.metrics.eventLoopLag) + }, + memory: this.metrics.memoryUsage, + cpu: this.metrics.cpuUsage, + activeConnections: this.metrics.activeConnections + }; + } +} +``` + +## Testing Strategy + +### Unit Tests +1. Create tests for each new utility class +2. Mock filesystem and network operations +3. Test error scenarios and edge cases + +### Integration Tests +1. Test async migration with real filesystem +2. Verify timer cleanup on shutdown +3. Test connection pool under load + +### Performance Tests +```typescript +// Create test/performance/event-loop-test.ts +import { tap, expect } from '@git.zone/tstest/tapbundle'; + +tap.test('should not block event loop', async () => { + const intervals: number[] = []; + let lastTime = Date.now(); + + const timer = setInterval(() => { + const now = Date.now(); + intervals.push(now - lastTime); + lastTime = now; + }, 10); + + // Run operations that might block + await runPotentiallyBlockingOperation(); + + clearInterval(timer); + + // Check that no interval exceeded 50ms (allowing some tolerance) + const maxInterval = Math.max(...intervals); + expect(maxInterval).toBeLessThan(50); +}); +``` + +## Migration Timeline + +### Week 1: Critical Fixes +- Day 1-2: Fix busy wait loop +- Day 3-4: Convert critical sync operations +- Day 5: Testing and validation + +### Week 2: Resource Management +- Day 1-2: Implement LifecycleComponent +- Day 3-4: Migrate components +- Day 5: Connection pool enhancement + +### Week 3: Optimizations +- Day 1-2: JSON operation optimization +- Day 3-4: Worker thread integration +- Day 5: Performance testing + +### Week 4: Monitoring & Polish +- Day 1-2: Performance monitoring +- Day 3-4: Load testing +- Day 5: Documentation and release + +## Error Handling Strategy + +### Graceful Degradation +```typescript +// Create ts/core/utils/error-handler.ts +export class ErrorHandler { + private static errorCounts = new Map(); + private static circuitBreakers = new Map(); + + static async withFallback( + operation: () => Promise, + fallback: () => Promise, + context: string + ): Promise { + const breaker = this.getCircuitBreaker(context); + + if (breaker.isOpen()) { + return fallback(); + } + + try { + const result = await operation(); + breaker.recordSuccess(); + return result; + } catch (error) { + breaker.recordFailure(); + this.recordError(context, error); + + if (breaker.isOpen()) { + logger.warn(`Circuit breaker opened for ${context}`); + } + + return fallback(); + } + } + + private static getCircuitBreaker(context: string): CircuitBreaker { + if (!this.circuitBreakers.has(context)) { + this.circuitBreakers.set(context, new CircuitBreaker({ + failureThreshold: 5, + resetTimeout: 60000 + })); + } + return this.circuitBreakers.get(context)!; + } +} + +// Usage example in Certificate Manager +async getCertificate(domain: string): Promise { + return ErrorHandler.withFallback( + // Try async operation + async () => { + await this.initialize(); + return this.loadCertificateAsync(domain); + }, + // Fallback to sync if needed + async () => { + logger.warn(`Falling back to sync certificate load for ${domain}`); + return this.loadCertificateSync(domain); + }, + 'certificate-load' + ); +} +``` + +## Backward Compatibility + +### API Preservation +1. **Maintain existing interfaces** - All public APIs remain unchanged +2. **Progressive enhancement** - New features are opt-in via configuration +3. **Sync method wrappers** - Provide sync-looking APIs that use async internally + +```typescript +// Example: Maintaining backward compatibility +export class CertStore { + // Old sync API (deprecated but maintained) + getCertificateSync(domain: string): ICertificateInfo | null { + console.warn('getCertificateSync is deprecated, use getCertificate'); + return this.syncFallbackGetCertificate(domain); + } + + // New async API + async getCertificate(domain: string): Promise { + return this.asyncGetCertificate(domain); + } + + // Smart detection for gradual migration + getCertificateAuto(domain: string, callback?: (err: Error | null, cert: ICertificateInfo | null) => void) { + if (callback) { + // Callback style for compatibility + this.getCertificate(domain) + .then(cert => callback(null, cert)) + .catch(err => callback(err, null)); + } else { + // Return promise for modern usage + return this.getCertificate(domain); } } } ``` ---- +### Configuration Compatibility +```typescript +// Support both old and new configuration formats +interface SmartProxyOptions { + // Old options (maintained) + preserveSourceIP?: boolean; + defaultAllowedIPs?: string[]; + + // New performance options (added) + performance?: { + asyncFilesystem?: boolean; + enhancedConnectionPool?: boolean; + workerThreads?: boolean; + }; +} +``` -## Benefits of This Approach +## Monitoring Dashboard -1. **Dead Simple API**: Just pass a function that gets the socket -2. **No New Classes**: No ForwardingHandler subclass needed -3. **Minimal Changes**: Only touches type definitions and one handler method -4. **Full Power**: Users have complete control over the socket -5. **Backward Compatible**: No changes to existing functionality -6. **Easy to Test**: Just test the socket handler functions directly +### Real-time Metrics Visualization +```typescript +// Create ts/core/monitoring/dashboard-server.ts +export class MonitoringDashboard { + private httpServer: http.Server; + private wsServer: WebSocket.Server; + private metricsHistory: MetricsHistory; ---- + async start(port: number = 9090): Promise { + this.httpServer = http.createServer(this.handleRequest.bind(this)); + this.wsServer = new WebSocket.Server({ server: this.httpServer }); + + this.wsServer.on('connection', (ws) => { + // Send current metrics + ws.send(JSON.stringify({ + type: 'initial', + data: this.metricsHistory.getLast(100) + })); + + // Subscribe to updates + const interval = setInterval(() => { + if (ws.readyState === WebSocket.OPEN) { + ws.send(JSON.stringify({ + type: 'update', + data: this.performanceMonitor.getMetrics() + })); + } + }, 1000); + + ws.on('close', () => clearInterval(interval)); + }); + + this.httpServer.listen(port); + logger.info(`Monitoring dashboard available at http://localhost:${port}`); + } -## Implementation Steps + private handleRequest(req: http.IncomingMessage, res: http.ServerResponse) { + if (req.url === '/') { + res.writeHead(200, { 'Content-Type': 'text/html' }); + res.end(this.getDashboardHTML()); + } else if (req.url === '/metrics') { + res.writeHead(200, { 'Content-Type': 'application/json' }); + res.end(JSON.stringify(this.performanceMonitor.getMetrics())); + } + } -1. Add `'socket-handler'` to `TRouteActionType` (5 minutes) -2. Add `socketHandler?: TSocketHandler` to `IRouteAction` (5 minutes) -3. Add socket-handler case in `RouteConnectionHandler.handleConnection()` (15 minutes) -4. Add helper functions (optional, 30 minutes) -5. Write tests (2 hours) -6. Update documentation (1 hour) + private getDashboardHTML(): string { + return ` + + + + SmartProxy Performance Monitor + + + + +

SmartProxy Performance Monitor

+ +
+
+

Event Loop Lag

+
--
+
+
+

Active Connections

+
--
+
+
+

Memory Usage

+
--
+
+
+

Connection Pool

+
--
+
+
+ +
+ +
+
+ +
+ + + + + `; + } +} +``` -**Total implementation time: ~4 hours** (vs 6 weeks for the complex version) +## Performance Benchmarking ---- +### Benchmark Suite +```typescript +// Create test/performance/benchmark.ts +import { SmartProxy } from '../../ts/index.js'; -## What We're NOT Doing +export class PerformanceBenchmark { + async runConnectionStresTest(): Promise { + const proxy = new SmartProxy({ /* config */ }); + await proxy.start(); + + const results = { + connectionRate: 0, + avgLatency: 0, + maxConnections: 0, + eventLoopLag: [] + }; + + // Monitor event loop during test + const lagSamples: number[] = []; + let lastCheck = process.hrtime.bigint(); + const monitor = setInterval(() => { + const now = process.hrtime.bigint(); + const lag = Number(now - lastCheck - 100_000_000n) / 1_000_000; + lagSamples.push(lag); + lastCheck = now; + }, 100); + + // Create connections with increasing rate + const startTime = Date.now(); + let connectionCount = 0; + + for (let rate = 100; rate <= 10000; rate += 100) { + const connections = await this.createConnections(rate); + connectionCount += connections.length; + + // Check if performance degrades + const avgLag = lagSamples.slice(-10).reduce((a, b) => a + b) / 10; + if (avgLag > 50) { + results.maxConnections = connectionCount; + break; + } + + await this.delay(1000); + } + + clearInterval(monitor); + await proxy.stop(); + + results.connectionRate = connectionCount / ((Date.now() - startTime) / 1000); + results.avgLatency = this.calculateAvgLatency(); + results.eventLoopLag = lagSamples; + + return results; + } +} +``` -- ❌ Creating new ForwardingHandler classes -- ❌ Complex context objects with utils -- ❌ HTTP request handling for socket handlers -- ❌ Complex protocol detection mechanisms -- ❌ Middleware patterns -- ❌ Lifecycle hooks +## Documentation Updates -Keep it simple. The user just wants to handle a socket. +### API Documentation +1. **Update JSDoc comments** for all modified methods +2. **Add migration guide** for async transitions +3. **Performance tuning guide** with recommended settings ---- +### Example Updates +```typescript +/** + * Gets a certificate for the specified domain + * @param domain - The domain to get certificate for + * @returns Promise resolving to certificate info or null + * @since v20.0.0 - Now returns Promise (breaking change) + * @example + * // Old way (deprecated) + * const cert = certStore.getCertificateSync('example.com'); + * + * // New way + * const cert = await certStore.getCertificate('example.com'); + * + * // Compatibility mode + * certStore.getCertificateAuto('example.com', (err, cert) => { + * if (err) console.error(err); + * else console.log(cert); + * }); + */ +async getCertificate(domain: string): Promise { + // Implementation +} +``` -## Success Criteria +## Rollback Strategy -- ✅ Users can define a route with `type: 'socket-handler'` -- ✅ Users can provide a function that receives the socket -- ✅ The function is called when a connection matches the route -- ✅ Error handling prevents crashes -- ✅ No performance impact on existing routes -- ✅ Clean, simple API that's easy to understand +Each phase is designed to be independently deployable with feature flags: ---- +```typescript +export const PerformanceFlags = { + useAsyncFilesystem: process.env.SMARTPROXY_ASYNC_FS !== 'false', + useEnhancedPool: process.env.SMARTPROXY_ENHANCED_POOL === 'true', + useWorkerThreads: process.env.SMARTPROXY_WORKERS === 'true', + enableMonitoring: process.env.SMARTPROXY_MONITORING === 'true' +}; +``` -## Implementation Notes (Completed) +### Gradual Rollout Plan +1. **Development**: All flags enabled +2. **Staging**: Monitor metrics for 1 week +3. **Production**: + - 10% traffic → 25% → 50% → 100% + - Monitor key metrics at each stage + - Rollback if metrics degrade -### What Was Implemented -1. **Type Definitions** - Added 'socket-handler' to TRouteActionType and TSocketHandler type -2. **Route Handler** - Added socket-handler case in RouteConnectionHandler switch statement -3. **Error Handling** - Both sync and async errors are caught and logged -4. **Initial Data Handling** - Initial chunks are re-emitted to handler's listeners -5. **Helper Functions** - Added createSocketHandlerRoute and pre-built handlers (echo, proxy, etc.) -6. **Full Test Coverage** - All test cases pass including async handlers and error handling +## Success Metrics -### Key Implementation Details -- Socket handlers require initial data from client to trigger routing (not TLS handshake) -- The handler receives the raw socket after route matching -- Both sync and async handlers are supported -- Errors in handlers terminate the connection gracefully -- Helper utilities provide common patterns (echo server, TCP proxy, line protocol) +1. **Event Loop Lag**: < 10ms average, < 50ms max +2. **Connection Handling**: Support 10k+ concurrent connections +3. **Memory Usage**: Stable under sustained load +4. **CPU Usage**: Efficient utilization across cores +5. **Response Time**: < 5ms overhead for proxy operations -### Usage Notes -- Clients must send initial data to trigger the handler (even just a newline) -- The socket is passed directly to the handler function -- Handler has complete control over the socket lifecycle -- No special context object needed - keeps it simple +## Risk Mitigation -**Total implementation time: ~3 hours** \ No newline at end of file +1. **Backward Compatibility**: Maintain existing APIs +2. **Gradual Rollout**: Use feature flags +3. **Monitoring**: Track metrics before/after changes +4. **Testing**: Comprehensive test coverage +5. **Documentation**: Update all API docs + +## Summary of Key Optimizations + +### Immediate Impact (Phase 1) +1. **Eliminate busy wait loop** - Unblocks event loop immediately +2. **Async filesystem operations** - Prevents I/O blocking +3. **Proper error handling** - Graceful degradation with fallbacks + +### Performance Enhancements (Phase 2-3) +1. **Enhanced connection pooling** - O(log n) operations with BinaryHeap +2. **Resource lifecycle management** - Prevents memory leaks +3. **Worker threads** - Offloads CPU-intensive operations +4. **Optimized JSON operations** - Reduces parsing overhead + +### Monitoring & Validation (Phase 4) +1. **Real-time dashboard** - Visual performance monitoring +2. **Event loop lag tracking** - Early warning system +3. **Automated benchmarking** - Regression prevention + +## Implementation Checklist + +### Phase 1: Critical Fixes (Priority: URGENT) +- [ ] Create `ts/core/utils/async-utils.ts` with delay function +- [ ] Fix busy wait loop in `nftables-proxy.ts` +- [ ] Create `ts/core/utils/fs-utils.ts` with AsyncFileSystem class +- [ ] Migrate `certificate-manager.ts` to async operations +- [ ] Migrate `cert-store.ts` to async operations +- [ ] Replace `execSync` with `execAsync` in `nftables-proxy.ts` +- [ ] Add comprehensive unit tests for async operations +- [ ] Performance test to verify event loop improvements + +### Phase 2: Resource Management +- [ ] Implement LifecycleComponent base class +- [ ] Migrate components to extend LifecycleComponent +- [ ] Implement BinaryHeap data structure +- [ ] Create EnhancedConnectionPool with queue support +- [ ] Add connection validation and health checks +- [ ] Implement proper timer cleanup in all components +- [ ] Add integration tests for resource management + +### Phase 3: Performance Optimizations +- [ ] Implement HashUtils for efficient object hashing +- [ ] Create WorkerPool for CPU-intensive operations +- [ ] Migrate NFTables operations to worker threads +- [ ] Optimize JSON operations with caching +- [ ] Add performance benchmarks + +### Phase 4: Monitoring & Polish +- [ ] Implement PerformanceMonitor class +- [ ] Create monitoring dashboard with WebSocket updates +- [ ] Add comprehensive metrics collection +- [ ] Document all API changes +- [ ] Create migration guide +- [ ] Update examples and tutorials + +## Next Steps + +1. **Immediate Action**: Fix the busy wait loop (blocks entire event loop) +2. **Code Review**: Review this plan with the team +3. **Feature Branch**: Create `feature/performance-optimization` +4. **Phase 1 Implementation**: Complete within 1 week +5. **Staging Deployment**: Test with real traffic patterns +6. **Gradual Rollout**: Use feature flags for production +7. **Monitor & Iterate**: Track metrics and adjust as needed + +## Expected Outcomes + +After implementing all phases: +- **10x improvement** in concurrent connection handling +- **90% reduction** in event loop blocking +- **50% reduction** in memory usage under load +- **Zero memory leaks** with proper resource cleanup +- **Real-time visibility** into performance metrics +- **Graceful degradation** under extreme load + +## Version Plan + +- **v19.6.0**: Phase 1 (Critical fixes) - Backward compatible +- **v19.7.0**: Phase 2 (Resource management) - Backward compatible +- **v19.8.0**: Phase 3 (Optimizations) - Backward compatible +- **v20.0.0**: Phase 4 (Full async) - Breaking changes with migration path \ No newline at end of file diff --git a/readme.plan2.md b/readme.plan2.md deleted file mode 100644 index 0a6d2f1..0000000 --- a/readme.plan2.md +++ /dev/null @@ -1,764 +0,0 @@ -# SmartProxy Simplification Plan: Unify Action Types - -## Summary -Complete removal of 'redirect', 'block', and 'static' action types, leaving only 'forward' and 'socket-handler'. All old code will be deleted entirely - no migration paths or backwards compatibility. Socket handlers will be enhanced to receive IRouteContext as a second parameter. - -## Goal -Create a dramatically simpler SmartProxy with only two action types, where everything is either proxied (forward) or handled by custom code (socket-handler). - -## Current State -```typescript -export type TRouteActionType = 'forward' | 'redirect' | 'block' | 'static' | 'socket-handler'; -export type TSocketHandler = (socket: plugins.net.Socket) => void | Promise; -``` - -## Target State -```typescript -export type TRouteActionType = 'forward' | 'socket-handler'; -export type TSocketHandler = (socket: plugins.net.Socket, context: IRouteContext) => void | Promise; -``` - -## Benefits -1. **Simpler API** - Only two action types to understand -2. **Unified handling** - Everything is either forwarding or custom socket handling -3. **More flexible** - Socket handlers can do anything the old types did and more -4. **Less code** - Remove specialized handlers and their dependencies -5. **Context aware** - Socket handlers get access to route context (domain, port, clientIp, etc.) -6. **Clean codebase** - No legacy code or migration paths - ---- - -## Phase 1: Code to Remove - -### 1.1 Action Type Handlers -- `RouteConnectionHandler.handleRedirectAction()` -- `RouteConnectionHandler.handleBlockAction()` -- `RouteConnectionHandler.handleStaticAction()` - -### 1.2 Handler Classes -- `RedirectHandler` class (http-proxy/handlers/) -- `StaticHandler` class (http-proxy/handlers/) - -### 1.3 Type Definitions -- 'redirect', 'block', 'static' from TRouteActionType -- IRouteRedirect interface -- IRouteStatic interface -- Related properties in IRouteAction - -### 1.4 Helper Functions -- `createStaticFileRoute()` -- Any other helpers that create redirect/block/static routes - ---- - -## Phase 2: Create Predefined Socket Handlers - -### 2.1 Block Handler -```typescript -export const SocketHandlers = { - // ... existing handlers - - /** - * Block connection immediately - */ - block: (message?: string) => (socket: plugins.net.Socket, context: IRouteContext) => { - // Can use context for logging or custom messages - const finalMessage = message || `Connection blocked from ${context.clientIp}`; - if (finalMessage) { - socket.write(finalMessage); - } - socket.end(); - }, - - /** - * HTTP block response - */ - httpBlock: (statusCode: number = 403, message?: string) => (socket: plugins.net.Socket, context: IRouteContext) => { - // Can customize message based on context - const defaultMessage = `Access forbidden for ${context.domain || context.clientIp}`; - const finalMessage = message || defaultMessage; - - const response = [ - `HTTP/1.1 ${statusCode} ${finalMessage}`, - 'Content-Type: text/plain', - `Content-Length: ${finalMessage.length}`, - 'Connection: close', - '', - finalMessage - ].join('\r\n'); - - socket.write(response); - socket.end(); - } -}; -``` - -### 2.2 Redirect Handler -```typescript -export const SocketHandlers = { - // ... existing handlers - - /** - * HTTP redirect handler - */ - httpRedirect: (locationTemplate: string, statusCode: number = 301) => (socket: plugins.net.Socket, context: IRouteContext) => { - let buffer = ''; - - socket.once('data', (data) => { - buffer += data.toString(); - - // Parse HTTP request - const lines = buffer.split('\r\n'); - const requestLine = lines[0]; - const [method, path] = requestLine.split(' '); - - // Use domain from context (more reliable than Host header) - const domain = context.domain || 'localhost'; - const port = context.port; - - // Replace placeholders in location using context - let finalLocation = locationTemplate - .replace('{domain}', domain) - .replace('{port}', String(port)) - .replace('{path}', path) - .replace('{clientIp}', context.clientIp); - - const message = `Redirecting to ${finalLocation}`; - const response = [ - `HTTP/1.1 ${statusCode} ${statusCode === 301 ? 'Moved Permanently' : 'Found'}`, - `Location: ${finalLocation}`, - 'Content-Type: text/plain', - `Content-Length: ${message.length}`, - 'Connection: close', - '', - message - ].join('\r\n'); - - socket.write(response); - socket.end(); - }); - } -}; -``` - -### 2.3 Benefits of Context in Socket Handlers -With routeContext as a second parameter, socket handlers can: -- Access client IP for logging or rate limiting -- Use domain information for multi-tenant handling -- Check if connection is TLS and what version -- Access route name/ID for metrics -- Build more intelligent responses based on context - -Example advanced handler: -```typescript -const rateLimitHandler = (maxRequests: number) => { - const ipCounts = new Map(); - - return (socket: net.Socket, context: IRouteContext) => { - const count = (ipCounts.get(context.clientIp) || 0) + 1; - ipCounts.set(context.clientIp, count); - - if (count > maxRequests) { - socket.write(`Rate limit exceeded for ${context.clientIp}\n`); - socket.end(); - return; - } - - // Process request... - }; -}; -``` - ---- - -## Phase 3: Update Helper Functions - -### 3.1 Update createHttpToHttpsRedirect -```typescript -export function createHttpToHttpsRedirect( - domains: string | string[], - httpsPort: number = 443, - options: Partial = {} -): IRouteConfig { - return { - name: options.name || `HTTP to HTTPS Redirect for ${Array.isArray(domains) ? domains.join(', ') : domains}`, - match: { - ports: options.match?.ports || 80, - domains - }, - action: { - type: 'socket-handler', - socketHandler: SocketHandlers.httpRedirect(`https://{domain}:${httpsPort}{path}`, 301) - }, - ...options - }; -} -``` - -### 3.2 Update createSocketHandlerRoute -```typescript -export function createSocketHandlerRoute( - domains: string | string[], - ports: TPortRange, - handler: TSocketHandler, - options: { name?: string; priority?: number; path?: string } = {} -): IRouteConfig { - return { - name: options.name || 'socket-handler-route', - priority: options.priority !== undefined ? options.priority : 50, - match: { - domains, - ports, - ...(options.path && { path: options.path }) - }, - action: { - type: 'socket-handler', - socketHandler: handler - } - }; -} - -``` - ---- - -## Phase 4: Core Implementation Changes - -### 4.1 Update Route Connection Handler -```typescript -// Remove these methods: -// - handleRedirectAction() -// - handleBlockAction() -// - handleStaticAction() - -// Update switch statement to only have: -switch (route.action.type) { - case 'forward': - return this.handleForwardAction(socket, record, route, initialChunk); - - case 'socket-handler': - this.handleSocketHandlerAction(socket, record, route, initialChunk); - return; - - default: - logger.log('error', `Unknown action type '${(route.action as any).type}'`); - socket.end(); - this.connectionManager.cleanupConnection(record, 'unknown_action'); -} -``` - -### 4.2 Update Socket Handler to Pass Context -```typescript -private async handleSocketHandlerAction( - socket: plugins.net.Socket, - record: IConnectionRecord, - route: IRouteConfig, - initialChunk?: Buffer -): Promise { - const connectionId = record.id; - - // Create route context for the handler - const routeContext = this.createRouteContext({ - connectionId: record.id, - port: record.localPort, - domain: record.lockedDomain, - clientIp: record.remoteIP, - serverIp: socket.localAddress || '', - isTls: record.isTLS || false, - tlsVersion: record.tlsVersion, - routeName: route.name, - routeId: route.id, - }); - - try { - // Call the handler with socket AND context - const result = route.action.socketHandler(socket, routeContext); - - // Rest of implementation stays the same... - } catch (error) { - // Error handling... - } -} -``` - -### 4.3 Clean Up Imports and Exports -- Remove imports of deleted handler classes -- Update index.ts files to remove exports -- Clean up any unused imports - ---- - -## Phase 5: Test Updates - -### 5.1 Remove Old Tests -- Delete tests for redirect action type -- Delete tests for block action type -- Delete tests for static action type - -### 5.2 Add New Socket Handler Tests -- Test block socket handler with context -- Test HTTP redirect socket handler with context -- Test that context is properly passed to all handlers - ---- - -## Phase 6: Documentation Updates - -### 6.1 Update README.md -- Remove documentation for redirect, block, static action types -- Document the two remaining action types: forward and socket-handler -- Add examples using socket handlers with context - -### 6.2 Update Type Documentation -```typescript -/** - * Route action types - * - 'forward': Proxy the connection to a target host:port - * - 'socket-handler': Pass the socket to a custom handler function - */ -export type TRouteActionType = 'forward' | 'socket-handler'; - -/** - * Socket handler function - * @param socket - The incoming socket connection - * @param context - Route context with connection information - */ -export type TSocketHandler = (socket: net.Socket, context: IRouteContext) => void | Promise; -``` - -### 6.3 Example Documentation -```typescript -// Example: Block connections from specific IPs -const ipBlocker = (socket: net.Socket, context: IRouteContext) => { - if (context.clientIp.startsWith('192.168.')) { - socket.write('Internal IPs not allowed\n'); - socket.end(); - return; - } - // Forward to backend... -}; - -// Example: Domain-based routing -const domainRouter = (socket: net.Socket, context: IRouteContext) => { - const backend = context.domain === 'api.example.com' ? 'api-server' : 'web-server'; - // Forward to appropriate backend... -}; -``` - ---- - -## Implementation Steps - -1. **Update TSocketHandler type** (15 minutes) - - Add IRouteContext as second parameter - - Update type definition in route-types.ts - -2. **Update socket handler implementation** (30 minutes) - - Create routeContext in handleSocketHandlerAction - - Pass context to socket handler function - - Update all existing socket handlers in route-helpers.ts - -3. **Remove old action types** (30 minutes) - - Remove 'redirect', 'block', 'static' from TRouteActionType - - Remove IRouteRedirect, IRouteStatic interfaces - - Clean up IRouteAction interface - -4. **Delete old handlers** (45 minutes) - - Delete handleRedirectAction, handleBlockAction, handleStaticAction methods - - Delete RedirectHandler and StaticHandler classes - - Remove imports and exports - -5. **Update route connection handler** (30 minutes) - - Simplify switch statement to only handle 'forward' and 'socket-handler' - - Remove all references to deleted action types - -6. **Create new socket handlers** (30 minutes) - - Implement SocketHandlers.block() with context - - Implement SocketHandlers.httpBlock() with context - - Implement SocketHandlers.httpRedirect() with context - -7. **Update helper functions** (30 minutes) - - Update createHttpToHttpsRedirect to use socket handler - - Delete createStaticFileRoute entirely - - Update any other affected helpers - -8. **Clean up tests** (1.5 hours) - - Delete all tests for removed action types - - Update socket handler tests to verify context parameter - - Add new tests for block/redirect socket handlers - -9. **Update documentation** (30 minutes) - - Update README.md - - Update type documentation - - Add examples of context usage - -**Total estimated time: ~5 hours** - ---- - -## Considerations - -### Benefits -- **Dramatically simpler API** - Only 2 action types instead of 5 -- **Consistent handling model** - Everything is either forwarding or custom handling -- **More powerful** - Socket handlers with context can do much more than old static types -- **Less code to maintain** - Removing hundreds of lines of specialized handler code -- **Better extensibility** - Easy to add new socket handlers for any use case -- **Context awareness** - All handlers get full connection context - -### Trade-offs -- Static file serving removed (users should use nginx/apache behind proxy) -- HTTP-specific logic (redirects) now in socket handlers (but more flexible) -- Slightly more verbose configuration for simple blocks/redirects - -### Why This Approach -1. **Simplicity wins** - Two concepts are easier to understand than five -2. **Power through context** - Socket handlers with context are more capable -3. **Clean break** - No migration paths means cleaner code -4. **Future proof** - Easy to add new handlers without changing core - ---- - -## Code Examples: Before and After - -### Block Action -```typescript -// BEFORE -{ - action: { type: 'block' } -} - -// AFTER -{ - action: { - type: 'socket-handler', - socketHandler: SocketHandlers.block() - } -} -``` - -### HTTP Redirect -```typescript -// BEFORE -{ - action: { - type: 'redirect', - redirect: { - to: 'https://{domain}:443{path}', - status: 301 - } - } -} - -// AFTER -{ - action: { - type: 'socket-handler', - socketHandler: SocketHandlers.httpRedirect('https://{domain}:443{path}', 301) - } -} -``` - -### Custom Handler with Context -```typescript -// NEW CAPABILITY - Access to full context -{ - action: { - type: 'socket-handler', - socketHandler: (socket, context) => { - console.log(`Connection from ${context.clientIp} to ${context.domain}:${context.port}`); - // Custom handling based on context... - } - } -} -``` - ---- - -## Detailed Implementation Tasks - -### Step 1: Update TSocketHandler Type (15 minutes) -- [x] Open `ts/proxies/smart-proxy/models/route-types.ts` -- [x] Find line 14: `export type TSocketHandler = (socket: plugins.net.Socket) => void | Promise;` -- [x] Import IRouteContext at top of file: `import type { IRouteContext } from '../../../core/models/route-context.js';` -- [x] Update TSocketHandler to: `export type TSocketHandler = (socket: plugins.net.Socket, context: IRouteContext) => void | Promise;` -- [x] Save file - -### Step 2: Update Socket Handler Implementation (30 minutes) -- [x] Open `ts/proxies/smart-proxy/route-connection-handler.ts` -- [x] Find `handleSocketHandlerAction` method (around line 790) -- [x] Add route context creation after line 809: - ```typescript - // Create route context for the handler - const routeContext = this.createRouteContext({ - connectionId: record.id, - port: record.localPort, - domain: record.lockedDomain, - clientIp: record.remoteIP, - serverIp: socket.localAddress || '', - isTls: record.isTLS || false, - tlsVersion: record.tlsVersion, - routeName: route.name, - routeId: route.id, - }); - ``` -- [x] Update line 812 from `const result = route.action.socketHandler(socket);` -- [x] To: `const result = route.action.socketHandler(socket, routeContext);` -- [x] Save file - -### Step 3: Update Existing Socket Handlers in route-helpers.ts (20 minutes) -- [x] Open `ts/proxies/smart-proxy/utils/route-helpers.ts` -- [x] Update `echo` handler (line 856): - - From: `echo: (socket: plugins.net.Socket) => {` - - To: `echo: (socket: plugins.net.Socket, context: IRouteContext) => {` -- [x] Update `proxy` handler (line 864): - - From: `proxy: (targetHost: string, targetPort: number) => (socket: plugins.net.Socket) => {` - - To: `proxy: (targetHost: string, targetPort: number) => (socket: plugins.net.Socket, context: IRouteContext) => {` -- [x] Update `lineProtocol` handler (line 879): - - From: `lineProtocol: (handler: (line: string, socket: plugins.net.Socket) => void) => (socket: plugins.net.Socket) => {` - - To: `lineProtocol: (handler: (line: string, socket: plugins.net.Socket) => void) => (socket: plugins.net.Socket, context: IRouteContext) => {` -- [ ] Update `httpResponse` handler (line 896): - - From: `httpResponse: (statusCode: number, body: string) => (socket: plugins.net.Socket) => {` - - To: `httpResponse: (statusCode: number, body: string) => (socket: plugins.net.Socket, context: IRouteContext) => {` -- [ ] Save file - -### Step 4: Remove Old Action Types from Type Definitions (15 minutes) -- [ ] Open `ts/proxies/smart-proxy/models/route-types.ts` -- [ ] Find line with TRouteActionType (around line 10) -- [ ] Change from: `export type TRouteActionType = 'forward' | 'redirect' | 'block' | 'static' | 'socket-handler';` -- [ ] To: `export type TRouteActionType = 'forward' | 'socket-handler';` -- [ ] Find and delete IRouteRedirect interface (around line 123-126) -- [ ] Find and delete IRouteStatic interface (if exists) -- [ ] Find IRouteAction interface -- [ ] Remove these properties: - - `redirect?: IRouteRedirect;` - - `static?: IRouteStatic;` -- [ ] Save file - -### Step 5: Delete Handler Classes (15 minutes) -- [ ] Delete file: `ts/proxies/http-proxy/handlers/redirect-handler.ts` -- [ ] Delete file: `ts/proxies/http-proxy/handlers/static-handler.ts` -- [ ] Open `ts/proxies/http-proxy/handlers/index.ts` -- [ ] Delete all content (the file only exports RedirectHandler and StaticHandler) -- [ ] Save empty file or delete it - -### Step 6: Remove Handler Methods from RouteConnectionHandler (30 minutes) -- [ ] Open `ts/proxies/smart-proxy/route-connection-handler.ts` -- [ ] Find and delete entire `handleRedirectAction` method (around line 723) -- [ ] Find and delete entire `handleBlockAction` method (around line 750) -- [ ] Find and delete entire `handleStaticAction` method (around line 773) -- [ ] Remove imports at top: - - `import { RedirectHandler, StaticHandler } from '../http-proxy/handlers/index.js';` -- [ ] Save file - -### Step 7: Update Switch Statement (15 minutes) -- [ ] Still in `route-connection-handler.ts` -- [ ] Find switch statement (around line 388) -- [ ] Remove these cases: - - `case 'redirect': return this.handleRedirectAction(...)` - - `case 'block': return this.handleBlockAction(...)` - - `case 'static': this.handleStaticAction(...); return;` -- [ ] Verify only 'forward' and 'socket-handler' cases remain -- [ ] Save file - -### Step 8: Add New Socket Handlers to route-helpers.ts (30 minutes) -- [ ] Open `ts/proxies/smart-proxy/utils/route-helpers.ts` -- [ ] Add import at top: `import type { IRouteContext } from '../../../core/models/route-context.js';` -- [ ] Add to SocketHandlers object: - ```typescript - /** - * Block connection immediately - */ - block: (message?: string) => (socket: plugins.net.Socket, context: IRouteContext) => { - const finalMessage = message || `Connection blocked from ${context.clientIp}`; - if (finalMessage) { - socket.write(finalMessage); - } - socket.end(); - }, - - /** - * HTTP block response - */ - httpBlock: (statusCode: number = 403, message?: string) => (socket: plugins.net.Socket, context: IRouteContext) => { - const defaultMessage = `Access forbidden for ${context.domain || context.clientIp}`; - const finalMessage = message || defaultMessage; - - const response = [ - `HTTP/1.1 ${statusCode} ${finalMessage}`, - 'Content-Type: text/plain', - `Content-Length: ${finalMessage.length}`, - 'Connection: close', - '', - finalMessage - ].join('\r\n'); - - socket.write(response); - socket.end(); - }, - - /** - * HTTP redirect handler - */ - httpRedirect: (locationTemplate: string, statusCode: number = 301) => (socket: plugins.net.Socket, context: IRouteContext) => { - let buffer = ''; - - socket.once('data', (data) => { - buffer += data.toString(); - - const lines = buffer.split('\r\n'); - const requestLine = lines[0]; - const [method, path] = requestLine.split(' '); - - const domain = context.domain || 'localhost'; - const port = context.port; - - let finalLocation = locationTemplate - .replace('{domain}', domain) - .replace('{port}', String(port)) - .replace('{path}', path) - .replace('{clientIp}', context.clientIp); - - const message = `Redirecting to ${finalLocation}`; - const response = [ - `HTTP/1.1 ${statusCode} ${statusCode === 301 ? 'Moved Permanently' : 'Found'}`, - `Location: ${finalLocation}`, - 'Content-Type: text/plain', - `Content-Length: ${message.length}`, - 'Connection: close', - '', - message - ].join('\r\n'); - - socket.write(response); - socket.end(); - }); - } - ``` -- [x] Save file - -### Step 9: Update Helper Functions (20 minutes) -- [x] Still in `route-helpers.ts` -- [x] Update `createHttpToHttpsRedirect` function (around line 109): - - Change the action to use socket handler: - ```typescript - action: { - type: 'socket-handler', - socketHandler: SocketHandlers.httpRedirect(`https://{domain}:${httpsPort}{path}`, 301) - } - ``` -- [x] Delete entire `createStaticFileRoute` function (lines 277-322) -- [x] Save file - -### Step 10: Update Test Files (1.5 hours) -#### 10.1 Update Socket Handler Tests -- [x] Open `test/test.socket-handler.ts` -- [x] Update all handler functions to accept context parameter -- [x] Open `test/test.socket-handler.simple.ts` -- [x] Update handler to accept context parameter -- [x] Open `test/test.socket-handler-race.ts` -- [x] Update handler to accept context parameter - -#### 10.2 Find and Update/Delete Redirect Tests -- [x] Search for files containing `type: 'redirect'` in test directory -- [x] For each file: - - [x] If it's a redirect-specific test, delete the file - - [x] If it's a mixed test, update redirect actions to use socket handlers -- [x] Files to check: - - [x] `test/test.route-redirects.ts` - deleted entire file - - [x] `test/test.forwarding.ts` - update any redirect tests - - [x] `test/test.forwarding.examples.ts` - update any redirect tests - - [x] `test/test.route-config.ts` - update any redirect tests - -#### 10.3 Find and Update/Delete Block Tests -- [x] Search for files containing `type: 'block'` in test directory -- [x] Update or delete as appropriate - -#### 10.4 Find and Delete Static Tests -- [x] Search for files containing `type: 'static'` in test directory -- [x] Delete static-specific test files -- [x] Remove static tests from mixed test files - -### Step 11: Clean Up Imports and Exports (20 minutes) -- [x] Open `ts/proxies/smart-proxy/utils/index.ts` -- [x] Ensure route-helpers.ts is exported -- [x] Remove any exports of deleted functions -- [x] Open `ts/index.ts` -- [x] Remove any exports of deleted types/interfaces -- [x] Search for any remaining imports of RedirectHandler or StaticHandler -- [x] Remove any found imports - -### Step 12: Documentation Updates (30 minutes) -- [x] Update README.md: - - [x] Remove any mention of redirect, block, static action types - - [x] Add examples of socket handlers with context - - [x] Document the two action types: forward and socket-handler -- [x] Update any JSDoc comments in modified files -- [x] Add examples showing context usage - -### Step 13: Final Verification (15 minutes) -- [x] Run build: `pnpm build` -- [x] Fix any compilation errors -- [x] Run tests: `pnpm test` -- [x] Fix any failing tests -- [x] Search codebase for any remaining references to: - - [x] 'redirect' action type - - [x] 'block' action type - - [x] 'static' action type - - [x] RedirectHandler - - [x] StaticHandler - - [x] IRouteRedirect - - [x] IRouteStatic - -### Step 14: Test New Functionality (30 minutes) -- [x] Create test for block socket handler with context -- [x] Create test for httpBlock socket handler with context -- [x] Create test for httpRedirect socket handler with context -- [x] Verify context is properly passed in all scenarios - ---- - -## Files to be Modified/Deleted - -### Files to Modify: -1. `ts/proxies/smart-proxy/models/route-types.ts` - Update types -2. `ts/proxies/smart-proxy/route-connection-handler.ts` - Remove handlers, update switch -3. `ts/proxies/smart-proxy/utils/route-helpers.ts` - Update handlers, add new ones -4. `ts/proxies/http-proxy/handlers/index.ts` - Remove exports -5. Various test files - Update to use socket handlers - -### Files to Delete: -1. `ts/proxies/http-proxy/handlers/redirect-handler.ts` -2. `ts/proxies/http-proxy/handlers/static-handler.ts` -3. `test/test.route-redirects.ts` (likely) -4. Any static-specific test files - -### Test Files Requiring Updates (15 files found): -- test/test.acme-http01-challenge.ts -- test/test.logger-error-handling.ts -- test/test.port80-management.node.ts -- test/test.route-update-callback.node.ts -- test/test.acme-state-manager.node.ts -- test/test.acme-route-creation.ts -- test/test.forwarding.ts -- test/test.route-redirects.ts -- test/test.forwarding.examples.ts -- test/test.acme-simple.ts -- test/test.acme-http-challenge.ts -- test/test.certificate-provisioning.ts -- test/test.route-config.ts -- test/test.route-utils.ts -- test/test.certificate-simple.ts - ---- - -## Success Criteria -- ✅ Only 'forward' and 'socket-handler' action types remain -- ✅ Socket handlers receive IRouteContext as second parameter -- ✅ All old handler code completely removed -- ✅ Redirect functionality works via context-aware socket handlers -- ✅ Block functionality works via context-aware socket handlers -- ✅ All tests updated and passing -- ✅ Documentation updated with new examples -- ✅ No performance regression -- ✅ Cleaner, simpler codebase \ No newline at end of file diff --git a/readme.problems.md b/readme.problems.md index 2ebe16b..15d95e0 100644 --- a/readme.problems.md +++ b/readme.problems.md @@ -1,86 +1,170 @@ -# SmartProxy Module Problems +# SmartProxy Performance Issues Report -Based on test analysis, the following potential issues have been identified in the SmartProxy module: +## Executive Summary +This report identifies performance issues and blocking operations in the SmartProxy codebase that could impact scalability and responsiveness under high load. -## 1. HttpProxy Route Configuration Issue -**Location**: `ts/proxies/http-proxy/http-proxy.ts:380` -**Problem**: The HttpProxy is trying to read the 'type' property of an undefined object when updating route configurations. -**Evidence**: `test.http-forwarding-fix.ts` fails with: -``` -TypeError: Cannot read properties of undefined (reading 'type') - at HttpProxy.updateRouteConfigs (/mnt/data/lossless/push.rocks/smartproxy/ts/proxies/http-proxy/http-proxy.ts:380:24) -``` -**Impact**: Routes with `useHttpProxy` configuration may not work properly. +## Critical Issues -## 2. Connection Forwarding Issues -**Problem**: Basic TCP forwarding appears to not be working correctly after the simplification to just 'forward' and 'socket-handler' action types. -**Evidence**: Multiple forwarding tests timeout waiting for data to be forwarded: -- `test.forwarding-fix-verification.ts` - times out waiting for forwarded data -- `test.connection-forwarding.ts` - times out on SNI-based forwarding -**Impact**: The 'forward' action type may not be properly forwarding connections to target servers. +### 1. **Synchronous Filesystem Operations** +These operations block the event loop and should be replaced with async alternatives: -## 3. Missing Certificate Manager Methods -**Problem**: Tests expect `provisionAllCertificates` method on certificate manager but it may not exist or may not be properly initialized. -**Evidence**: Multiple tests fail with "this.certManager.provisionAllCertificates is not a function" -**Impact**: Certificate provisioning may not work as expected. +#### Certificate Management +- `ts/proxies/http-proxy/certificate-manager.ts:29`: `fs.existsSync()` +- `ts/proxies/http-proxy/certificate-manager.ts:30`: `fs.mkdirSync()` +- `ts/proxies/http-proxy/certificate-manager.ts:49-50`: `fs.readFileSync()` for loading certificates -## 4. Route Update Mechanism -**Problem**: The route update mechanism may have issues preserving certificate manager callbacks and other state. -**Evidence**: Tests specifically designed to verify callback preservation after route updates. -**Impact**: Dynamic route updates might break certificate management functionality. +#### NFTables Proxy +- `ts/proxies/nftables-proxy/nftables-proxy.ts`: Multiple uses of `execSync()` for system commands +- `ts/proxies/nftables-proxy/nftables-proxy.ts`: Multiple `fs.writeFileSync()` and `fs.unlinkSync()` operations -## 5. Route-Specific Security Not Fully Implemented -**Problem**: While the route definitions support security configurations (ipAllowList, ipBlockList, authentication), these are not being enforced at the route level. -**Evidence**: -- SecurityManager has methods like `isIPAuthorized` for route-specific security -- Route connection handler only checks global IP validation, not route-specific security rules -- No evidence of route.action.security being checked when handling connections -**Impact**: Route-specific security rules defined in configuration are not enforced, potentially allowing unauthorized access. -**Status**: ✅ FIXED - Route-specific IP allow/block lists are now enforced when a route is matched. Authentication is logged as not enforceable for non-terminated connections. -**Additional Fix**: Removed security checks from route matching logic - security is now properly enforced AFTER a route is matched, not during matching. +#### Certificate Store +- `ts/proxies/smart-proxy/cert-store.ts:8`: `ensureDirSync()` +- `ts/proxies/smart-proxy/cert-store.ts:15,31,76`: `fileExistsSync()` +- `ts/proxies/smart-proxy/cert-store.ts:77`: `removeManySync()` -## 6. Security Property Location Consolidation -**Problem**: Security was defined in two places - route.security and route.action.security - causing confusion. -**Status**: ✅ FIXED - Consolidated to only route.security. Removed action.security from types and updated all references. +### 2. **Event Loop Blocking Operations** + +#### Busy Wait Loop +- `ts/proxies/nftables-proxy/nftables-proxy.ts:235-238`: + ```typescript + const waitUntil = Date.now() + retryDelayMs; + while (Date.now() < waitUntil) { + // busy wait - blocks event loop completely + } + ``` + This is extremely problematic as it blocks the entire Node.js event loop. + +### 3. **Potential Memory Leaks** + +#### Timer Management Issues +Several timers are created without proper cleanup: +- `ts/proxies/http-proxy/function-cache.ts`: `setInterval()` without storing reference for cleanup +- `ts/proxies/http-proxy/request-handler.ts`: `setInterval()` for rate limit cleanup without cleanup +- `ts/core/utils/shared-security-manager.ts`: `cleanupInterval` stored but no cleanup method + +#### Event Listener Accumulation +- Multiple instances of event listeners being added without corresponding cleanup +- Connection handlers add listeners without always removing them on connection close + +### 4. **Connection Pool Management** + +#### ConnectionPool (ts/proxies/http-proxy/connection-pool.ts) +**Good practices observed:** +- Proper connection lifecycle management +- Periodic cleanup of idle connections +- Connection limits enforcement + +**Potential issues:** +- No backpressure mechanism when pool is full +- Synchronous sorting operation in `cleanupConnectionPool()` could be slow with many connections + +### 5. **Resource Management Issues** + +#### Socket Cleanup +- Some error paths don't properly clean up sockets +- Missing `removeAllListeners()` in some error scenarios could lead to memory leaks + +#### Timeout Management +- Inconsistent timeout handling across different components +- Some sockets created without timeout settings + +### 6. **JSON Operations on Large Objects** +- `ts/proxies/smart-proxy/cert-store.ts:21`: `JSON.parse()` on certificate metadata +- `ts/proxies/smart-proxy/cert-store.ts:71`: `JSON.stringify()` with pretty printing +- `ts/proxies/http-proxy/function-cache.ts:76`: `JSON.stringify()` for cache keys (called frequently) ## Recommendations -1. **Verify Forward Action Implementation**: Check that the 'forward' action type properly establishes bidirectional data flow between client and target server. ✅ FIXED - Basic forwarding now works correctly. +### Immediate Actions (High Priority) -2. **Fix HttpProxy Route Handling**: Ensure that route objects passed to HttpProxy.updateRouteConfigs have the expected structure with all required properties. ✅ FIXED - Routes now preserve their structure. +1. **Replace Synchronous Operations** + ```typescript + // Instead of: + if (fs.existsSync(path)) { ... } + + // Use: + try { + await fs.promises.access(path); + // file exists + } catch { + // file doesn't exist + } + ``` -3. **Review Certificate Manager API**: Ensure all expected methods exist and are properly documented. +2. **Fix Busy Wait Loop** + ```typescript + // Instead of: + while (Date.now() < waitUntil) { } + + // Use: + await new Promise(resolve => setTimeout(resolve, retryDelayMs)); + ``` -4. **Add Integration Tests**: Many unit tests are testing internal implementation details. Consider adding more integration tests that test the public API. +3. **Add Timer Cleanup** + ```typescript + class Component { + private cleanupTimer?: NodeJS.Timeout; + + start() { + this.cleanupTimer = setInterval(() => { ... }, 60000); + } + + stop() { + if (this.cleanupTimer) { + clearInterval(this.cleanupTimer); + this.cleanupTimer = undefined; + } + } + } + ``` -5. **Implement Route-Specific Security**: Add security checks when a route is matched to enforce route-specific IP allow/block lists and authentication rules. ✅ FIXED - IP allow/block lists are now enforced at the route level. +### Medium Priority -6. **Fix TLS Detection Logic**: The connection handler was treating all connections as TLS. This has been partially fixed but needs proper testing for all TLS modes. +1. **Optimize JSON Operations** + - Cache JSON.stringify results for frequently used objects + - Consider using faster hashing for cache keys (e.g., crypto.createHash) + - Use streaming JSON parsers for large objects -## 7. HTTP Domain Matching Issue -**Problem**: Routes with domain restrictions fail to match HTTP connections because domain information is only available after HTTP headers are received, but route matching happens immediately upon connection. -**Evidence**: -- `test.http-port8080-forwarding.ts` - "No route found for connection on port 8080" despite having a matching route -- HTTP connections provide domain info via the Host header, which arrives after the initial TCP connection -- Route matching in `handleInitialData` happens before HTTP headers are parsed -**Impact**: HTTP routes with domain restrictions cannot be matched, forcing users to remove domain restrictions for HTTP routes. -**Root Cause**: For non-TLS connections, SmartProxy attempts to match routes immediately, but the domain information needed for matching is only available after parsing HTTP headers. -**Status**: ✅ FIXED - Added skipDomainCheck parameter to route matching for HTTP proxy ports. When a port is configured with useHttpProxy and the connection is not TLS, domain validation is skipped at the initial route matching stage, allowing the HttpProxy to handle domain-based routing after headers are received. +2. **Improve Connection Pool** + - Implement backpressure/queueing when pool is full + - Use a heap or priority queue for connection management instead of sorting -## 8. HttpProxy Plain HTTP Forwarding Issue -**Problem**: HttpProxy is an HTTPS server but SmartProxy forwards plain HTTP connections to it via `useHttpProxy` configuration. -**Evidence**: -- `test.http-port8080-forwarding.ts` - Connection immediately closed after forwarding to HttpProxy -- HttpProxy is created with `http2.createSecureServer` expecting TLS connections -- SmartProxy forwards raw HTTP data to HttpProxy's HTTPS port -**Impact**: Plain HTTP connections cannot be handled by HttpProxy, despite `useHttpProxy` configuration suggesting this should work. -**Root Cause**: Design mismatch - HttpProxy is designed for HTTPS/TLS termination, not plain HTTP forwarding. -**Status**: Documented. The `useHttpProxy` configuration should only be used for ports that receive TLS connections requiring termination. For plain HTTP forwarding, use direct forwarding without HttpProxy. +3. **Standardize Resource Cleanup** + - Create a base class for components with lifecycle management + - Ensure all event listeners are removed on cleanup + - Add abort controllers for better cancellation support -## 9. Route Security Configuration Location Issue -**Problem**: Tests were placing security configuration in `route.action.security` instead of `route.security`. -**Evidence**: -- `test.route-security.ts` - IP block list test failing because security was in wrong location -- IRouteConfig interface defines security at route level, not inside action -**Impact**: Security rules defined in action.security were ignored, causing tests to fail. -**Status**: ✅ FIXED - Updated tests to place security configuration at the correct location (route.security). \ No newline at end of file +### Long-term Improvements + +1. **Worker Threads** + - Move CPU-intensive operations to worker threads + - Consider using worker pools for NFTables operations + +2. **Monitoring and Metrics** + - Add performance monitoring for event loop lag + - Track connection pool utilization + - Monitor memory usage patterns + +3. **Graceful Degradation** + - Implement circuit breakers for backend connections + - Add request queuing with overflow protection + - Implement adaptive timeout strategies + +## Impact Assessment + +These issues primarily affect: +- **Scalability**: Blocking operations limit concurrent connection handling +- **Responsiveness**: Event loop blocking causes latency spikes +- **Stability**: Memory leaks could cause crashes under sustained load +- **Resource Usage**: Inefficient resource management increases memory/CPU usage + +## Testing Recommendations + +1. Load test with high connection counts (10k+ concurrent) +2. Monitor event loop lag under stress +3. Test long-running scenarios to detect memory leaks +4. Benchmark with async vs sync operations to measure improvement + +## Conclusion + +While SmartProxy has good architectural design and many best practices, the identified blocking operations and resource management issues could significantly impact performance under high load. The most critical issues (busy wait loop and synchronous filesystem operations) should be addressed immediately. \ No newline at end of file