Skip to content

fix(alerts): Stronger typing around human_desc#117883

Open
kcons wants to merge 3 commits into
masterfrom
kcons/moretypes4
Open

fix(alerts): Stronger typing around human_desc#117883
kcons wants to merge 3 commits into
masterfrom
kcons/moretypes4

Conversation

@kcons

@kcons kcons commented Jun 16, 2026

Copy link
Copy Markdown
Member

There were previously exceptions due to type issues that should've been easy to catch statically here, so it's worth being explicit with the typing and simplifying where we can to avoid such issues recurring.

I aimed to minimize behavior changes, but found 4 pre-existing bugs in the process that addition type refinements and simplifications made visible to tools, so fixed those too.

@kcons kcons requested a review from a team as a code owner June 16, 2026 22:32
@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Jun 16, 2026

@saponifi3d saponifi3d left a comment

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.

typppeeesss! (thanks for keeping this a smaller focused pr!) lgtm.

Comment thread src/sentry/incidents/endpoints/serializers/alert_rule_trigger_action.py Outdated
priority = obj.data.get("priority")
priority: str | None = obj.data.get("priority")
type_value = ActionService.get_value(obj.type)
assert type_value is not None, f"Unknown ActionService for type {obj.type}"

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 assert in WorkflowEngineActionSerializer will cause a crash when serializing workflow actions with types like GITHUB or JIRA, as they are not supported by ActionService.
Severity: HIGH

Suggested Fix

Instead of asserting, the serializer should gracefully handle unsupported action types. This could involve filtering them out from the serialized output or logging a warning and skipping the problematic action, which would prevent the API endpoint from crashing.

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/incidents/endpoints/serializers/workflow_engine_action.py#L58

Potential issue: The `WorkflowEngineActionSerializer` uses an `assert` to validate that
the `obj.type` of a `workflow_engine.Action` is a valid `ActionService`. However, the
`Action.Type` enum allows for types like `GITHUB`, `JIRA`, `WEBHOOK`, and `PLUGIN`,
which are not defined in `ActionService`. When `ActionService.get_value()` is called
with one of these unsupported types, it returns `None`. This triggers the `assert
type_value is not None` statement, raising an `AssertionError` and crashing the API
endpoint responsible for serializing metric alert rules. This will prevent users with
alert rules configured with these action types from viewing or managing them.

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

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.

ok, valid point.

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.

2 participants