Skip to content

Conversation

@shaohuzhang1
Copy link
Contributor

fix: When executing the knowledge base workflow, the loop node reported an error and the workflow was not terminated

…ed an error and the workflow was not terminated
@f2c-ci-robot
Copy link

f2c-ci-robot bot commented Dec 17, 2025

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@f2c-ci-robot
Copy link

f2c-ci-robot bot commented Dec 17, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

return None
return current_result
except Exception as e:
traceback.print_exc()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The provided code seems generally correct for handling asynchronous execution. However, here are some minor improvements and points to consider:

  1. Exception Handling: It's good that you've wrapped the exception handling around the list(result) call, but it might be redundant since Python already traps exceptions within the try-except block.

  2. Status Check: The condition if current_node.status == 500: will only trigger if current_node has been assigned an attribute named status. You should ensure this assignment before using it. Additionally, returning None on error can sometimes mask underlying problems; consider logging such errors instead of immediately returning null.

  3. Return Type Compatibility: Returning None from the method could pose compatibility issues with future integrations or data processing pipelines that expect a non-null value.

  4. Code Readability: It looks clean and follows standard syntax, so readability could potentially benefit from slightly improving comments explaining each step (though they're present).

Here’s a revised version:

def hand_node_result(self, current_node, node_result_future):
    try:
        # Wait for the result asynchronously
        result = await node_result_future.result()

        if result is not None:
            # If result contains items, process them
            list(result)

        # Handle specific status codes separately
        if current_node.status in [500]:
            # Log or handle internal server errors more gracefully
            print(f"Error: Node status {current_node.status}")
            return

        # Return successful results
        return current_result
    except Exception as e:
        # Print detailed traceback for debugging purposes
        traceback.print_exc()

This approach maintains clarity while adding some additional logic related to status checks and error handling. Always review critical control flow and error handling to ensure resilience against unexpected conditions.

@shaohuzhang1 shaohuzhang1 merged commit f36d881 into v2 Dec 17, 2025
3 of 6 checks passed
@shaohuzhang1 shaohuzhang1 deleted the pr@v2@fix_workflow_knoweldge branch December 17, 2025 06:37
shaohuzhang1 added a commit that referenced this pull request Dec 17, 2025
…ed an error and the workflow was not terminated (#4532)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants