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
This commit is contained in:
Aniket 2026-06-14 00:20:32 +05:30
parent f85cb27ef8
commit 7fd78bdb2d

View file

@ -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):