try to fix navigation error

[LawyZheng]

🚀	This description was created by Ellipsis for commit 3a86bc9

Summary:

Improved error handling and recovery during browser navigation in skyvern/webeye/browser_factory.py.

Key points:

• Modified skyvern/webeye/browser_factory.py to improve error handling during navigation.
• Updated check_and_fix_state to log navigation errors as warnings with exc_info=True.
• Enhanced get_or_create_page to retry navigation if specific errors occur (net::ERR).
• Added close_current_open_page method to encapsulate logic for closing the current page and browser context.
• Refactored error handling in get_or_create_page to use close_current_open_page for better code reuse and clarity.

---

Generated with ❤️ by ellipsis.dev

[ellipsis-dev]

👍 Looks good to me! Reviewed everything up to 16cb8b8 in 34.05341 seconds

More details

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

1. skyvern/webeye/browser_factory.py:239

• Draft comment:
  Ensure that the change from LOG.exception to LOG.warning with exc_info=True is intentional and correctly captures the necessary debugging information.
• Reason this comment was not posted:
  Confidence changes required: 33%
  The PR introduces a change in the logging level from exception to warning when an error occurs during navigation. This change might be intentional to reduce the severity level in logs, but it's important to ensure that the exc_info=True is correctly used to include the traceback in the logs, which is beneficial for debugging.

2. skyvern/webeye/browser_factory.py:275

• Draft comment:
  Ensure that the retry logic for errors containing 'net::ERR' is appropriate for the types of network errors expected and does not inadvertently mask other critical errors.
• Reason this comment was not posted:
  Confidence changes required: 33%
  The PR introduces a retry mechanism specifically for navigation errors containing 'net::ERR'. This is a common pattern to handle transient network errors. However, the code does not differentiate between different types of network errors, which might not all be transient. It's important to ensure that this retry logic is appropriate for the types of errors expected and does not inadvertently mask other critical errors.

3. skyvern/webeye/browser_factory.py:288

• Draft comment:
  Ensure that the new method close_current_open_page correctly handles all necessary cleanup tasks and does not leave any resources hanging, especially in error scenarios.
• Reason this comment was not posted:
  Confidence changes required: 33%
  The PR introduces a new method close_current_open_page which consolidates the logic for closing pages and the browser context. This is a good practice as it avoids code duplication and centralizes the cleanup logic. However, it's important to ensure that this method is correctly handling all necessary cleanup tasks and does not leave any resources hanging, especially in error scenarios.

Workflow ID: wflow_IZ83qbsjoH3SF3Lx

---

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

[ellipsis-dev]

❌ Changes requested. Incremental review on 3a86bc9 in 52.347891 seconds

More details

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

Workflow ID: wflow_T6qy7nQKIOisUxV4

---

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.

[ellipsis-dev]

Consider refining the exception handling in the retry mechanism to specifically catch navigation-related exceptions (e.g., playwright.async_api.Error) instead of a generic Exception. This would prevent the retry logic from being triggered by unrelated exceptions and improve the robustness of the error handling.