From 7fd78bdb2d31d7709fe50deefec7a300640d2028 Mon Sep 17 00:00:00 2001 From: Aniket <127662135+jasoncobra3@users.noreply.github.com> Date: Sun, 14 Jun 2026 00:20:32 +0530 Subject: [PATCH] fix: ensure Playwright pages and browser are closed on failure Closes #25880 SafePlaywrightURLLoader (sync and async) previously left Playwright pages open indefinitely and skipped browser.close() when an unhandled exception occurred mid-loop. - Wrapped page creation/usage in try/finally so page.close() always runs after each URL, success or failure - Wrapped browser usage in try/finally so browser.close() always runs, even on early exit via raised exception - continue_on_failure behavior preserved - No existing unit tests for this class --- backend/open_webui/retrieval/web/utils.py | 98 ++++++++++++++--------- 1 file changed, 58 insertions(+), 40 deletions(-) diff --git a/backend/open_webui/retrieval/web/utils.py b/backend/open_webui/retrieval/web/utils.py index afa73a9e0e..1beca7cbb5 100644 --- a/backend/open_webui/retrieval/web/utils.py +++ b/backend/open_webui/retrieval/web/utils.py @@ -561,60 +561,78 @@ class SafePlaywrightURLLoader(PlaywrightURLLoader, RateLimitMixin, URLProcessing from playwright.sync_api import sync_playwright with sync_playwright() as p: - # Use remote browser if ws_endpoint is provided, otherwise use local browser - if self.playwright_ws_url: - browser = p.chromium.connect(self.playwright_ws_url) - else: - browser = p.chromium.launch(headless=self.headless, proxy=self.proxy) + browser = None + try: + # Use remote browser if ws_endpoint is provided, otherwise use local browser + if self.playwright_ws_url: + browser = p.chromium.connect(self.playwright_ws_url) + else: + browser = p.chromium.launch(headless=self.headless, proxy=self.proxy) - for url in self.urls: - try: - self._safe_process_url_sync(url) - page = browser.new_page() - page.route('**/*', self._intercept_navigation_sync) - response = page.goto(url, timeout=self.playwright_timeout) - if response is None: - raise ValueError(f'page.goto() returned None for url {url}') + for url in self.urls: + page = None + try: + self._safe_process_url_sync(url) + page = browser.new_page() + page.route('**/*', self._intercept_navigation_sync) + response = page.goto(url, timeout=self.playwright_timeout) + if response is None: + raise ValueError(f'page.goto() returned None for url {url}') + + text = self.evaluator.evaluate(page, browser, response) + except Exception as e: + if self.continue_on_failure: + log.exception(f'Error loading {url}: {e}') + continue + raise e + finally: + if page: + page.close() - text = self.evaluator.evaluate(page, browser, response) metadata = {'source': url} yield Document(page_content=text, metadata=metadata) - except Exception as e: - if self.continue_on_failure: - log.exception(f'Error loading {url}: {e}') - continue - raise e - browser.close() + finally: + if browser: + browser.close() async def alazy_load(self) -> AsyncIterator[Document]: """Safely load URLs asynchronously with support for remote browser.""" from playwright.async_api import async_playwright async with async_playwright() as p: - # Use remote browser if ws_endpoint is provided, otherwise use local browser - if self.playwright_ws_url: - browser = await p.chromium.connect(self.playwright_ws_url) - else: - browser = await p.chromium.launch(headless=self.headless, proxy=self.proxy) + browser = None + try: + # Use remote browser if ws_endpoint is provided, otherwise use local browser + if self.playwright_ws_url: + browser = await p.chromium.connect(self.playwright_ws_url) + else: + browser = await p.chromium.launch(headless=self.headless, proxy=self.proxy) - for url in self.urls: - try: - await self._safe_process_url(url) - page = await browser.new_page() - await page.route('**/*', self._intercept_navigation) - response = await page.goto(url, timeout=self.playwright_timeout) - if response is None: - raise ValueError(f'page.goto() returned None for url {url}') + for url in self.urls: + page = None + try: + await self._safe_process_url(url) + page = await browser.new_page() + await page.route('**/*', self._intercept_navigation) + response = await page.goto(url, timeout=self.playwright_timeout) + if response is None: + raise ValueError(f'page.goto() returned None for url {url}') + + text = await self.evaluator.evaluate_async(page, browser, response) + except Exception as e: + if self.continue_on_failure: + log.exception(f'Error loading {url}: {e}') + continue + raise e + finally: + if page: + await page.close() - text = await self.evaluator.evaluate_async(page, browser, response) metadata = {'source': url} yield Document(page_content=text, metadata=metadata) - except Exception as e: - if self.continue_on_failure: - log.exception(f'Error loading {url}: {e}') - continue - raise e - await browser.close() + finally: + if browser: + await browser.close() class SafeWebBaseLoader(WebBaseLoader):