Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

handle navigation failure and fail a task in workflow run #1205

Merged

Conversation

wintonzheng
Copy link
Contributor

@wintonzheng wintonzheng commented Nov 16, 2024

Important

Enhance TaskBlock.execute() to handle navigation failures by marking tasks as failed and logging exceptions before re-raising them.

  • Behavior:
    • In TaskBlock.execute(), handle FailedToNavigateToUrl by updating task status to failed in the database and logging the failure reason before re-raising the exception.
    • Handle general exceptions similarly by updating task status to failed and logging the exception.
  • Logging:
    • Add logging for exceptions in TaskBlock.execute() to provide context on task and workflow run IDs.

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

<!-- ELLIPSIS_HIDDEN -->

> [!IMPORTANT]
> Enhance `TaskBlock.execute()` to handle navigation failures by marking tasks as failed and logging exceptions before re-raising them.
>
>   - **Behavior**:
>     - In `TaskBlock.execute()`, handle `FailedToNavigateToUrl` by updating task status to failed in the database and logging the failure reason before re-raising the exception.
>     - Handle general exceptions similarly by updating task status to failed and logging the exception.
>   - **Logging**:
>     - Add logging for exceptions in `TaskBlock.execute()` to provide context on task and workflow run IDs.
>
> <sup>This description was created by </sup>[<img alt="Ellipsis" src="https://img.shields.io/badge/Ellipsis-blue?color=175173">](https://www.ellipsis.dev?ref=Skyvern-AI%2Fskyvern-cloud&utm_source=github&utm_medium=referral)<sup> for 82e641ed46e72ac0a29098e374d0b13060855132. It will automatically update as commits are pushed.</sup>

<!-- ELLIPSIS_HIDDEN -->
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! Incremental review on bc3e2d4 in 9 seconds

More details
  • Looked at 49 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. skyvern/forge/sdk/workflow/models/block.py:318
  • Draft comment:
    Consider adding logging for FailedToNavigateToUrl exceptions to maintain consistency with general exception handling.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR introduces exception handling for FailedToNavigateToUrl and general exceptions in TaskBlock.execute(). However, the logging for FailedToNavigateToUrl is missing, which is inconsistent with the handling of general exceptions.

Workflow ID: wflow_WwWHh9ZgntyuxQKl


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

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 bc3e2d4 in 59 seconds

More details
  • Looked at 49 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. skyvern/forge/sdk/workflow/models/block.py:318
  • Draft comment:
    Consider adding logging for FailedToNavigateToUrl exceptions to maintain consistency with general exception handling.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR introduces exception handling for FailedToNavigateToUrl and general exceptions in the TaskBlock.execute() method. However, the logging for FailedToNavigateToUrl is missing, which is inconsistent with the handling of general exceptions.
2. skyvern/forge/sdk/workflow/models/block.py:310
  • Draft comment:
    This code duplicates existing exception handling logic in skyvern/forge/agent.py. Consider refactoring to use a common function for handling FailedToNavigateToUrl exceptions.

  • Exception handling for FailedToNavigateToUrl (agent.py)

  • Reason this comment was not posted:
    Based on historical feedback, this comment is too similar to comments previously marked by users as bad.

Workflow ID: wflow_y9M8mlPILSS2ccQC


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

@wintonzheng wintonzheng merged commit 19fcd40 into main Nov 16, 2024
2 checks passed
@wintonzheng wintonzheng deleted the shu/handle_navigation_failure_and_fail_task_in_workflow branch November 16, 2024 04:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant