ref(errors): Explicitly throw on empty group_id snuba query#110923
Conversation
dc9c61d to
fb8f0c5
Compare
| if len(in_groups) > 0: | ||
| triple = ["group_id", "IN", get_all_merged_group_ids(in_groups)] | ||
| else: | ||
| raise UnsupportedSearchQuery("Found empty intersection of group_ids") |
There was a problem hiding this comment.
Bug: The new UnsupportedSearchQuery exception is not handled in all code paths that call SnubaQueryParams, such as in preview.py and backend.py, creating a risk of unhandled exceptions.
Severity: MEDIUM
Suggested Fix
Wrap the calls to SnubaQueryParams or the functions that use it (like raw_query and bulk_raw_query) in preview.py and backend.py with a try...except UnsupportedSearchQuery block, similar to the handling in executors.py. This will ensure the exception is caught consistently across all call sites.
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/utils/snuba.py#L984
Potential issue: The `SnubaQueryParams.__init__` method can now raise an
`UnsupportedSearchQuery` exception when `group_id` filtering conditions result in an
empty set. While this exception is caught in the search path (`executors.py`), it is not
handled in other callers. Specifically, calls to `raw_query` and `bulk_raw_query` from
`src/sentry/rules/history/preview.py` and
`src/sentry/services/eventstore/snuba/backend.py` do not have exception handlers for
`UnsupportedSearchQuery`. This creates a risk of unhandled exceptions in those code
paths if the triggering conditions are met.
fb8f0c5 to
b43f715
Compare
| if len(in_groups) > 0: | ||
| triple = ["group_id", "IN", get_all_merged_group_ids(in_groups)] | ||
| else: | ||
| raise UnsupportedSearchQuery("Found empty intersection of group_ids") |
There was a problem hiding this comment.
Bug: The new UnsupportedSearchQuery exception is not handled in GroupSerializerSnuba, causing an API error when user search filters result in an empty set of group IDs.
Severity: HIGH
Suggested Fix
Wrap the calls to aliased_query within the _execute_error_seen_stats_query and _execute_generic_seen_stats_query methods in GroupSerializerSnuba with a try...except UnsupportedSearchQuery block to gracefully handle cases where no groups match the search conditions.
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/utils/snuba.py#L984
Potential issue: The introduction of `UnsupportedSearchQuery` in
`SnubaQueryParams._preprocess_group_id_redirects` can cause unhandled exceptions.
Specifically, the `GroupSerializerSnuba` class calls `aliased_query` in its
`_execute_error_seen_stats_query` and `_execute_generic_seen_stats_query` methods. These
calls can trigger the new exception if a user-supplied search filter (e.g.,
`group_id:X`) conflicts with the groups being serialized, resulting in an empty set of
group IDs. Because there is no `try...except` block in these methods to catch
`UnsupportedSearchQuery`, the exception will bubble up and cause an API error.
b43f715 to
457b017
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| if len(in_groups) > 0: | ||
| triple = ["group_id", "IN", get_all_merged_group_ids(in_groups)] | ||
| else: | ||
| raise UnsupportedSearchQuery("Found empty intersection of group_ids") |
There was a problem hiding this comment.
Bug: The new UnsupportedSearchQuery exception raised for empty group_id intersections is not handled in src/sentry/rules/history/preview.py, which can lead to unhandled exceptions.
Severity: HIGH
Suggested Fix
The preview() function in src/sentry/rules/history/preview.py should be updated to also catch UnsupportedSearchQuery. This will prevent the exception from propagating uncaught and crashing the request handler.
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/utils/snuba.py#L984
Potential issue: The `preview.py` module constructs `SnubaQueryParams`, which, with the
new changes, can raise an `UnsupportedSearchQuery` exception if an empty `group_ids`
list is provided (e.g., from `get_top_groups`). This results in a `("group_id", "IN",
[])` condition. The exception is raised during `SnubaQueryParams` initialization but is
not caught by the `preview()` function, which only handles `PreviewException`. This will
cause an unhandled exception to propagate and crash the request handler for the rule
preview API.
This breaks clickhouse. Let's raise a SnubaError that spells out the problem more clearly instead of letting things get that far.
457b017 to
71fc556
Compare
| else: | ||
| raise SnubaError("Found empty intersection of group_ids") |
There was a problem hiding this comment.
Bug: The code raises a SnubaError for an empty group_id intersection, but the calling loop only catches UnsupportedSearchQuery, leading to an unhandled exception.
Severity: MEDIUM
Suggested Fix
Modify the exception handling in the loop within executors.py to also catch SnubaError. This will prevent the unhandled exception from propagating and allow the system to handle cases with empty group_id intersections gracefully, similar to how UnsupportedSearchQuery is handled.
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/utils/snuba.py#L982-L983
Potential issue: In `_preprocess_group_id_redirects`, a `SnubaError` is raised if the
intersection of `group_id` filters is empty. This function is called during parameter
preparation within a loop in `executors.py`. However, this loop only has an exception
handler for `UnsupportedSearchQuery`. Since `SnubaError` does not inherit from
`UnsupportedSearchQuery`, the exception is not caught. This will cause the unhandled
`SnubaError` to propagate up the call stack, resulting in an API error instead of being
gracefully handled. This scenario occurs with conflicting `group_id` filters, especially
after group merging.
This breaks clickhouse. Let's raise a SnubaError that spells out the problem more clearly instead of letting things get that far.

This breaks clickhouse. Let's raise a SnubaError that spells out the problem more clearly instead of letting things get that far.