Skip to content

fix(workflows): Handle duplicated AlertRuleWorkflow entries for a Workflow more gracefully#113138

Merged
kcons merged 1 commit intomasterfrom
kcons/bandaid
Apr 16, 2026
Merged

fix(workflows): Handle duplicated AlertRuleWorkflow entries for a Workflow more gracefully#113138
kcons merged 1 commit intomasterfrom
kcons/bandaid

Conversation

@kcons
Copy link
Copy Markdown
Member

@kcons kcons commented Apr 15, 2026

We've got a few fixes for this situation in the works, but if we can avoid crashing in the meantime that seems ideal.

Part of ISWF-2470.

@kcons kcons requested a review from a team as a code owner April 15, 2026 23:39
@linear-code
Copy link
Copy Markdown

linear-code bot commented Apr 15, 2026

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Apr 15, 2026
Comment on lines 67 to 69
rule_id = alert_rule_workflow.rule_id
except AlertRuleWorkflow.DoesNotExist:
rule_id = None
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The call to AlertRuleWorkflow.objects.get() does not handle the MultipleObjectsReturned exception, which can occur if duplicate entries exist for a project, leading to a crash.
Severity: HIGH

Suggested Fix

Modify the except block to catch both AlertRuleWorkflow.DoesNotExist and AlertRuleWorkflow.MultipleObjectsReturned exceptions. This pattern is common elsewhere in the codebase for handling .get() calls.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: src/sentry/rules/history/backends/postgres.py#L67-L69

Potential issue: The `get_rule_workflow` function calls
`AlertRuleWorkflow.objects.get()` to retrieve a workflow. While the query is filtered by
`project_id`, it is still possible for multiple `AlertRuleWorkflow` entries to exist for
different rules within the same project. The code only handles the `DoesNotExist`
exception. If the query returns multiple objects, it will raise an unhandled
`MultipleObjectsReturned` exception, causing a crash in the
`ProjectRuleStatsIndexEndpoint` and `ProjectRuleGroupHistoryIndexEndpoint` API handlers.

Did we get this right? 👍 / 👎 to inform future reviews.

Copy link
Copy Markdown
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 926e629. Configure here.

# scope to the project. Django evaluates this as a single SQL
# statement (subquery), not a separate round-trip.
qs = qs.filter(rule_id__in=Rule.objects.filter(project_id=project_id).values("id"))
alert_rule_workflow = qs.get()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

MultipleObjectsReturned not caught despite graceful-handling intent

Medium Severity

The qs.get() call can still raise MultipleObjectsReturned if more than one AlertRuleWorkflow matches after filtering — for example, if two distinct rules in the same project both point to the same workflow. The try/except only catches AlertRuleWorkflow.DoesNotExist, so a MultipleObjectsReturned exception would propagate as an unhandled 500 error. Since the PR's stated goal is to "avoid crashing," catching (or otherwise handling) MultipleObjectsReturned — e.g., by using qs.first() or adding it to the except clause — would make the fix more robust.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 926e629. Configure here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

it's true, but shouldn't convention avoid this and we'd want to be notified about this as a different error?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yep.

@kcons
Copy link
Copy Markdown
Member Author

kcons commented Apr 15, 2026

The bots are right, this doesn't fully prevent, but we're only trying to avoid the known duplication issue with one item per project.

@kcons kcons merged commit 753dc7b into master Apr 16, 2026
57 checks passed
@kcons kcons deleted the kcons/bandaid branch April 16, 2026 00:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants