Skip to content

fix(workflow_engine): Normalize error.handled values to 0/1#115740

Merged
kcons merged 6 commits into
masterfrom
kcons/unhand
May 18, 2026
Merged

fix(workflow_engine): Normalize error.handled values to 0/1#115740
kcons merged 6 commits into
masterfrom
kcons/unhand

Conversation

@kcons
Copy link
Copy Markdown
Member

@kcons kcons commented May 18, 2026

This is essentially reproducing the logic at src/sentry/api/event_search.py#L1132-L1136 to coerce this type.
Ideally, we'd be able to reuse the parsing logic to handle our filters generally, but it isn't an easy match.

Query showing known cases https://redash.getsentry.net/queries/11226/source.

@kcons kcons requested a review from a team as a code owner May 18, 2026 19:15
@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label May 18, 2026
Comment thread src/sentry/workflow_engine/handlers/condition/event_frequency_query_handlers.py Outdated
Comment thread src/sentry/workflow_engine/handlers/condition/event_frequency_query_handlers.py Outdated
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 57250fc. Configure here.

Comment thread src/sentry/workflow_engine/handlers/condition/event_frequency_query_handlers.py Outdated
Comment on lines +244 to +250
comparable = raw.lower() if isinstance(raw, str) else raw
if comparable in _HANDLED_TRUE_VALUES:
int_value = 1
elif comparable in _HANDLED_FALSE_VALUES:
int_value = 0
else:
logger.error(
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: A filter on error.handled with an IS_IN or NOT_IN match will crash if the value is a single boolean-like string like "true".
Severity: MEDIUM

Suggested Fix

Before the IS_IN/NOT_IN match handling, ensure that if the rhs value was normalized from a boolean-like string to an integer, it is converted back to a string. This will satisfy the type expectation of the IS_IN handler and prevent the ValueError. Alternatively, the IS_IN handler could be updated to gracefully handle integer inputs for boolean 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/handlers/condition/event_frequency_query_handlers.py#L244-L250

Potential issue: When a filter condition uses `error.handled` with an `IS_IN` or
`NOT_IN` match type, and the provided value is a single boolean-like string (e.g.,
`"true"`), the system will crash. The normalization logic converts the string value to
an integer (`1` or `0`) and assigns it to the `rhs` variable. However, the subsequent
`IS_IN`/`NOT_IN` handler expects `rhs` to be a string. The type check `isinstance(rhs,
str)` fails, leading to an unhandled `ValueError`. Although using `IS_IN` for a boolean
attribute is uncommon, the UI permits this configuration, making this a reachable crash
scenario.

Copy link
Copy Markdown
Member

@ceorourke ceorourke left a comment

Choose a reason for hiding this comment

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

very nice

@kcons kcons enabled auto-merge (squash) May 18, 2026 21:37
@kcons kcons merged commit 4de0c1c into master May 18, 2026
64 checks passed
@kcons kcons deleted the kcons/unhand branch May 18, 2026 21:40
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