Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Check if video exists before assignment #1788

Merged
merged 1 commit into from
Feb 18, 2025

Conversation

satansdeer
Copy link
Contributor

@satansdeer satansdeer commented Feb 18, 2025

Important

Check for existence of page.video before accessing its path in set_working_page() to prevent errors.

  • Behavior:
    • In set_working_page() in browser_factory.py, check if page.video exists before accessing video.path() to prevent errors when page.video is None.
    • Applied in two locations within the function to ensure robustness against missing video objects.

This description was created by Ellipsis for 3e21f9f. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good to me! Reviewed everything up to 3e21f9f in 36 seconds

More details
  • Looked at 24 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:485
  • Draft comment:
    Good check for the existence of page.video before assigning video_path.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
2. skyvern/webeye/browser_factory.py:497
  • Draft comment:
    Replicate check for page.video to avoid calling await on a potentially non-existing object.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
3. skyvern/webeye/browser_factory.py:485
  • Draft comment:
    Good improvement: adding a check for page.video before calling its path() method. Consider logging or handling the case when page.video is not available.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
4. skyvern/webeye/browser_factory.py:497
  • Draft comment:
    Consistent check for page.video here ensures robustness. If video is missing, no assignment is attempted.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None

Workflow ID: wflow_bHfHYE8HrqkwZcm9


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

@wintonzheng wintonzheng merged commit 71cbd27 into Skyvern-AI:main Feb 18, 2025
2 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants