update

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@push.rocks/smartproxy",
-  "version": "19.3.14",
+  "version": "19.4.0",
   "private": false,
   "description": "A powerful proxy package with unified route-based configuration for high traffic management. Features include SSL/TLS support, flexible routing patterns, WebSocket handling, advanced security options, and automatic ACME certificate management.",
   "main": "dist_ts/index.js",

readme.plan.md

@@ -25,32 +25,32 @@ We need a more intelligent approach to port binding that understands when a port
 ### Implementation Plan
 
 #### Phase 1: Improve Port Manager Intelligence
-- [ ] Enhance `PortManager` to distinguish between ports that need new bindings vs ports that can reuse existing bindings
+- [x] Enhance `PortManager` to distinguish between ports that need new bindings vs ports that can reuse existing bindings
-- [ ] Add an internal tracking mechanism to detect when a requested port is already bound internally
+- [x] Add an internal tracking mechanism to detect when a requested port is already bound internally
-- [ ] Modify port addition logic to skip binding operations for ports already bound by SmartProxy
+- [x] Modify port addition logic to skip binding operations for ports already bound by SmartProxy
-- [ ] Implement reference counting for port bindings to track how many routes use each port
+- [x] Implement reference counting for port bindings to track how many routes use each port
-- [ ] Add logic to release port bindings when no routes are using them anymore
+- [x] Add logic to release port bindings when no routes are using them anymore
-- [ ] Update error handling to provide more context for port binding failures
+- [x] Update error handling to provide more context for port binding failures
 
 #### Phase 2: Refine ACME Challenge Route Integration
-- [ ] Modify `addChallengeRoute()` to check if the port is already in use by SmartProxy
+- [x] Modify `addChallengeRoute()` to check if the port is already in use by SmartProxy
-- [ ] Ensure route updates don't trigger unnecessary port binding operations
+- [x] Ensure route updates don't trigger unnecessary port binding operations
-- [ ] Implement a merging strategy for ACME routes with existing routes on the same port
+- [x] Implement a merging strategy for ACME routes with existing routes on the same port
-- [ ] Add diagnostic logging to track route and port binding relationships
+- [x] Add diagnostic logging to track route and port binding relationships
 
 #### Phase 3: Enhance Proxy Route Management
-- [ ] Restructure route update process to group routes by port
+- [x] Restructure route update process to group routes by port
-- [ ] Implement a more efficient route update mechanism that minimizes port binding operations
+- [x] Implement a more efficient route update mechanism that minimizes port binding operations
-- [ ] Develop port lifecycle management to track usage across route changes
+- [x] Develop port lifecycle management to track usage across route changes
-- [ ] Add validation to detect potential binding conflicts before attempting operations
+- [x] Add validation to detect potential binding conflicts before attempting operations
 - [ ] Create a proper route dependency graph to understand the relationships between routes
-- [ ] Implement efficient detection of "orphaned" ports that no longer have associated routes
+- [x] Implement efficient detection of "orphaned" ports that no longer have associated routes
 
 #### Phase 4: Improve Error Handling and Recovery
-- [ ] Enhance error messages to be more specific about the nature of port conflicts
+- [x] Enhance error messages to be more specific about the nature of port conflicts
-- [ ] Add recovery mechanisms for common port binding scenarios
+- [x] Add recovery mechanisms for common port binding scenarios
 - [ ] Implement a fallback port selection strategy for ACME challenges
-- [ ] Create a more robust validation system to catch issues before they cause runtime errors
+- [x] Create a more robust validation system to catch issues before they cause runtime errors
 
 ### Detailed Technical Tasks
 

ts/proxies/smart-proxy/certificate-manager.ts

