Skip to content

fix(aci): Make associate_new_group_with_detector more robust #103418

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Nov 15, 2025
+34 −0
Merged

fix(aci): Make associate_new_group_with_detector more robust #103418

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions src/sentry/workflow_engine/processors/detector.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,28 @@ def associate_new_group_with_detector(group: Group, detector_id: int | None = No
},
)
return False

Comment on lines 382 to +385
Copy link

@sentry sentry bot Nov 15, 2025

Choose a reason for hiding this comment

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

Bug: Detector.get_error_detector_for_project() lacks error handling, causing Detector.DoesNotExist to crash the function if no detector is found.
Severity: CRITICAL | Confidence: 0.95

🔍 Detailed Analysis

The code at src/sentry/workflow_engine/processors/detector.py lines 382-384 calls Detector.get_error_detector_for_project(group.project.id).id without error handling. The get_error_detector_for_project() method uses Django's .get(), which raises Detector.DoesNotExist if no matching detector is found for a project. This unhandled exception will crash the associate_new_group_with_detector() function if an error detector is missing for a project, leading to unexpected failures in the workflow engine.

💡 Suggested Fix

Wrap the call to Detector.get_error_detector_for_project() in a try-except Detector.DoesNotExist block. If the exception occurs, set detector_id = None to gracefully handle the missing detector, similar to how it's handled later in the function.

🤖 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/workflow_engine/processors/detector.py#L382-L385

Potential issue: The code at `src/sentry/workflow_engine/processors/detector.py` lines
382-384 calls `Detector.get_error_detector_for_project(group.project.id).id` without
error handling. The `get_error_detector_for_project()` method uses Django's `.get()`,
which raises `Detector.DoesNotExist` if no matching detector is found for a project.
This unhandled exception will crash the `associate_new_group_with_detector()` function
if an error detector is missing for a project, leading to unexpected failures in the
workflow engine.

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

Reference_id: 2701266

# Check if the detector exists. If not, create DetectorGroup with null detector_id
# to make it clear that we were associated with a detector that no longer exists.
if not Detector.objects.filter(id=detector_id).exists():
metrics.incr(
"workflow_engine.associate_new_group_with_detector",
tags={"group_type": group.type, "result": "detector_missing"},
)
logger.warning(
"associate_new_group_with_detector_detector_missing",
extra={
"group_id": group.id,
"group_type": group.type,
"detector_id": detector_id,
},
)
DetectorGroup.objects.get_or_create(
detector_id=None,
group_id=group.id,
)
return True

DetectorGroup.objects.get_or_create(
detector_id=detector_id,
group_id=group.id,
Expand Down
12 changes: 12 additions & 0 deletions tests/sentry/workflow_engine/processors/test_detector.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1058,3 +1058,15 @@ def test_feedback_group_returns_false(self) -> None:
group = self.create_group(project=self.project, type=FeedbackGroup.type_id)
assert not associate_new_group_with_detector(group)
assert not DetectorGroup.objects.filter(group_id=group.id).exists()

def test_deleted_detector_creates_null_association(self) -> None:
group = self.create_group(project=self.project, type=MetricIssue.type_id)
deleted_detector_id = self.metric_detector.id

self.metric_detector.delete()

assert associate_new_group_with_detector(group, deleted_detector_id)

detector_group = DetectorGroup.objects.get(group_id=group.id)
assert detector_group.detector_id is None
assert detector_group.group_id == group.id
Loading