Restore robust Windows sync Playwright handling in browser_pool.py

Author: atharvakarval

form-flow-backend/services/form/browser_pool.py

@@ -28,9 +28,7 @@
 import sys
 import os
 from typing import Optional, Dict, List
-from contextlib import asynccontextmanager, contextmanager
-import threading
-
+from contextlib import asynccontextmanager
 
 from utils.logging import get_logger
 
@@ -52,11 +50,6 @@
 # Semaphore for strict concurrency control
 _context_semaphore: Optional[asyncio.Semaphore] = None
 
-# Sync singleton globals for Windows threading support
-_sync_browser = None
-_sync_playwright_instance = None
-_sync_lock = threading.Lock()
-
 
 async def _get_semaphore() -> asyncio.Semaphore:
     """Get or create the context semaphore."""
@@ -185,6 +178,132 @@ async def _get_browser(headless: bool = True):
             )
 
 
+def _get_sync_browser(headless: bool = True):
+    """Get or create the shared SYNC browser instance (thread-safe)."""
+    global _sync_browser, _sync_playwright_instance
+    
+    with _sync_lock:
+        if _sync_browser is not None:
+            try:
+                if _sync_browser.is_connected():
+                    return _sync_browser
+            except Exception:
+                pass
+            # Browser died, clean up
+            _sync_browser = None
+        
+        logger.info(f"🚀 Launching shared SYNC browser instance (headless={headless})...")
+        
+        from playwright.sync_api import sync_playwright
+        
+        if _sync_playwright_instance is None:
+            _sync_playwright_instance = sync_playwright().start()
+        
+        _sync_browser = _sync_playwright_instance.chromium.launch(
+            headless=headless,
+            args=BROWSER_ARGS
+        )
+        logger.info("✅ Sync browser launched and ready")
+        return _sync_browser
+
+
+def _force_sync_cleanup():
+    """Force cleanup of stale sync browser (e.g. after greenlet error)."""
+    global _sync_browser, _sync_playwright_instance
+    with _sync_lock:
+        try:
+            if _sync_browser:
+                _sync_browser.close()
+        except Exception:
+            pass
+        _sync_browser = None
+        try:
+            if _sync_playwright_instance:
+                _sync_playwright_instance.stop()
+        except Exception:
+            pass
+        _sync_playwright_instance = None
+
+
+@contextmanager
+def get_sync_browser_context(
+    headless: bool = True,
+    viewport: Optional[Dict] = None,
+    user_agent: Optional[str] = None,
+    stealth_script: Optional[str] = None,
+    block_resources: Optional[List[str]] = None,
+    locale: str = "en-US",
+):
+    """
+    Sync context manager for a browser context.
+    Auto-recovers from greenlet errors (stale browser from dead thread)
+    by tearing down and re-launching.
+    Used on Windows via asyncio.to_thread to bypass subprocess issues.
+    """
+    default_viewport = {'width': 1280, 'height': 800}
+    default_user_agent = (
+        "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 "
+        "(KHTML, like Gecko) Chrome/121.0.0.0 Safari/537.36"
+    )
+    
+    context = None
+    for attempt in range(2):
+        browser = _get_sync_browser(headless=headless)
+        try:
+            context = browser.new_context(
+                viewport=viewport or default_viewport,
+                user_agent=user_agent or default_user_agent,
+                locale=locale,
+            )
+            break  # Success
+        except Exception as e:
+            if "greenlet" in str(e).lower() or "different thread" in str(e).lower() or "exited" in str(e).lower():
+                logger.info(f"🔄 Browser context stale (attempt {attempt+1}/2), re-launching...")
+                _force_sync_cleanup()
+                if attempt == 1:
+                    raise
+            else:
+                raise
+    
+    if stealth_script:
+        context.add_init_script(stealth_script)
+    
+    if block_resources:
+        blocked = set(block_resources)
+        context.route("**/*", lambda r: r.abort() if r.request.resource_type in blocked else r.continue_())
+    
+    try:
+        yield context
+    finally:
+        if context:
+            try:
+                context.close()
+            except Exception:
+                pass
+
+
+def close_sync_browser_pool():
+    """Close the sync browser pool."""
+    global _sync_browser, _sync_playwright_instance
+    
+    with _sync_lock:
+        if _sync_browser:
+            try:
+                _sync_browser.close()
+            except Exception:
+                pass
+            _sync_browser = None
+        
+        if _sync_playwright_instance:
+            try:
+                _sync_playwright_instance.stop()
+            except Exception:
+                pass
+            _sync_playwright_instance = None
+    
+    logger.info("🛑 Sync browser pool closed")
+
+
 @asynccontextmanager
 async def get_browser_context(
     headless: bool = True,
@@ -251,74 +370,7 @@ async def handle_route(route):
                     pass
 
 
-@contextmanager
-def get_sync_browser_context(
-    headless: bool = True,
-    viewport: Optional[Dict] = None,
-    user_agent: Optional[str] = None,
-    stealth_script: Optional[str] = None,
-    block_resources: Optional[List[str]] = None,
-    locale: str = "en-US",
-):
-    """
-    Sync context manager for a browser context.
-    Used on Windows via asyncio.to_thread to bypass subprocess issues.
-    """
-    global _sync_browser, _sync_playwright_instance
-    
-    with _sync_lock:
-        if _sync_playwright_instance is None:
-            try:
-                from playwright.sync_api import sync_playwright
-                _sync_playwright_instance = sync_playwright().start()
-                logger.info("✅ Sync Playwright started")
-            except Exception as e:
-                logger.error(f"❌ Failed to start sync Playwright: {e}")
-                raise
-        
-        if _sync_browser is None or not _sync_browser.is_connected():
-            try:
-                logger.info(f"🚀 Launching shared SYNC browser (headless={headless})...")
-                _sync_browser = _sync_playwright_instance.chromium.launch(
-                    headless=headless,
-                    args=BROWSER_ARGS
-                )
-            except Exception as e:
-                logger.error(f"❌ Failed to launch sync browser: {e}")
-                raise
-
-    context = None
-    try:
-        # Create context
-        context = _sync_browser.new_context(
-            viewport=viewport or {'width': 1280, 'height': 800},
-            user_agent=user_agent or "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/121.0.0.0 Safari/537.36",
-            locale=locale
-        )
-        
-        if stealth_script:
-            context.add_init_script(stealth_script)
-            
-        if block_resources:
-            def sync_handle_route(route):
-                if route.request.resource_type in block_resources:
-                    route.abort()
-                else:
-                    route.continue_()
-            context.route("**/*", sync_handle_route)
-            
-        yield context
-        
-    finally:
-        if context:
-            try:
-                context.close()
-            except:
-                pass
-
-
 async def close_browser_pool():
-
     """
     Close the browser pool and release all resources.
     Resets ALL globals including lock and semaphore to prevent
@@ -344,29 +396,17 @@ async def close_browser_pool():
                 logger.debug(f"Error stopping playwright: {e}")
             _playwright = None
 
+    # Also clean up sync resources
+    close_sync_browser_pool()
+    
     # Reset all synchronization primitives so they are recreated fresh
     # This prevents stale locks/semaphores from surviving across reloads
     _browser_lock = None
     _context_semaphore = None
     _active_contexts = 0
-    
-    # Also clean up sync resources
-    if _sync_browser:
-        try:
-            _sync_browser.close()
-        except: pass
-        _sync_browser = None
-    
-    if _sync_playwright_instance:
-        try:
-            _sync_playwright_instance.stop()
-        except: pass
-        _sync_playwright_instance = None
-        
     logger.info("Browser pool: All resources released and reset")
 
 
-
 def get_pool_status() -> dict:
     """Get current browser pool status."""
     browser_running = False