Handle exception if missing video

[satansdeer]

Important

Add exception handling in set_working_page() in browser_factory.py to log errors when retrieving video paths.

• Exception Handling:
  • Add generic Exception handler in set_working_page() in browser_factory.py to log errors when retrieving video paths.
  • Logs exceptions with LOG.exception() including exc_info=True for detailed traceback.

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

[ellipsis-dev]

👍 Looks good to me! Reviewed everything up to 4d85416 in 1 minute and 27 seconds

More details

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

1. skyvern/webeye/browser_factory.py:488

• Draft comment:
  Catching a generic Exception may hide unexpected errors. Consider catching a more specific exception related to a missing video.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%

1. The code is already handling the most likely error (TimeoutError) specifically. 2. The video.path() operation could fail in various unexpected ways (IO errors, permission errors, etc). 3. The error is properly logged with exc_info=True. 4. The code continues gracefully after logging. 5. Being more specific here could miss legitimate errors that should be caught and logged.
   I could be wrong about the possible error types - maybe there is a specific Playwright video-related exception that should be caught.
   Looking at the code context, this is a non-critical path for getting video artifacts. Catching all exceptions and logging them is appropriate here since we want to continue execution regardless of video path errors.
   The generic Exception catch is appropriate here as a fallback after the specific TimeoutError handling. The error is properly logged and the code continues gracefully.

2. skyvern/webeye/browser_factory.py:501

• Draft comment:
  Catching a generic Exception here might conceal issues. If possible, handle specific exceptions to differentiate between missing video and other errors.
• Reason this comment was not posted:
  Marked as duplicate.

3. skyvern/webeye/browser_factory.py:488

• Draft comment:
  Added a generic except Exception block. Consider catching a more specific exception (e.g., attribute errors for missing video) to avoid masking unexpected issues. Also, note that LOG.exception already includes exception info by default.
• Reason this comment was not posted:
  Marked as duplicate.

4. skyvern/webeye/browser_factory.py:501

• Draft comment:
  Generic exception catch here too. Consider handling specific exceptions to avoid masking bugs. The redundant use of exc_info=True may also be unnecessary with LOG.exception.
• Reason this comment was not posted:
  Marked as duplicate.

Workflow ID: wflow_BtxLIKSRXxeNroPn

---

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

[wintonzheng]

this is fine but can't this just be if page.video is not None: self.browser_artifacts.video_artifacts[index].video_path = await page.video.path()?