shu/do not raise TargetClosedError in cleanup (#1220)
This commit is contained in:
@@ -1542,6 +1542,11 @@ class ForgeAgent:
|
||||
async def cleanup_browser_and_create_artifacts(
|
||||
self, close_browser_on_completion: bool, last_step: Step, task: Task
|
||||
) -> None:
|
||||
"""
|
||||
Developer notes: we should not expect any exception to be raised here.
|
||||
This function should handle exceptions gracefully.
|
||||
If errors are raised and not caught inside this function, please catch and handle them.
|
||||
"""
|
||||
# We need to close the browser even if there is no webhook callback url or api key
|
||||
browser_state = await app.BROWSER_MANAGER.cleanup_for_task(task.task_id, close_browser_on_completion)
|
||||
if browser_state:
|
||||
|
||||
@@ -514,20 +514,29 @@ class BrowserState:
|
||||
async with asyncio.timeout(BROWSER_CLOSE_TIMEOUT):
|
||||
if self.browser_context and close_browser_on_completion:
|
||||
LOG.info("Closing browser context and its pages")
|
||||
await self.browser_context.close()
|
||||
try:
|
||||
await self.browser_context.close()
|
||||
except Exception:
|
||||
LOG.warning("Failed to close browser context", exc_info=True)
|
||||
LOG.info("Main browser context and all its pages are closed")
|
||||
if self.browser_cleanup is not None:
|
||||
self.browser_cleanup()
|
||||
LOG.info("Main browser cleanup is excuted")
|
||||
try:
|
||||
self.browser_cleanup()
|
||||
LOG.info("Main browser cleanup is excuted")
|
||||
except Exception:
|
||||
LOG.warning("Failed to execute browser cleanup", exc_info=True)
|
||||
except asyncio.TimeoutError:
|
||||
LOG.error("Timeout to close browser context, going to stop playwright directly")
|
||||
|
||||
try:
|
||||
async with asyncio.timeout(BROWSER_CLOSE_TIMEOUT):
|
||||
if self.pw and close_browser_on_completion:
|
||||
LOG.info("Stopping playwright")
|
||||
await self.pw.stop()
|
||||
LOG.info("Playwright is stopped")
|
||||
try:
|
||||
LOG.info("Stopping playwright")
|
||||
await self.pw.stop()
|
||||
LOG.info("Playwright is stopped")
|
||||
except Exception:
|
||||
LOG.warning("Failed to stop playwright", exc_info=True)
|
||||
except asyncio.TimeoutError:
|
||||
LOG.error("Timeout to close playwright, might leave the broswer opening forever")
|
||||
|
||||
|
||||
@@ -202,6 +202,10 @@ class BrowserManager:
|
||||
LOG.info("BrowserManger is closed")
|
||||
|
||||
async def cleanup_for_task(self, task_id: str, close_browser_on_completion: bool = True) -> BrowserState | None:
|
||||
"""
|
||||
Developer notes: handle errors here. Do not raise error from this function.
|
||||
If error occurs, log it and address the cleanup error.
|
||||
"""
|
||||
LOG.info("Cleaning up for task")
|
||||
browser_state_to_close = self.pages.pop(task_id, None)
|
||||
if browser_state_to_close:
|
||||
|
||||
Reference in New Issue
Block a user