@@ -401,9 +401,13 @@ export class SmartCertManager {
   
   /**
    * Add challenge route to SmartProxy
+   * 
+   * This method adds a special route for ACME HTTP-01 challenges, which typically uses port 80.
+   * Since we may already be listening on port 80 for regular routes, we need to be
+   * careful about how we add this route to avoid binding conflicts.
    */
   private async addChallengeRoute(): Promise<void> {
-    // Check with state manager first
+    // Check with state manager first - avoid duplication
     if (this.acmeStateManager && this.acmeStateManager.isChallengeRouteActive()) {
       try {
         logger.log('info', 'Challenge route already active in global state, skipping', { component: 'certificate-manager' });
@@ -437,6 +441,7 @@ export class SmartCertManager {
     const challengePort = this.globalAcmeDefaults?.port || 80;
     
     // Check if any existing routes are already using this port
+    // This helps us determine if we need to create a new binding or can reuse existing one
     const portInUseByRoutes = this.routes.some(route => {
       const routePorts = Array.isArray(route.match.ports) ? route.match.ports : [route.match.ports];
       return routePorts.some(p => {
@@ -450,36 +455,37 @@ export class SmartCertManager {
         return false;
       });
     });
-    
+
-    if (portInUseByRoutes) {
-      logger.log('info', `Port ${challengePort} is already used by another route, merging ACME challenge route`, {
-        port: challengePort,
-        component: 'certificate-manager'
-      });
-    }
-    
-    // Add the challenge route
-    const challengeRoute = this.challengeRoute;
-    
-    // If the port is already in use by other routes in this SmartProxy instance,
-    // we can safely add the ACME challenge route without trying to bind to the port again
     try {
-      // Check if we're already listening on the challenge port
+      // Log whether port is already in use by other routes
-      const isPortAlreadyBound = portInUseByRoutes;
+      if (portInUseByRoutes) {
-      
-      if (isPortAlreadyBound) {
         try {
-          logger.log('info', `Port ${challengePort} is already bound by SmartProxy, adding ACME challenge route without rebinding`, { 
+          logger.log('info', `Port ${challengePort} is already used by another route, merging ACME challenge route`, {
             port: challengePort,
-            component: 'certificate-manager' 
+            component: 'certificate-manager'
           });
         } catch (error) {
           // Silently handle logging errors
-          console.log(`[INFO] Port ${challengePort} is already bound by SmartProxy, adding ACME challenge route without rebinding`);
+          console.log(`[INFO] Port ${challengePort} is already used by another route, merging ACME challenge route`);
+        }
+      } else {
+        try {
+          logger.log('info', `Adding new ACME challenge route on port ${challengePort}`, {
+            port: challengePort,
+            component: 'certificate-manager'
+          });
+        } catch (error) {
+          // Silently handle logging errors
+          console.log(`[INFO] Adding new ACME challenge route on port ${challengePort}`);
         }
       }
       
+      // Add the challenge route to the existing routes
+      const challengeRoute = this.challengeRoute;
       const updatedRoutes = [...this.routes, challengeRoute];
+      
+      // With the re-ordering of start(), port binding should already be done
+      // This updateRoutes call should just add the route without binding again
       await this.updateRoutesCallback(updatedRoutes);
       this.challengeRouteActive = true;
       
@@ -495,28 +501,47 @@ export class SmartCertManager {
         console.log('[INFO] ACME challenge route successfully added');
       }
     } catch (error) {
-      // Handle specific EADDRINUSE errors differently based on whether it's an internal conflict
+      // Enhanced error handling based on error type
       if ((error as any).code === 'EADDRINUSE') {
         try {
-          logger.log('error', `Failed to add challenge route on port ${challengePort}: ${error.message}`, { 
+          logger.log('warn', `Challenge port ${challengePort} is unavailable - it's already in use by another process. Consider configuring a different ACME port.`, { 
-            error: (error as Error).message, 
+            port: challengePort,
+            error: (error as Error).message,
+            component: 'certificate-manager'
+          });
+        } catch (logError) {
+          // Silently handle logging errors
+          console.log(`[WARN] Challenge port ${challengePort} is unavailable - it's already in use by another process. Consider configuring a different ACME port.`);
+        }
+        
+        // Provide a more informative and actionable error message
+        throw new Error(
+          `ACME HTTP-01 challenge port ${challengePort} is already in use by another process. ` + 
+          `Please configure a different port using the acme.port setting (e.g., 8080).`
+        );
+      } else if (error.message && error.message.includes('EADDRINUSE')) {
+        // Some Node.js versions embed the error code in the message rather than the code property
+        try {
+          logger.log('warn', `Port ${challengePort} conflict detected: ${error.message}`, {
             port: challengePort,
             component: 'certificate-manager'
           });
         } catch (logError) {
           // Silently handle logging errors
-          console.log(`[ERROR] Failed to add challenge route on port ${challengePort}: ${error.message}`);
+          console.log(`[WARN] Port ${challengePort} conflict detected: ${error.message}`);
         }
         
-        // Provide a more informative error message
+        // More detailed error message with suggestions
         throw new Error(
-          `Port ${challengePort} is already in use. ` + 
+          `ACME HTTP challenge port ${challengePort} conflict detected. ` +
-          `If it's in use by an external process, configure a different port in the ACME settings. ` +
+          `To resolve this issue, try one of these approaches:\n` +
-          `If it's in use by SmartProxy, there may be a route configuration issue.`
+          `1. Configure a different port in ACME settings (acme.port)\n` +
+          `2. Add a regular route that uses port ${challengePort} before initializing the certificate manager\n` +
+          `3. Stop any other services that might be using port ${challengePort}`
         );
       }
       
-      // Log and rethrow other errors
+      // Log and rethrow other types of errors
       try {
         logger.log('error', `Failed to add challenge route: ${(error as Error).message}`, { 
           error: (error as Error).message, 

ts/proxies/smart-proxy/smart-proxy.ts

@@ -313,21 +313,6 @@ export class SmartProxy extends plugins.EventEmitter {
       return;
     }
 
-    // Initialize certificate manager before starting servers
-    await this.initializeCertificateManager();
-
-    // Initialize and start HttpProxy if needed
-    if (this.settings.useHttpProxy && this.settings.useHttpProxy.length > 0) {
-      await this.httpProxyBridge.initialize();
-      
-      // Connect HttpProxy with certificate manager
-      if (this.certManager) {
-        this.certManager.setHttpProxy(this.httpProxyBridge.getHttpProxy());
-      }
-      
-      await this.httpProxyBridge.start();
-    }
-
     // Validate the route configuration
     const configWarnings = this.routeManager.validateConfiguration();
     
@@ -362,9 +347,25 @@ export class SmartProxy extends plugins.EventEmitter {
       }
     }
 
-    // Start port listeners using the PortManager
+    // Initialize and start HttpProxy if needed - before port binding
+    if (this.settings.useHttpProxy && this.settings.useHttpProxy.length > 0) {
+      await this.httpProxyBridge.initialize();
+      await this.httpProxyBridge.start();
+    }
+
+    // Start port listeners using the PortManager BEFORE initializing certificate manager
+    // This ensures all required ports are bound and ready when adding ACME challenge routes
     await this.portManager.addPorts(listeningPorts);
     
+    // Initialize certificate manager AFTER port binding is complete
+    // This ensures the ACME challenge port is already bound and ready when needed
+    await this.initializeCertificateManager();
+    
+    // Connect certificate manager with HttpProxy if both are available
+    if (this.certManager && this.httpProxyBridge.getHttpProxy()) {
+      this.certManager.setHttpProxy(this.httpProxyBridge.getHttpProxy());
+    }
+
     // Now that ports are listening, provision any required certificates
     if (this.certManager) {
       logger.log('info', 'Starting certificate provisioning now that ports are ready', { component: 'certificate-manager' });
@@ -570,7 +571,10 @@ export class SmartProxy extends plugins.EventEmitter {
   public async updateRoutes(newRoutes: IRouteConfig[]): Promise<void> {
     return this.routeUpdateLock.runExclusive(async () => {
       try {
-        logger.log('info', `Updating routes (${newRoutes.length} routes)`, { routeCount: newRoutes.length, component: 'route-manager' });
+        logger.log('info', `Updating routes (${newRoutes.length} routes)`, { 
+          routeCount: newRoutes.length, 
+          component: 'route-manager' 
+        });
       } catch (error) {
         // Silently handle logging errors
         console.log(`[INFO] Updating routes (${newRoutes.length} routes)`);
@@ -580,25 +584,60 @@ export class SmartProxy extends plugins.EventEmitter {
       const oldPortUsage = this.updatePortUsageMap(this.settings.routes);
       const newPortUsage = this.updatePortUsageMap(newRoutes);
       
+      // Get the lists of currently listening ports and new ports needed
+      const currentPorts = new Set(this.portManager.getListeningPorts());
+      const newPortsSet = new Set(newPortUsage.keys());
+      
+      // Log the port usage for debugging
+      try {
+        logger.log('debug', `Current listening ports: ${Array.from(currentPorts).join(', ')}`, {
+          ports: Array.from(currentPorts),
+          component: 'smart-proxy'
+        });
+        
+        logger.log('debug', `Ports needed for new routes: ${Array.from(newPortsSet).join(', ')}`, {
+          ports: Array.from(newPortsSet),
+          component: 'smart-proxy'
+        });
+      } catch (error) {
+        // Silently handle logging errors
+        console.log(`[DEBUG] Current listening ports: ${Array.from(currentPorts).join(', ')}`);
+        console.log(`[DEBUG] Ports needed for new routes: ${Array.from(newPortsSet).join(', ')}`);
+      }
+      
       // Find orphaned ports - ports that no longer have any routes
       const orphanedPorts = this.findOrphanedPorts(oldPortUsage, newPortUsage);
       
-      // Find new ports that need binding
+      // Find new ports that need binding (only ports that we aren't already listening on)
-      const currentPorts = new Set(this.portManager.getListeningPorts());
-      const newPortsSet = new Set(newPortUsage.keys());
       const newBindingPorts = Array.from(newPortsSet).filter(p => !currentPorts.has(p));
+      
+      // Check for ACME challenge port to give it special handling
+      const acmePort = this.settings.acme?.port || 80;
+      const acmePortNeeded = newPortsSet.has(acmePort);
+      const acmePortListed = newBindingPorts.includes(acmePort);
+      
+      if (acmePortNeeded && acmePortListed) {
+        try {
+          logger.log('info', `Adding ACME challenge port ${acmePort} to routes`, { 
+            port: acmePort,
+            component: 'smart-proxy'
+          });
+        } catch (error) {
+          // Silently handle logging errors
+          console.log(`[INFO] Adding ACME challenge port ${acmePort} to routes`);
+        }
+      }
 
-      // Get existing routes that use NFTables
+      // Get existing routes that use NFTables and update them
       const oldNfTablesRoutes = this.settings.routes.filter(
         r => r.action.forwardingEngine === 'nftables'
       );
       
-      // Get new routes that use NFTables
       const newNfTablesRoutes = newRoutes.filter(
         r => r.action.forwardingEngine === 'nftables'
       );
       
-      // Find routes to remove, update, or add
+      // Update existing NFTables routes
       for (const oldRoute of oldNfTablesRoutes) {
         const newRoute = newNfTablesRoutes.find(r => r.name === oldRoute.name);
         
@@ -611,7 +650,7 @@ export class SmartProxy extends plugins.EventEmitter {
         }
       }
       
-      // Find new routes to add
+      // Add new NFTables routes
       for (const newRoute of newNfTablesRoutes) {
         const oldRoute = oldNfTablesRoutes.find(r => r.name === newRoute.name);
         
@@ -624,7 +663,7 @@ export class SmartProxy extends plugins.EventEmitter {
       // Update routes in RouteManager
       this.routeManager.updateRoutes(newRoutes);
 
-      // Release orphaned ports first
+      // Release orphaned ports first to free resources
       if (orphanedPorts.length > 0) {
         try {
           logger.log('info', `Releasing ${orphanedPorts.length} orphaned ports: ${orphanedPorts.join(', ')}`, { 
@@ -638,7 +677,7 @@ export class SmartProxy extends plugins.EventEmitter {
         await this.portManager.removePorts(orphanedPorts);
       }
       
-      // Add new ports
+      // Add new ports if needed
       if (newBindingPorts.length > 0) {
         try {
           logger.log('info', `Binding to ${newBindingPorts.length} new ports: ${newBindingPorts.join(', ')}`, { 
@@ -649,7 +688,38 @@ export class SmartProxy extends plugins.EventEmitter {
           // Silently handle logging errors
           console.log(`[INFO] Binding to ${newBindingPorts.length} new ports: ${newBindingPorts.join(', ')}`);
         }
-        await this.portManager.addPorts(newBindingPorts);
+        
+        // Handle port binding with improved error recovery
+        try {
+          await this.portManager.addPorts(newBindingPorts);
+        } catch (error) {
+          // Special handling for port binding errors
+          // This provides better diagnostics for ACME challenge port conflicts
+          if ((error as any).code === 'EADDRINUSE') {
+            const port = (error as any).port || newBindingPorts[0];
+            const isAcmePort = port === acmePort;
+            
+            if (isAcmePort) {
+              try {
+                logger.log('warn', `Could not bind to ACME challenge port ${port}. It may be in use by another application.`, {
+                  port,
+                  component: 'smart-proxy'
+                });
+              } catch (logError) {
+                console.log(`[WARN] Could not bind to ACME challenge port ${port}. It may be in use by another application.`);
+              }
+              
+              // Re-throw with more helpful message
+              throw new Error(
+                `ACME challenge port ${port} is already in use by another application. ` +
+                `Configure a different port in settings.acme.port (e.g., 8080) or free up port ${port}.`
+              );
+            }
+          }
+          
+          // Re-throw the original error for other cases
+          throw error;
+        }
       }
       
       // Update settings with the new routes