Ensure Task Clenup - add timeout to browser context close #714
Conversation
<!-- ELLIPSIS_HIDDEN --> | 🚀 | This description was created by [Ellipsis](https://www.ellipsis.dev) for commit 938bc7e21ccda1f5ba8f10f7e290372aa17ed4e6 | |--------|--------| ### Summary: Introduced `task_cleanup` methods and browser context timeout for efficient Chrome process and browser state management, with updates across multiple files. **Key points**: - Introduced `task_cleanup` methods for Chrome process and browser state management. - Added `task_cleanup` in `cloud/agent_functions.py` to terminate Chrome processes. - Implemented `task_cleanup` in `skyvern/forge/agent_functions.py` for browser state handling. - Updated `skyvern/forge/agent.py` to call `task_cleanup` during `send_task_response`. - Enhanced `skyvern/webeye/browser_manager.py` to handle `TimeoutError` during task cleanup. - Added `BROWSER_CLOSE_TIMEOUT` constant in `skyvern/constants.py`. - Updated `BrowserManager.cleanup_for_task` to use `asyncio.timeout` for closing browser contexts. - Integrated `cleanup_browser_and_create_artifacts` into `task_cleanup`. - Moved Chrome process termination to `workers/temporal_workflows.py`. ---- Generated with ❤️ by [ellipsis.dev](https://www.ellipsis.dev) <!-- ELLIPSIS_HIDDEN -->
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to b8dc27d in 10 seconds
More details
- Looked at
58lines of code in2files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_KNZvZepQpCtAo18D
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| LOG.info("Task is cleaned up") | ||
| try: | ||
| if browser_state_to_close: | ||
| async with asyncio.timeout(BROWSER_CLOSE_TIMEOUT): |
There was a problem hiding this comment.
asyncio.timeout is incorrect; use asyncio.wait_for instead. This issue is also present in other parts of the code.
There was a problem hiding this comment.
❌ Changes requested. Incremental review on b8dc27d in 21 seconds
More details
- Looked at
57lines of code in2files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_XWlyfhPDqJEUmg5R
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| LOG.info("Task is cleaned up") | ||
| try: | ||
| if browser_state_to_close: | ||
| async with asyncio.timeout(BROWSER_CLOSE_TIMEOUT): |
There was a problem hiding this comment.
asyncio.timeout is not a valid function. Use asyncio.wait_for to apply a timeout to an async operation.
| async with asyncio.timeout(BROWSER_CLOSE_TIMEOUT): | |
| async with asyncio.wait_for(BROWSER_CLOSE_TIMEOUT): |
Summary:
Introduced
task_cleanupmethods and browser context timeout for efficient Chrome process and browser state management, with enhanced error handling and updates across multiple files.Key points:
task_cleanupmethods for Chrome process and browser state management.task_cleanupincloud/agent_functions.pyto terminate Chrome processes.task_cleanupinskyvern/forge/agent_functions.pyfor browser state handling.skyvern/forge/agent.pyto calltask_cleanupduringsend_task_response.skyvern/webeye/browser_manager.pyto handleTimeoutErrorduring task cleanup.BROWSER_CLOSE_TIMEOUTconstant inskyvern/constants.pyfor browser context close timeout.BrowserManager.cleanup_for_taskto useasyncio.timeoutfor closing browser contexts.TimeoutErrorincleanup_for_taskto log a warning if timeout occurs.cleanup_browser_and_create_artifactsintotask_cleanup.workers/temporal_workflows.py.Generated with ❤️ by ellipsis.dev