Skip to content

Commit 2e2c869

Browse files
authored
fix(issues) - Correct issues endpoint postgres timeout error handling (#88158)
follow up on #87691, it turns out the organization/issues endpoint actually did not use handle_query_errors and so this fix did not catch. added it now to the endpoint, aligning it with the other endpoints we have there. Also made the catch more specific, as QUERY_CANCELED from postgres are not always timeouts, same code is used for user cancelations, which do not apply to the error message we are converting to. Added endpoint tests as well to cover this.
1 parent d31eed4 commit 2e2c869

File tree

4 files changed

+72
-11
lines changed

4 files changed

+72
-11
lines changed

src/sentry/api/utils.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -427,7 +427,9 @@ def handle_query_errors() -> Generator[None]:
427427
raise APIException(detail=message)
428428
except OperationalError as error:
429429
if hasattr(error, "pgcode") and error.pgcode == psycopg2.errorcodes.QUERY_CANCELED:
430-
if options.get("api.postgres-query-timeout-error-handling.enabled"):
430+
error_message = str(error)
431+
is_timeout = "canceling statement due to statement timeout" in error_message
432+
if is_timeout and options.get("api.postgres-query-timeout-error-handling.enabled"):
431433
sentry_sdk.set_tag("query.error_reason", "Postgres statement timeout")
432434
sentry_sdk.capture_exception(error, level="warning")
433435
raise Throttled(

src/sentry/issues/endpoints/organization_group_index.py

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
from sentry.api.paginator import DateTimePaginator, Paginator
3030
from sentry.api.serializers import serialize
3131
from sentry.api.serializers.models.group_stream import StreamGroupSerializerSnuba
32-
from sentry.api.utils import get_date_range_from_stats_period
32+
from sentry.api.utils import get_date_range_from_stats_period, handle_query_errors
3333
from sentry.constants import ALLOWED_FUTURE_DELTA
3434
from sentry.exceptions import InvalidParams, InvalidSearchQuery
3535
from sentry.models.environment import Environment
@@ -41,7 +41,6 @@
4141
from sentry.search.events.constants import EQUALITY_OPERATORS
4242
from sentry.search.snuba.backend import assigned_or_suggested_filter
4343
from sentry.search.snuba.executors import FIRST_RELEASE_FILTERS, get_search_filter
44-
from sentry.snuba import discover
4544
from sentry.utils.cursors import Cursor, CursorResult
4645
from sentry.utils.validators import normalize_event_id
4746

@@ -366,14 +365,15 @@ def get(self, request: Request, organization: Organization) -> Response:
366365
return Response(serialize(groups, request.user, serializer(), request=request))
367366

368367
try:
369-
cursor_result, query_kwargs = self._search(
370-
request,
371-
organization,
372-
projects,
373-
environments,
374-
{"count_hits": True, "date_to": end, "date_from": start},
375-
)
376-
except (ValidationError, discover.InvalidSearchQuery) as exc:
368+
with handle_query_errors():
369+
cursor_result, query_kwargs = self._search(
370+
request,
371+
organization,
372+
projects,
373+
environments,
374+
{"count_hits": True, "date_to": end, "date_from": start},
375+
)
376+
except ValidationError as exc:
377377
return Response({"detail": str(exc)}, status=400)
378378

379379
results = list(cursor_result)

tests/sentry/api/test_utils.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,9 @@ def test_handle_postgres_timeout(self):
204204
class TimeoutError(OperationalError):
205205
pgcode = psycopg2.errorcodes.QUERY_CANCELED
206206

207+
def __str__(self):
208+
return "canceling statement due to statement timeout"
209+
207210
# Test when option is disabled (default)
208211
try:
209212
with handle_query_errors():
@@ -223,6 +226,21 @@ class TimeoutError(OperationalError):
223226
== "Query timeout. Please try with a smaller date range or fewer conditions."
224227
)
225228

229+
def test_handle_postgres_user_cancel(self):
230+
class UserCancelError(OperationalError):
231+
pgcode = psycopg2.errorcodes.QUERY_CANCELED
232+
233+
def __str__(self):
234+
return "canceling statement due to user request"
235+
236+
# Should propagate the error regardless of the feature flag
237+
with override_options({"api.postgres-query-timeout-error-handling.enabled": True}):
238+
try:
239+
with handle_query_errors():
240+
raise UserCancelError()
241+
except Exception as e:
242+
assert isinstance(e, UserCancelError) # Should propagate original error
243+
226244
@patch("sentry.api.utils.ParseError")
227245
def test_handle_other_operational_error(self, mock_parse_error):
228246
class OtherError(OperationalError):

tests/sentry/issues/endpoints/test_organization_group_index.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
from unittest.mock import MagicMock, Mock, call, patch
55
from uuid import uuid4
66

7+
import psycopg2
8+
from django.db import OperationalError
79
from django.urls import reverse
810
from django.utils import timezone
911

@@ -4059,6 +4061,45 @@ def test_flags_and_tags_query(self, _: MagicMock) -> None:
40594061
response = self.get_success_response(query='flags["test:flag"]:false')
40604062
assert len(json.loads(response.content)) == 0
40614063

4064+
def test_postgres_query_timeout(self, mock_query: MagicMock) -> None:
4065+
"""Test that a Postgres OperationalError with QueryCanceled pgcode becomes a 429 error
4066+
only when it's a statement timeout, and remains a 500 for user cancellation"""
4067+
4068+
class TimeoutError(OperationalError):
4069+
pgcode = psycopg2.errorcodes.QUERY_CANCELED
4070+
4071+
def __str__(self):
4072+
return "canceling statement due to statement timeout"
4073+
4074+
class UserCancelError(OperationalError):
4075+
pgcode = psycopg2.errorcodes.QUERY_CANCELED
4076+
4077+
def __str__(self):
4078+
return "canceling statement due to user request"
4079+
4080+
self.login_as(user=self.user)
4081+
4082+
# Test statement timeout when option is disabled (default)
4083+
mock_query.side_effect = TimeoutError()
4084+
response = self.get_response()
4085+
assert response.status_code == 500 # Should propagate original error
4086+
4087+
# Test statement timeout when option is enabled
4088+
with override_options({"api.postgres-query-timeout-error-handling.enabled": True}):
4089+
mock_query.side_effect = TimeoutError()
4090+
response = self.get_response()
4091+
assert response.status_code == 429
4092+
assert (
4093+
response.data["detail"]
4094+
== "Query timeout. Please try with a smaller date range or fewer conditions."
4095+
)
4096+
4097+
# Test user cancellation - should return 500 regardless of feature flag
4098+
with override_options({"api.postgres-query-timeout-error-handling.enabled": True}):
4099+
mock_query.side_effect = UserCancelError()
4100+
response = self.get_response()
4101+
assert response.status_code == 500
4102+
40624103

40634104
class GroupUpdateTest(APITestCase, SnubaTestCase):
40644105
endpoint = "sentry-api-0-organization-group-index"

0 commit comments

Comments
 (0)