ref(eco): Cleans up code for integration proxy error handling

src/sentry/integrations/api/endpoints/integration_proxy.py

@@ -3,6 +3,7 @@
 import logging
 from collections import defaultdict
 from collections.abc import Mapping
+from dataclasses import dataclass
 from enum import StrEnum
 from typing import Any
 from urllib.parse import urljoin
@@ -23,7 +24,12 @@
 from sentry.integrations.models.organization_integration import OrganizationIntegration
 from sentry.integrations.utils.metrics import IntegrationProxyEvent, IntegrationProxyEventType
 from sentry.metrics.base import Tags
-from sentry.shared_integrations.exceptions import ApiHostError, ApiTimeoutError
+from sentry.shared_integrations.exceptions import (
+    ApiForbiddenError,
+    ApiHostError,
+    ApiTimeoutError,
+    ApiUnauthorized,
+)
 from sentry.silo.base import SiloMode
 from sentry.silo.util import (
     PROXY_BASE_URL_HEADER,
@@ -57,12 +63,48 @@ class IntegrationProxyFailureMetricType(StrEnum):
     INVALID_SENDER = "invalid_sender"
     INVALID_REQUEST = "invalid_request"
     INVALID_IDENTITY = "invalid_identity"
+    UNAUTHORIZED = "unauthorized"
     HOST_UNREACHABLE_ERROR = "host_unreachable_error"
     HOST_TIMEOUT_ERROR = "host_timeout_error"
+    FORBIDDEN_ERROR = "forbidden_error"
     UNKNOWN_ERROR = "unknown_error"
     FAILED_VALIDATION = "failed_validation"
 
 
+@dataclass
+class IntegrationProxyErrorMapping:
+    failure_type: IntegrationProxyFailureMetricType
+    message: str
+    status_code: int
+
+
+IntegrationProxyErrorMap: dict[type[Exception], IntegrationProxyErrorMapping] = {
+    IdentityNotValid: IntegrationProxyErrorMapping(
+        IntegrationProxyFailureMetricType.INVALID_IDENTITY,
+        "invalid_identity",
+        ApiUnauthorized.code,
+    ),
+    ApiForbiddenError: IntegrationProxyErrorMapping(
+        IntegrationProxyFailureMetricType.FORBIDDEN_ERROR, "forbidden_error", ApiForbiddenError.code
+    ),
+    ApiUnauthorized: IntegrationProxyErrorMapping(
+        IntegrationProxyFailureMetricType.UNAUTHORIZED,
+        "unauthorized",
+        ApiUnauthorized.code,
+    ),
+    ApiHostError: IntegrationProxyErrorMapping(
+        IntegrationProxyFailureMetricType.HOST_UNREACHABLE_ERROR,
+        "host_unreachable_error",
+        ApiHostError.code,
+    ),
+    ApiTimeoutError: IntegrationProxyErrorMapping(
+        IntegrationProxyFailureMetricType.HOST_TIMEOUT_ERROR,
+        "host_timeout_error",
+        ApiTimeoutError.code,
+    ),
+}
+
+
 @control_silo_endpoint
 class InternalIntegrationProxyEndpoint(Endpoint):
     publish_status = defaultdict(lambda: ApiPublishStatus.PRIVATE)
@@ -313,20 +355,14 @@ def handle_exception_with_details(
         handler_context: Mapping[str, Any] | None = None,
         scope: Scope | None = None,
     ) -> DRFResponse:
-        if isinstance(exc, IdentityNotValid):
-            logger.warning("hybrid_cloud.integration_proxy.invalid_identity", extra=self.log_extra)
-            self._add_failure_metric(IntegrationProxyFailureMetricType.INVALID_IDENTITY)
-            return self.respond(status=400)
-        elif isinstance(exc, ApiHostError):
-            logger.info(
-                "hybrid_cloud.integration_proxy.host_unreachable_error", extra=self.log_extra
-            )
-            self._add_failure_metric(IntegrationProxyFailureMetricType.HOST_UNREACHABLE_ERROR)
-            return self.respond(status=exc.code)
-        elif isinstance(exc, ApiTimeoutError):
-            logger.info("hybrid_cloud.integration_proxy.host_timeout_error", extra=self.log_extra)
-            self._add_failure_metric(IntegrationProxyFailureMetricType.HOST_TIMEOUT_ERROR)
-            return self.respond(status=exc.code)
+        for exc_type, error_mapping in IntegrationProxyErrorMap.items():
+            if isinstance(exc, exc_type):
+                logger.warning(
+                    "hybrid_cloud.integration_proxy.{failure_type}",
+                    extra={"failure_type": error_mapping.failure_type},
+                )
+                self._add_failure_metric(error_mapping.failure_type)
+                return self.respond(context={"detail": str(exc)}, status=error_mapping.status_code)
 
         self._add_failure_metric(IntegrationProxyFailureMetricType.UNKNOWN_ERROR)
         return super().handle_exception_with_details(request, exc, handler_context, scope)

tests/sentry/integrations/api/endpoints/test_integration_proxy.py

@@ -21,7 +21,7 @@
 from sentry.integrations.types import EventLifecycleOutcome
 from sentry.metrics.base import Tags
 from sentry.shared_integrations.client.proxy import IntegrationProxyClient
-from sentry.shared_integrations.exceptions import ApiHostError, ApiTimeoutError
+from sentry.shared_integrations.exceptions import ApiHostError, ApiTimeoutError, ApiUnauthorized
 from sentry.silo.base import SiloMode
 from sentry.silo.util import (
     PROXY_BASE_PATH,
@@ -413,7 +413,7 @@ def test_successful_response(self, mock_metrics, mock_get_installation):
         mock_metrics.assert_not_called()
 
     def raise_exception(self, exc_type: type[Exception], *args, **kwargs):
-        raise exc_type(*args)
+        raise exc_type(*args, **kwargs)
 
     @override_settings(SENTRY_SUBNET_SECRET=SENTRY_SUBNET_SECRET, SILO_MODE=SiloMode.CONTROL)
     @patch.object(ExampleIntegration, "get_client")
@@ -435,7 +435,7 @@ def test_handles_identity_not_valid(
 
         proxy_response = self.client.get(self.path, **headers)
 
-        assert proxy_response.status_code == 400
+        assert proxy_response.status_code == 401
         assert proxy_response.data is None
 
         self.assert_metric_count(
@@ -455,6 +455,48 @@ def test_handles_identity_not_valid(
             mock_metrics=mock_metrics,
         )
 
+    @override_settings(SENTRY_SUBNET_SECRET=SENTRY_SUBNET_SECRET, SILO_MODE=SiloMode.CONTROL)
+    @patch.object(ExampleIntegration, "get_client")
+    @patch.object(InternalIntegrationProxyEndpoint, "client", spec=IntegrationProxyClient)
+    @patch.object(metrics, "incr")
+    def test_handles_api_unauthorized(
+        self, mock_metrics: MagicMock, mock_client: MagicMock, mock_get_client: MagicMock
+    ) -> None:
+        signature_path = f"/{self.proxy_path}"
+        headers = self.create_request_headers(
+            signature_path=signature_path, integration_id=self.org_integration.id
+        )
+        mock_client.base_url = "https://example.com/api"
+        mock_client.authorize_request = MagicMock(side_effect=lambda req: req)
+        mock_client.request = MagicMock(
+            side_effect=lambda *args, **kwargs: self.raise_exception(
+                exc_type=ApiUnauthorized, text="hah, noooo"
+            )
+        )
+        mock_get_client.return_value = mock_client
+
+        proxy_response = self.client.get(self.path, **headers)
+
+        assert proxy_response.status_code == 401
+        assert proxy_response.data is None
+
+        self.assert_metric_count(
+            metric_name=IntegrationProxySuccessMetricType.INITIALIZE,
+            count=1,
+            mock_metrics=mock_metrics,
+            kwargs_to_match={"sample_rate": 1.0, "tags": None},
+        )
+        self.assert_failure_metric_count(
+            failure_type=IntegrationProxyFailureMetricType.UNAUTHORIZED,
+            count=1,
+            mock_metrics=mock_metrics,
+        )
+        self.assert_metric_count(
+            metric_name=IntegrationProxySuccessMetricType.COMPLETE_RESPONSE_CODE,
+            count=0,
+            mock_metrics=mock_metrics,
+        )
+
     @override_settings(SENTRY_SUBNET_SECRET=SENTRY_SUBNET_SECRET, SILO_MODE=SiloMode.CONTROL)
     @patch.object(ExampleIntegration, "get_client")
     @patch.object(InternalIntegrationProxyEndpoint, "client", spec=IntegrationProxyClient)