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): move failed messages into a dead letter queue #9501

Merged

Conversation

ntindle
Copy link
Member

@ntindle ntindle commented Feb 19, 2025

When we fail to process something, we don't want to keep retrying forever. We should store those and process them later

Changes 🏗️

  • Fix the type of the failed exchange from Direct to Topic to allow filtering based on name (allows us later to do more advanced handling of queue types)
  • abstract processing the messages in a queue a bit to reduce repeated code
  • abstract how we check if a user wants a notification so that its a bit easier to process
  • Handle errors better
  • Abstract model parsing

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:
    • ...
Example test plan
  • 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

@ntindle ntindle requested a review from a team as a code owner February 19, 2025 23:45
@ntindle ntindle requested review from Pwuts and majdyz and removed request for a team February 19, 2025 23:45
@github-actions github-actions bot added the platform/backend AutoGPT Platform - Back end label Feb 19, 2025
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

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

Error Handling

The _run_queue method accesses the 'message' variable in the except block after it may have been unbound if QueueEmpty is raised. This could cause a NameError.

except Exception as e:
    logger.error(f"Error in notification service loop: {e}")
    self.run_and_wait(message.reject(requeue=False))
Missing Return

The _should_email_user_based_on_preference function appears to have a syntax error - missing closing parenthesis which could affect the return value.

async def _should_email_user_based_on_preference(
    self, user_id: str, event_type: NotificationType
) -> bool:
    return (
        get_db_client()
        .get_user_notification_preference(user_id)
        .preferences[event_type]
    )

Copy link

netlify bot commented Feb 19, 2025

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

Name Link
🔨 Latest commit 6d3f8cb
🔍 Latest deploy log https://app.netlify.com/sites/auto-gpt-docs-dev/deploys/67b6fded9bc9f10008daf685

Copy link

deepsource-io bot commented Feb 19, 2025

Here's the code health analysis summary for commits 6300563..6d3f8cb. View details on DeepSource ↗.

Analysis Summary

AnalyzerStatusSummaryLink
DeepSource JavaScript LogoJavaScript✅ SuccessView Check ↗
DeepSource Python LogoPython✅ Success
❗ 19 occurences introduced
View Check ↗

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

Copy link

netlify bot commented Feb 19, 2025

Deploy Preview for auto-gpt-docs canceled.

Name Link
🔨 Latest commit 6d3f8cb
🔍 Latest deploy log https://app.netlify.com/sites/auto-gpt-docs/deploys/67b6fdedc2034a0008ccc891

@github-actions github-actions bot added size/l and removed size/m labels Feb 19, 2025
Copy link
Member

@Bentlybro Bentlybro left a comment

Choose a reason for hiding this comment

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

Tested and works :)

@Bentlybro Bentlybro enabled auto-merge February 20, 2025 10:03
@Bentlybro Bentlybro added this pull request to the merge queue Feb 20, 2025
Merged via the queue into dev with commit 4ae0166 Feb 20, 2025
22 checks passed
@Bentlybro Bentlybro deleted the ntindle/secrt-1093-push-failed-messages-to-failed-queue branch February 20, 2025 10:27
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