improve navigation error handle (#2008)

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="")
+        try:
-        for retry_time in range(retry_times):
+            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)
+                LOG.info(f"Trying to navigate to {url} and waiting for 5 seconds.", url=url, retry_time=retry_time)
-            try:
+                try:
-                start_time = time.time()
+                    start_time = time.time()
-                await page.goto(url, timeout=settings.BROWSER_LOADING_TIMEOUT_MS)
+                    await page.goto(url, timeout=settings.BROWSER_LOADING_TIMEOUT_MS)
-                end_time = time.time()
+                    end_time = time.time()
-                LOG.info(
+                    LOG.info(
-                    "Page loading time",
+                        "Page loading time",
-                    loading_time=end_time - start_time,
+                        loading_time=end_time - start_time,
-                    url=url,
+                        url=url,
-                )
+                    )
-                await asyncio.sleep(5)
+                    await asyncio.sleep(5)
-                LOG.info(f"Successfully went to {url}", url=url, retry_time=retry_time)
+                    LOG.info(f"Successfully went to {url}", url=url, retry_time=retry_time)
-                return
+                    return
 
-            except Exception as e:
+                except Exception as e:
-                navigation_error = e
+                    if retry_time >= retry_times - 1:
-                LOG.warning(
+                        raise FailedToNavigateToUrl(url=url, error_message=str(e))
-                    f"Error while navigating to url: {str(navigation_error)}",
+
-                    exc_info=True,
+                    LOG.warning(
-                    url=url,
+                        f"Error while navigating to url: {str(e)}",
-                    retry_time=retry_time,
+                        exc_info=True,
-                )
+                        url=url,
-                # Wait for 5 seconds before retrying
+                        retry_time=retry_time,
-                await asyncio.sleep(5)
+                    )
-        else:
+                    # 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 e
-                raise FailedToNavigateToUrl(url=url, error_message=str(navigation_error))
-            raise navigation_error
 
     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:
+    async def close_current_open_page(self) -> bool:
-        await self._close_all_other_pages()
+        try:
-        if self.browser_context is not None:
+            async with asyncio.timeout(BROWSER_CLOSE_TIMEOUT):
-            await self.browser_context.close()
+                await self._close_all_other_pages()
-        self.browser_context = None
+                if self.browser_context is not None:
-        await self.set_working_page(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()