Skip to content

Conversation

@kcons
Copy link
Member

@kcons kcons commented Nov 15, 2025

Since we know Detectors can go away (even before we have a chance to process a new occurrence) and we have a convention for representing that with DetectorGroup, update associate_new_group_with_detector accordingly.

@kcons kcons requested a review from cathteng November 15, 2025 00:03
@kcons kcons requested a review from a team as a code owner November 15, 2025 00:03
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Nov 15, 2025
Copy link
Member

@cathteng cathteng left a comment

Choose a reason for hiding this comment

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

nice

Comment on lines 382 to +385
},
)
return False

Copy link

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

@kcons kcons enabled auto-merge (squash) November 15, 2025 00:21
@kcons kcons merged commit e61a46b into master Nov 15, 2025
66 checks passed
@kcons kcons deleted the kcons/safetyfirst branch November 15, 2025 00:24
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