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

fix(backend): Fix error pin output not being propagated into the next nodes #8408

Merged
merged 5 commits into from
Oct 24, 2024

Conversation

majdyz
Copy link
Contributor

@majdyz majdyz commented Oct 23, 2024

Background

Some blocks have an error output pin, this pin should be auto-populated upon any exception in the block code.
But currently, this output is not propagated into the next node, because the execution is halted on error.

Changes 🏗️

Add propagation logic to the block exception processing step, to allow connected error pin output to be propagated into the next node.
image

Testing 🔍

Note

Only for the new autogpt platform, currently in autogpt_platform/

  • Create from scratch and execute an agent with at least 3 blocks
  • Import an agent from file upload, and confirm it executes correctly
  • Upload agent to marketplace
  • Import an agent from marketplace and confirm it executes correctly
  • Edit an agent from monitor, and confirm it executes correctly

@majdyz majdyz requested a review from a team as a code owner October 23, 2024 12:19
@github-actions github-actions bot added the platform/backend AutoGPT Platform - Back end label Oct 23, 2024
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Error Handling
The new error handling logic propagates errors to the next nodes. Verify if this is the intended behavior and if it could lead to unexpected cascading errors in the system.

Resource Management
The credentials lock is now released in the finally block. Ensure this doesn't introduce any potential resource leaks or race conditions.

Credit Management
The credit check and spending have been moved. Verify if this new placement ensures proper credit management in all execution scenarios, including error cases.

@majdyz majdyz requested a review from ntindle October 23, 2024 13:05
@majdyz majdyz requested a review from ntindle October 24, 2024 14:04
ntindle
ntindle previously approved these changes Oct 24, 2024
@majdyz majdyz requested a review from ntindle October 24, 2024 17:10
@majdyz majdyz merged commit 1321faf into dev Oct 24, 2024
7 checks passed
@majdyz majdyz deleted the zamilmajdy/fix-error-not-propagated branch October 24, 2024 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants