diff --git a/skyvern/webeye/browser_factory.py b/skyvern/webeye/browser_factory.py index cee7f6e1..4945bdbb 100644 --- a/skyvern/webeye/browser_factory.py +++ b/skyvern/webeye/browser_factory.py @@ -11,7 +11,7 @@ from typing import Any, Awaitable, Callable, Protocol import aiofiles import httpx import structlog -from playwright.async_api import BrowserContext, ConsoleMessage, Download, Error, Page, Playwright +from playwright.async_api import BrowserContext, ConsoleMessage, Download, Page, Playwright from pydantic import BaseModel, PrivateAttr from skyvern.config import settings @@ -470,40 +470,41 @@ class BrowserState: await self.navigate_to_url(page=page, url=url) async def navigate_to_url(self, page: Page, url: str, retry_times: int = NAVIGATION_MAX_RETRY_TIME) -> None: - navigation_error: Exception = FailedToNavigateToUrl(url=url, error_message="") - for retry_time in range(retry_times): - LOG.info(f"Trying to navigate to {url} and waiting for 5 seconds.", url=url, retry_time=retry_time) - try: - start_time = time.time() - await page.goto(url, timeout=settings.BROWSER_LOADING_TIMEOUT_MS) - end_time = time.time() - LOG.info( - "Page loading time", - loading_time=end_time - start_time, - url=url, - ) - await asyncio.sleep(5) - LOG.info(f"Successfully went to {url}", url=url, retry_time=retry_time) - return + try: + for retry_time in range(retry_times): + LOG.info(f"Trying to navigate to {url} and waiting for 5 seconds.", url=url, retry_time=retry_time) + try: + start_time = time.time() + await page.goto(url, timeout=settings.BROWSER_LOADING_TIMEOUT_MS) + end_time = time.time() + LOG.info( + "Page loading time", + loading_time=end_time - start_time, + url=url, + ) + await asyncio.sleep(5) + LOG.info(f"Successfully went to {url}", url=url, retry_time=retry_time) + return - except Exception as e: - navigation_error = e - LOG.warning( - f"Error while navigating to url: {str(navigation_error)}", - exc_info=True, - url=url, - retry_time=retry_time, - ) - # Wait for 5 seconds before retrying - await asyncio.sleep(5) - else: + except Exception as e: + if retry_time >= retry_times - 1: + raise FailedToNavigateToUrl(url=url, error_message=str(e)) + + LOG.warning( + f"Error while navigating to url: {str(e)}", + exc_info=True, + url=url, + retry_time=retry_time, + ) + # Wait for 5 seconds before retrying + await asyncio.sleep(5) + + except Exception as e: LOG.exception( - f"Failed to navigate to {url} after {retry_times} retries: {str(navigation_error)}", + f"Failed to navigate to {url} after {retry_times} retries: {str(e)}", url=url, ) - if isinstance(navigation_error, Error): - raise FailedToNavigateToUrl(url=url, error_message=str(navigation_error)) - raise navigation_error + raise e async def get_working_page(self) -> Page | None: # HACK: currently, assuming the last page is always the working page. @@ -576,7 +577,9 @@ class BrowserState: error_message = str(e) if "net::ERR" not in error_message: raise e - await self.close_current_open_page() + if not await self.close_current_open_page(): + LOG.warning("Failed to close the current open page") + raise e await self.check_and_fix_state( url=url, proxy_location=proxy_location, @@ -587,7 +590,9 @@ class BrowserState: page = await self.__assert_page() if not await BrowserContextFactory.validate_browser_context(await self.get_working_page()): - await self.close_current_open_page() + if not await self.close_current_open_page(): + LOG.warning("Failed to close the current open page, going to skip the browser context validation") + return page await self.check_and_fix_state( url=url, proxy_location=proxy_location, @@ -598,12 +603,18 @@ class BrowserState: page = await self.__assert_page() return page - async def close_current_open_page(self) -> None: - await self._close_all_other_pages() - if self.browser_context is not None: - await self.browser_context.close() - self.browser_context = None - await self.set_working_page(None) + async def close_current_open_page(self) -> bool: + try: + async with asyncio.timeout(BROWSER_CLOSE_TIMEOUT): + await self._close_all_other_pages() + if self.browser_context is not None: + await self.browser_context.close() + self.browser_context = None + await self.set_working_page(None) + return True + except Exception: + LOG.warning("Error while closing the current open page", exc_info=True) + return False async def stop_page_loading(self) -> None: page = await self.__assert_page()