shu/do not raise TargetClosedError in cleanup

[wintonzheng]

• do not raise TargetClosedError in task cleanup when target has been closed
• better error handling: make sure browser clean up won't raise error that breaks other cleanup progress (like artifact upload)

---

Important

Improve error handling in task cleanup by logging and handling exceptions without raising TargetClosedError in agent.py, browser_factory.py, and browser_manager.py.

• Error Handling:
  • In agent.py, cleanup_browser_and_create_artifacts() now includes a note to handle exceptions gracefully without raising them.
  • In browser_factory.py, close() method now logs and handles exceptions during browser context and Playwright closure.
  • In browser_manager.py, cleanup_for_task() and cleanup_for_workflow_run() now log errors instead of raising them during cleanup.

^{This description was created by for e44f363. It will automatically update as commits are pushed.}

[ellipsis-dev]

👍 Looks good to me! Reviewed everything up to e44f363 in 22 seconds

More details

• Looked at 73 lines of code in 3 files
• Skipped 0 files when reviewing.
• Skipped posting 3 drafted comments based on config settings.

1. skyvern/webeye/browser_factory.py:525

• Draft comment:
  Typo in log message: 'excuted' should be 'executed'.

                            LOG.info("Main browser cleanup is executed")

• Reason this comment was not posted:
  Confidence changes required: 50%
  The PR aims to handle exceptions gracefully in the cleanup process. The current implementation in browser_factory.py and browser_manager.py aligns with this intent by catching general exceptions and logging them. However, there is a minor typo in the log message that should be corrected for clarity.

2. skyvern/webeye/browser_factory.py:535

• Draft comment:
  Typo in log message: 'broswer' should be 'browser'.

            LOG.error("Timeout to close playwright, might leave the browser opening forever")

• Reason this comment was not posted:
  Confidence changes required: 50%
  The PR aims to handle exceptions gracefully in the cleanup process. The current implementation in browser_factory.py and browser_manager.py aligns with this intent by catching general exceptions and logging them. However, there is a minor typo in the log message that should be corrected for clarity.

3. skyvern/webeye/browser_manager.py:202

• Draft comment:
  Typo in log message: 'Manger' should be 'Manager'.

        LOG.info("BrowserManager is closed")

• Reason this comment was not posted:
  Confidence changes required: 50%
  The PR aims to handle exceptions gracefully in the cleanup process. The current implementation in browser_factory.py and browser_manager.py aligns with this intent by catching general exceptions and logging them. However, there is a minor typo in the log message that should be corrected for clarity.

Workflow ID: wflow_WISnm2IIFHmboyBF

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.