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

feat(backend): low balance notiifcation #9534

Open
wants to merge 27 commits into
base: dev
Choose a base branch
from

Conversation

ntindle
Copy link
Member

@ntindle ntindle commented Feb 27, 2025

For emailing, we want the user to know when an agent stopped because their balance was too low. This is the first step of that.

Changes 🏗️

  • Raise InsufficientBalanceError from credit system rather than value error when user runs out of money
  • Handle when an agent output isn't hooked up well
  • Fix the contents of the email for low balance to be a bit more aligned with the PRD
  • expose the topup intent from the db manager
  • objectify the execution stats so we can pass it around a bit more type safe
  • extract the notification stuff in manager into a function

Checklist 📋

For code changes:

  • I have clearly listed my changes in the PR description
  • I have made a test plan
  • I have tested my changes according to the test plan:
    • Set balance to $0.01
    • Run an agent that costs something more than $0.01
    • Check you get an email
    • Check your top up link works

@github-actions github-actions bot added size/l platform/backend AutoGPT Platform - Back end and removed size/l labels Feb 27, 2025
Copy link

deepsource-io bot commented Feb 27, 2025

Here's the code health analysis summary for commits 27a5635..1fe69ab. View details on DeepSource ↗.

Analysis Summary

AnalyzerStatusSummaryLink
DeepSource JavaScript LogoJavaScript✅ SuccessView Check ↗
DeepSource Python LogoPython✅ Success
❗ 65 occurences introduced
🎯 60 occurences resolved
View Check ↗

💡 If you’re a repository administrator, you can configure the quality gates from the settings.

Base automatically changed from zamilmajdy/add-graph-level-cost to dev February 27, 2025 09:58
@github-actions github-actions bot added the conflicts Automatically applied to PRs with merge conflicts label Feb 27, 2025
Copy link
Contributor

This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

Copy link

netlify bot commented Feb 27, 2025

Deploy Preview for auto-gpt-docs-dev canceled.

Name Link
🔨 Latest commit 1fe69ab
🔍 Latest deploy log https://app.netlify.com/sites/auto-gpt-docs-dev/deploys/67c68b7ea0f793000895265f

@@ -594,6 +581,14 @@ def _on_node_execution(
log_metadata.info(
f"Failed node execution {node_exec.node_exec_id}: {e}"
)
if (
Copy link
Member Author

Choose a reason for hiding this comment

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

this will trigger multiple times

Copy link

netlify bot commented Feb 27, 2025

Deploy Preview for auto-gpt-docs canceled.

Name Link
🔨 Latest commit 1fe69ab
🔍 Latest deploy log https://app.netlify.com/sites/auto-gpt-docs/deploys/67c68b7e08798f0008a75ef6

…indle/secrt-1120-low-balance-preventing-block-execution
@github-actions github-actions bot removed the conflicts Automatically applied to PRs with merge conflicts label Feb 28, 2025
Copy link
Contributor

Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly.

@github-actions github-actions bot added the conflicts Automatically applied to PRs with merge conflicts label Mar 1, 2025
Copy link
Contributor

github-actions bot commented Mar 1, 2025

This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

Copy link
Contributor

github-actions bot commented Mar 3, 2025

Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly.

@github-actions github-actions bot removed the conflicts Automatically applied to PRs with merge conflicts label Mar 3, 2025
@ntindle ntindle marked this pull request as ready for review March 3, 2025 22:29
@ntindle ntindle requested a review from a team as a code owner March 3, 2025 22:29
@ntindle ntindle requested review from Pwuts and Bentlybro and removed request for a team March 3, 2025 22:29
Copy link

qodo-merge-pro bot commented Mar 3, 2025

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

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

Error Handling

The low balance error handling and notification logic could lead to race conditions or duplicate notifications if multiple nodes fail simultaneously due to insufficient balance.

if isinstance(low_balance_error, InsufficientBalanceError):
    cls._handle_low_balance_notif(
        graph_exec.user_id,
        graph_exec.graph_id,
        exec_stats,
        low_balance_error,
    )
    raise low_balance_error
Null Safety

The output value handling change could lead to unexpected behavior if the input_data dictionary doesn't contain the expected keys or contains null values.

outputs[exec.input_data["name"]].append(
    exec.input_data.get("value", None)
)
Resource Cleanup

The running_executions dictionary items are only removed in the callback, which could lead to memory leaks if callbacks fail to execute.

running_executions: dict[str, AsyncResult] = {}
low_balance_error: Optional[InsufficientBalanceError] = None

@ntindle ntindle requested a review from majdyz March 4, 2025 05:13
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.

2 participants