Browse Source

decouple handle_query_errors and move into api utils (#62232)

In many places we're extending the `OrganizationEventsEndpointBase`
purely for access to the `handle_query_errors` helper.

There are notes in multiple area's mentioning that this helper should be
decoupled from the base class implementations.
such as
[here](https://github.com/getsentry/sentry/blob/master/src/sentry/api/endpoints/organization_stats_v2.py#L217)
and
[here](https://github.com/getsentry/sentry/blob/master/src/sentry/api/endpoints/organization_sessions.py#L22)
Nathan Hsieh 1 year ago
parent
commit
c09a546738
2 changed files with 141 additions and 3 deletions
  1. 87 2
      src/sentry/api/utils.py
  2. 54 1
      tests/sentry/api/test_utils.py

+ 87 - 2
src/sentry/api/utils.py

@@ -5,24 +5,28 @@ import logging
 import re
 import sys
 import traceback
+from contextlib import contextmanager
 from datetime import timedelta
-from typing import Any, List, Literal, Mapping, Tuple, overload
+from typing import Any, Generator, List, Literal, Mapping, Tuple, overload
 from urllib.parse import urlparse
 
+import sentry_sdk
 from django.conf import settings
 from django.http import HttpResponseNotAllowed
 from django.utils import timezone
 from django.views.decorators.csrf import csrf_exempt
+from rest_framework.exceptions import APIException, ParseError
 from rest_framework.request import Request
 from sentry_sdk import Scope
 
 from sentry import options
 from sentry.auth.superuser import is_active_superuser
-from sentry.exceptions import InvalidParams
+from sentry.exceptions import IncompatibleMetricsQuery, InvalidParams, InvalidSearchQuery
 from sentry.models.apikey import is_api_key_auth
 from sentry.models.apitoken import is_api_token_auth
 from sentry.models.organization import Organization
 from sentry.models.orgauthtoken import is_org_auth_token_auth
+from sentry.search.events.constants import TIMEOUT_ERROR_MESSAGE
 from sentry.search.utils import InvalidQuery, parse_datetime_string
 from sentry.services.hybrid_cloud import extract_id_from
 from sentry.services.hybrid_cloud.organization import (
@@ -33,6 +37,22 @@ from sentry.services.hybrid_cloud.organization import (
 )
 from sentry.utils.dates import parse_stats_period
 from sentry.utils.sdk import capture_exception, merge_context_into_scope
+from sentry.utils.snuba import (
+    DatasetSelectionError,
+    QueryConnectionFailed,
+    QueryExecutionError,
+    QueryExecutionTimeMaximum,
+    QueryIllegalTypeOfArgument,
+    QueryMemoryLimitExceeded,
+    QueryMissingColumn,
+    QueryOutsideRetentionError,
+    QuerySizeExceeded,
+    QueryTooManySimultaneous,
+    RateLimitExceeded,
+    SchemaValidationError,
+    SnubaError,
+    UnqualifiedQueryError,
+)
 
 logger = logging.getLogger(__name__)
 
@@ -346,3 +366,68 @@ def get_auth_api_token_type(auth: object) -> str | None:
     if is_api_key_auth(auth):
         return "api_key"
     return None
+
+
+# NOTE: This duplicates OrganizationEventsEndpointBase.handle_query_errors
+# TODO: move other references over to this implementation rather than the organization_events implementation
+@contextmanager
+def handle_query_errors(self) -> Generator[None, None, None]:
+    try:
+        yield
+    except InvalidSearchQuery as error:
+        message = str(error)
+        # Special case the project message since it has so many variants so tagging is messy otherwise
+        if message.endswith("do not exist or are not actively selected."):
+            sentry_sdk.set_tag(
+                "query.error_reason", "Project in query does not exist or not selected"
+            )
+        else:
+            sentry_sdk.set_tag("query.error_reason", message)
+        raise ParseError(detail=message)
+    except ArithmeticError as error:
+        message = str(error)
+        sentry_sdk.set_tag("query.error_reason", message)
+        raise ParseError(detail=message)
+    except QueryOutsideRetentionError as error:
+        sentry_sdk.set_tag("query.error_reason", "QueryOutsideRetentionError")
+        raise ParseError(detail=str(error))
+    except QueryIllegalTypeOfArgument:
+        message = "Invalid query. Argument to function is wrong type."
+        sentry_sdk.set_tag("query.error_reason", message)
+        raise ParseError(detail=message)
+    except IncompatibleMetricsQuery as error:
+        message = str(error)
+        sentry_sdk.set_tag("query.error_reason", f"Metric Error: {message}")
+        raise ParseError(detail=message)
+    except SnubaError as error:
+        message = "Internal error. Please try again."
+        if isinstance(
+            error,
+            (
+                RateLimitExceeded,
+                QueryMemoryLimitExceeded,
+                QueryExecutionTimeMaximum,
+                QueryTooManySimultaneous,
+            ),
+        ):
+            sentry_sdk.set_tag("query.error_reason", "Timeout")
+            raise ParseError(detail=TIMEOUT_ERROR_MESSAGE)
+        elif isinstance(error, (UnqualifiedQueryError)):
+            sentry_sdk.set_tag("query.error_reason", str(error))
+            raise ParseError(detail=str(error))
+        elif isinstance(
+            error,
+            (
+                DatasetSelectionError,
+                QueryConnectionFailed,
+                QueryExecutionError,
+                QuerySizeExceeded,
+                SchemaValidationError,
+                QueryMissingColumn,
+            ),
+        ):
+            sentry_sdk.capture_exception(error)
+            message = "Internal error. Your query failed to run."
+        else:
+            sentry_sdk.capture_exception(error)
+        raise APIException(detail=message)

+ 54 - 1
tests/sentry/api/test_utils.py

@@ -4,6 +4,7 @@ from unittest.mock import MagicMock, patch
 
 import pytest
 from django.utils import timezone
+from rest_framework.exceptions import APIException
 from sentry_sdk import Scope
 from sentry_sdk.utils import exc_info_from_error
 
@@ -11,11 +12,28 @@ from sentry.api.utils import (
     MAX_STATS_PERIOD,
     customer_domain_path,
     get_date_range_from_params,
+    handle_query_errors,
     print_and_capture_handler_exception,
 )
-from sentry.exceptions import InvalidParams
+from sentry.exceptions import IncompatibleMetricsQuery, InvalidParams, InvalidSearchQuery
 from sentry.testutils.cases import APITestCase
 from sentry.testutils.helpers.datetime import freeze_time
+from sentry.utils.snuba import (
+    DatasetSelectionError,
+    QueryConnectionFailed,
+    QueryExecutionError,
+    QueryExecutionTimeMaximum,
+    QueryIllegalTypeOfArgument,
+    QueryMemoryLimitExceeded,
+    QueryMissingColumn,
+    QueryOutsideRetentionError,
+    QuerySizeExceeded,
+    QueryTooManySimultaneous,
+    RateLimitExceeded,
+    SchemaValidationError,
+    SnubaError,
+    UnqualifiedQueryError,
+)
 
 
 class GetDateRangeFromParamsTest(unittest.TestCase):
@@ -200,3 +218,38 @@ def test_customer_domain_path():
     ]
     for input_path, expected in scenarios:
         assert expected == customer_domain_path(input_path)
+
+
+class FooBarError(Exception):
+    pass
+
+
+class HandleQueryErrorsTest:
+    @patch("sentry.api.utils.ParseError")
+    def test_handle_query_errors(self, mock_parse_error):
+        exceptions = [
+            DatasetSelectionError,
+            IncompatibleMetricsQuery,
+            InvalidParams,
+            InvalidSearchQuery,
+            QueryConnectionFailed,
+            QueryExecutionError,
+            QueryExecutionTimeMaximum,
+            QueryIllegalTypeOfArgument,
+            QueryMemoryLimitExceeded,
+            QueryMissingColumn,
+            QueryOutsideRetentionError,
+            QuerySizeExceeded,
+            QueryTooManySimultaneous,
+            RateLimitExceeded,
+            SchemaValidationError,
+            SnubaError,
+            UnqualifiedQueryError,
+        ]
+        mock_parse_error.return_value = FooBarError()
+        for ex in exceptions:
+            try:
+                with handle_query_errors(self):
+                    raise ex
+            except Exception as e:
+                assert isinstance(e, (FooBarError, APIException))