Skip to content

fix(aci): Add support for an empty conditionGroup in the API for the Error detector #112641

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
saponifi3d merged 3 commits into from
Apr 10, 2026
Merged

fix(aci): Add support for an empty conditionGroup in the API for the Error detector #112641

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
354cd53
Accept an empty conditionGroup in the API for the error detector
saponifi3d Apr 9, 2026
a6bf103
'fix' mypy by ignorning it. We should eventually get this consistent …
saponifi3d Apr 10, 2026
7eaf842
update method can now support as a value for data conditions
saponifi3d Apr 10, 2026
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
2 changes: 1 addition & 1 deletion src/sentry/workflow_engine/endpoints/validators/base/detector.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ def update(self, instance: Detector, validated_data: dict[str, Any]) -> Detector
validated_data.pop("owner")
)

if "condition_group" in validated_data:
if "condition_group" in validated_data and validated_data.get("condition_group"):
condition_group = validated_data.pop("condition_group")
data_conditions: list[DataConditionType] = condition_group.get("conditions")

Expand Down
12 changes: 10 additions & 2 deletions src/sentry/workflow_engine/endpoints/validators/error_detector.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,14 @@
class ErrorDetectorValidator(BaseDetectorTypeValidator):
data_source_required = False

# TODO - Update the Error Detector to store grouping rules in the DataCondition
condition_group = serializers.DictField(
required=False,
allow_null=True,
) # type: ignore[assignment] # Intentionally overrides nested serializer to accept empty dicts
Copy link
Copy Markdown
Contributor Author

@saponifi3d saponifi3d Apr 10, 2026

Choose a reason for hiding this comment

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

just fyi - the TODO is the point in which we could clean this up. the problem is mostly related to the error detector not storing the grouping rules as data conditions / this model.


fingerprinting_rules = serializers.CharField(required=False, allow_blank=True, allow_null=True)

resolve_age = EmptyIntegerField(
required=False,
allow_null=True,
Expand All @@ -31,11 +38,12 @@ def validate_type(self, value: str) -> builtins.type[GroupType]:
return type

def validate_condition_group(self, value: Any) -> Any:
if value is not None:
if value:
raise serializers.ValidationError(
"Condition group is not supported for error detectors"
)
return value

return None
Comment on lines 40 to +46
Copy link
Copy Markdown
Contributor

@sentry sentry bot Apr 10, 2026

Choose a reason for hiding this comment

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

Bug: Returning None from validate_condition_group for an empty dictionary causes an AttributeError during the update operation because the update method doesn't expect a None value.
Severity: CRITICAL

Suggested Fix

Either modify validate_condition_group() to return an empty dictionary instead of None, or add a check in the update method to handle cases where condition_group is None before attempting to access its attributes.

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/endpoints/validators/error_detector.py#L40-L46

Potential issue: The `validate_condition_group` method was changed to return `None` when
an empty dictionary is provided. While this allows the validation to pass, the
serializer's `update` method does not handle this `None` value. When updating an Error
Detector with an empty `condition_group`, the code later attempts to call the `.get()`
method on this `None` value, causing an `AttributeError: 'NoneType' object has no
attribute 'get'` and crashing the request. This occurs because there is no null check
before the attribute access in the `update` logic.

Copy link
Copy Markdown
Contributor Author

@saponifi3d saponifi3d Apr 10, 2026

Choose a reason for hiding this comment

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

changed the .update logic to also ensure that it's not none for safety.


def validate_name(self, value: Any) -> str:
# if name is different from existing, raise an error
Expand Down
8 changes: 8 additions & 0 deletions tests/sentry/workflow_engine/detectors/test_error_detector.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,14 @@ def test_invalid_condition_group(self) -> None:
)
]

def test_valid_condition_group(self) -> None:
data = {
**self.valid_data,
"condition_group": {},
}
validator = ErrorDetectorValidator(data=data, context=self.context)
assert validator.is_valid()

def test_update_existing_with_valid_data(self) -> None:
user = self.create_user()
self.create_member(organization=self.project.organization, user=user)
Expand Down
Loading