Browse Source

feat(sentry apps): Update sentry app components to return errors (#84187)

Christinarlong 1 day ago
parent
commit
e9a93788cc

+ 3 - 10
src/sentry/sentry_apps/api/bases/sentryapps.py

@@ -5,7 +5,6 @@ from collections.abc import Sequence
 from functools import wraps
 from typing import Any
 
-import sentry_sdk
 from rest_framework.permissions import BasePermission
 from rest_framework.request import Request
 from rest_framework.response import Response
@@ -113,22 +112,16 @@ class IntegrationPlatformEndpoint(Endpoint):
         ) or super().handle_exception_with_details(request, exc, handler_context, scope)
 
     def _handle_sentry_app_exception(self, exception: Exception):
-        if isinstance(exception, SentryAppIntegratorError) or isinstance(exception, SentryAppError):
-            response_body: dict[str, Any] = {"detail": exception.message}
-
-            if public_context := exception.public_context:
-                response_body.update({"context": public_context})
+        if isinstance(exception, (SentryAppError, SentryAppIntegratorError)):
+            response_body = exception.to_public_dict()
 
             response = Response(response_body, status=exception.status_code)
             response.exception = True
             return response
 
         elif isinstance(exception, SentryAppSentryError):
-            error_id = sentry_sdk.capture_exception(exception)
             return Response(
-                {
-                    "detail": f"An issue occured during the integration platform process. Sentry error ID: {error_id}"
-                },
+                {exception.to_public_dict()},
                 status=500,
             )
         # If not an audited sentry app error then default to using default error handler

+ 12 - 12
src/sentry/sentry_apps/api/endpoints/sentry_app_components.py

@@ -19,11 +19,7 @@ from sentry.sentry_apps.api.serializers.sentry_app_component import SentryAppCom
 from sentry.sentry_apps.components import SentryAppComponentPreparer
 from sentry.sentry_apps.models.sentry_app_component import SentryAppComponent
 from sentry.sentry_apps.models.sentry_app_installation import SentryAppInstallation
-from sentry.sentry_apps.utils.errors import (
-    SentryAppError,
-    SentryAppIntegratorError,
-    SentryAppSentryError,
-)
+from sentry.sentry_apps.utils.errors import SentryAppError, SentryAppIntegratorError
 
 logger = logging.getLogger("sentry.sentry_apps.components")
 
@@ -44,7 +40,7 @@ class SentryAppComponentsEndpoint(SentryAppBaseEndpoint):
             queryset=sentry_app.components.all(),
             paginator_cls=OffsetPaginator,
             on_results=lambda x: serialize(
-                x, request.user, errors=[], serializer=SentryAppComponentSerializer()
+                x, request.user, errors={}, serializer=SentryAppComponentSerializer()
             ),
         )
 
@@ -63,7 +59,7 @@ class OrganizationSentryAppComponentsEndpoint(ControlSiloOrganizationEndpoint):
         organization: RpcOrganization,
     ) -> Response:
         components = []
-        errors = []
+        errors = {}
 
         with sentry_sdk.start_transaction(name="sentry.api.sentry_app_components.get"):
             with sentry_sdk.start_span(op="sentry-app-components.get_installs"):
@@ -84,9 +80,12 @@ class OrganizationSentryAppComponentsEndpoint(ControlSiloOrganizationEndpoint):
                     with sentry_sdk.start_span(op="sentry-app-components.prepare_components"):
                         try:
                             SentryAppComponentPreparer(component=component, install=install).run()
-                        except (SentryAppIntegratorError, SentryAppError):
-                            errors.append(str(component.uuid))
-                        except (SentryAppSentryError, Exception) as e:
+
+                        except (SentryAppIntegratorError, SentryAppError) as e:
+                            errors[str(component.uuid)] = e.to_public_dict()
+
+                        except Exception as e:
+                            error_id = sentry_sdk.capture_exception(e)
                             logger.info(
                                 "component-preparation-error",
                                 exc_info=e,
@@ -96,10 +95,11 @@ class OrganizationSentryAppComponentsEndpoint(ControlSiloOrganizationEndpoint):
                                     "installation_uuid": install.uuid,
                                 },
                             )
-                            errors.append(str(component.uuid))
+                            errors[str(component.uuid)] = {
+                                "detail": f"Something went wrong while trying to link issue for component: {str(component.uuid)}. Sentry error ID: {error_id}"
+                            }
 
                         components.append(component)
-
         return self.paginate(
             request=request,
             queryset=components,

+ 1 - 1
src/sentry/sentry_apps/api/serializers/sentry_app_component.py

@@ -14,7 +14,7 @@ class SentryAppComponentSerializer(Serializer):
             "uuid": str(obj.uuid),
             "type": obj.type,
             "schema": obj.schema,
-            "error": True if str(obj.uuid) in errors else False,
+            "error": errors.get(str(obj.uuid), ""),
             "sentryApp": {
                 "uuid": obj.sentry_app.uuid,
                 "slug": obj.sentry_app.slug,

+ 19 - 1
src/sentry/sentry_apps/utils/errors.py

@@ -1,5 +1,5 @@
 from enum import Enum
-from typing import Any
+from typing import Any, TypedDict
 
 import sentry_sdk
 from rest_framework.response import Response
@@ -11,6 +11,11 @@ class SentryAppErrorType(Enum):
     SENTRY = "sentry"
 
 
+class SentryAppPublicErrorBody(TypedDict, total=False):
+    detail: str
+    context: dict[str, Any]
+
+
 class SentryAppBaseError(Exception):
     error_type: SentryAppErrorType
     status_code: int
@@ -29,6 +34,13 @@ class SentryAppBaseError(Exception):
         self.webhook_context = webhook_context or {}
         self.message = message
 
+    def to_public_dict(self) -> SentryAppPublicErrorBody:
+        error_body: SentryAppPublicErrorBody = {"detail": self.message}
+        if public_context := self.public_context:
+            error_body.update({"context": public_context})
+
+        return error_body
+
     def response_from_exception(self) -> Response:
         response: dict[str, Any] = {"detail": self.message}
         if public_context := self.public_context:
@@ -53,6 +65,12 @@ class SentryAppSentryError(SentryAppBaseError):
     error_type = SentryAppErrorType.SENTRY
     status_code = 500
 
+    def to_public_dict(self) -> SentryAppPublicErrorBody:
+        error_id = sentry_sdk.capture_exception(self)
+        return {
+            "detail": f"An issue occured during the integration platform process. Sentry error ID: {error_id}"
+        }
+
     def response_from_exception(self) -> Response:
         sentry_sdk.capture_exception(self)
         response: dict[str, Any] = {

+ 198 - 10
tests/sentry/sentry_apps/api/endpoints/test_sentry_app_components.py

@@ -1,9 +1,13 @@
+from collections.abc import Sequence
 from unittest.mock import patch
 
+import responses
+
 from sentry.api.serializers.base import serialize
 from sentry.constants import SentryAppInstallationStatus
 from sentry.coreapi import APIError
 from sentry.sentry_apps.models.sentry_app import SentryApp
+from sentry.sentry_apps.models.sentry_app_component import SentryAppComponent
 from sentry.sentry_apps.utils.errors import SentryAppIntegratorError, SentryAppSentryError
 from sentry.testutils.cases import APITestCase
 from sentry.testutils.silo import control_silo_test
@@ -40,7 +44,7 @@ class SentryAppComponentsTest(APITestCase):
             "uuid": str(self.component.uuid),
             "type": "issue-link",
             "schema": self.component.schema,
-            "error": False,
+            "error": "",
             "sentryApp": {
                 "uuid": self.sentry_app.uuid,
                 "slug": self.sentry_app.slug,
@@ -104,7 +108,7 @@ class OrganizationSentryAppComponentsTest(APITestCase):
             "uuid": str(self.component1.uuid),
             "type": "issue-link",
             "schema": self.component1.schema,
-            "error": False,
+            "error": "",
             "sentryApp": {
                 "uuid": self.sentry_app1.uuid,
                 "slug": self.sentry_app1.slug,
@@ -117,7 +121,7 @@ class OrganizationSentryAppComponentsTest(APITestCase):
             "uuid": str(self.component2.uuid),
             "type": "issue-link",
             "schema": self.component2.schema,
-            "error": False,
+            "error": "",
             "sentryApp": {
                 "uuid": self.sentry_app2.uuid,
                 "slug": self.sentry_app2.slug,
@@ -143,7 +147,7 @@ class OrganizationSentryAppComponentsTest(APITestCase):
                 "uuid": str(component.uuid),
                 "type": "alert-rule",
                 "schema": component.schema,
-                "error": False,
+                "error": "",
                 "sentryApp": {
                     "uuid": sentry_app.uuid,
                     "slug": sentry_app.slug,
@@ -161,7 +165,10 @@ class OrganizationSentryAppComponentsTest(APITestCase):
 
     @patch("sentry.sentry_apps.components.SentryAppComponentPreparer.run")
     def test_component_prep_errors_are_isolated(self, run):
-        run.side_effect = [SentryAppIntegratorError(message="whoops"), self.component2]
+        run.side_effect = [
+            SentryAppIntegratorError(message="zoinks!", public_context={"foo": "bar"}),
+            self.component2,
+        ]
 
         response = self.get_success_response(
             self.org.slug, qs_params={"projectId": self.project.id}
@@ -174,7 +181,7 @@ class OrganizationSentryAppComponentsTest(APITestCase):
                 "uuid": str(self.component1.uuid),
                 "type": self.component1.type,
                 "schema": self.component1.schema,
-                "error": True,
+                "error": {"detail": "zoinks!", "context": {"foo": "bar"}},
                 "sentryApp": {
                     "uuid": self.sentry_app1.uuid,
                     "slug": self.sentry_app1.slug,
@@ -186,7 +193,7 @@ class OrganizationSentryAppComponentsTest(APITestCase):
                 "uuid": str(self.component2.uuid),
                 "type": self.component2.type,
                 "schema": self.component2.schema,
-                "error": False,
+                "error": "",
                 "sentryApp": {
                     "uuid": self.sentry_app2.uuid,
                     "slug": self.sentry_app2.slug,
@@ -198,9 +205,169 @@ class OrganizationSentryAppComponentsTest(APITestCase):
 
         assert response.data == expected
 
+    @responses.activate
+    def test_component_prep_api_error(self):
+        responses.add(
+            method=responses.GET,
+            url="https://example.com/",
+            json={"error": "the dumpsters on fire!!!"},
+            status=500,
+            content_type="application/json",
+        )
+
+        responses.add(
+            method=responses.GET,
+            url="https://example.com/",
+            json={"error": "couldnt find the dumpsters :C"},
+            status=404,
+            content_type="application/json",
+        )
+
+        response = self.get_success_response(
+            self.org.slug, qs_params={"projectId": self.project.id}
+        )
+        expected = [
+            {
+                "uuid": str(self.component1.uuid),
+                "type": self.component1.type,
+                "schema": self.component1.schema,
+                "error": {
+                    "detail": f"Something went wrong while getting options for Select FormField from {self.sentry_app1.slug}"
+                },
+                "sentryApp": {
+                    "uuid": self.sentry_app1.uuid,
+                    "slug": self.sentry_app1.slug,
+                    "name": self.sentry_app1.name,
+                    "avatars": get_sentry_app_avatars(self.sentry_app1),
+                },
+            },
+            {
+                "uuid": str(self.component2.uuid),
+                "type": self.component2.type,
+                "schema": self.component2.schema,
+                "error": {
+                    "detail": f"Something went wrong while getting options for Select FormField from {self.sentry_app2.slug}"
+                },
+                "sentryApp": {
+                    "uuid": self.sentry_app2.uuid,
+                    "slug": self.sentry_app2.slug,
+                    "name": self.sentry_app2.name,
+                    "avatars": get_sentry_app_avatars(self.sentry_app2),
+                },
+            },
+        ]
+
+        assert response.data == expected
+
+    @responses.activate
+    def test_component_prep_validation_error(self):
+        component1_uris = self._get_component_uris(
+            component_field="link", component=self.component1
+        )
+
+        component2_uris = self._get_component_uris(
+            component_field="link", component=self.component2
+        )
+
+        # We only get the first uri since the SentryAppComponentPreparer will short circuit after getting the first error
+        responses.add(
+            method=responses.GET,
+            url=f"https://example.com{component1_uris[0]}?installationId={self.install1.uuid}",
+            json=[{"bruh": "the dumpsters on fire!!!"}],
+            status=200,
+            content_type="application/json",
+        )
+
+        responses.add(
+            method=responses.GET,
+            url=f"https://example.com{component2_uris[0]}?installationId={self.install2.uuid}",
+            json={},
+            status=200,
+            content_type="application/json",
+        )
+
+        response = self.get_success_response(
+            self.org.slug, qs_params={"projectId": self.project.id}
+        )
+        expected = [
+            {
+                "uuid": str(self.component1.uuid),
+                "type": self.component1.type,
+                "schema": self.component1.schema,
+                "error": {
+                    "detail": "Missing `value` or `label` in option data for Select FormField"
+                },
+                "sentryApp": {
+                    "uuid": self.sentry_app1.uuid,
+                    "slug": self.sentry_app1.slug,
+                    "name": self.sentry_app1.name,
+                    "avatars": get_sentry_app_avatars(self.sentry_app1),
+                },
+            },
+            {
+                "uuid": str(self.component2.uuid),
+                "type": self.component2.type,
+                "schema": self.component2.schema,
+                "error": {
+                    "detail": f"Invalid response format for Select FormField in {self.sentry_app2.slug} from uri: {component2_uris[0]}"
+                },
+                "sentryApp": {
+                    "uuid": self.sentry_app2.uuid,
+                    "slug": self.sentry_app2.slug,
+                    "name": self.sentry_app2.name,
+                    "avatars": get_sentry_app_avatars(self.sentry_app2),
+                },
+            },
+        ]
+
+        assert response.data == expected
+
+    @patch("sentry_sdk.capture_exception")
+    @patch("sentry.sentry_apps.components.SentryAppComponentPreparer.run")
+    def test_component_prep_general_error(self, run, capture_exception):
+        run.side_effect = [Exception(":dead:"), SentryAppSentryError("government secrets here")]
+        capture_exception.return_value = 1
+        response = self.get_success_response(
+            self.org.slug, qs_params={"projectId": self.project.id}
+        )
+        expected = [
+            {
+                "uuid": str(self.component1.uuid),
+                "type": self.component1.type,
+                "schema": self.component1.schema,
+                "error": {
+                    "detail": f"Something went wrong while trying to link issue for component: {str(self.component1.uuid)}. Sentry error ID: {capture_exception.return_value}"
+                },
+                "sentryApp": {
+                    "uuid": self.sentry_app1.uuid,
+                    "slug": self.sentry_app1.slug,
+                    "name": self.sentry_app1.name,
+                    "avatars": get_sentry_app_avatars(self.sentry_app1),
+                },
+            },
+            {
+                "uuid": str(self.component2.uuid),
+                "type": self.component2.type,
+                "schema": self.component2.schema,
+                "error": {
+                    "detail": f"Something went wrong while trying to link issue for component: {str(self.component2.uuid)}. Sentry error ID: {capture_exception.return_value}"
+                },
+                "sentryApp": {
+                    "uuid": self.sentry_app2.uuid,
+                    "slug": self.sentry_app2.slug,
+                    "name": self.sentry_app2.name,
+                    "avatars": get_sentry_app_avatars(self.sentry_app2),
+                },
+            },
+        ]
+
+        assert response.data == expected
+
+    @patch("sentry_sdk.capture_exception")
     @patch("sentry.sentry_apps.components.SentryAppComponentPreparer.run")
-    def test_component_prep_errors_dont_bring_down_everything(self, run):
+    def test_component_prep_errors_dont_bring_down_everything(self, run, capture_exception):
         run.side_effect = [APIError(), SentryAppSentryError(message="kewl")]
+        capture_exception.return_value = 1
 
         response = self.get_success_response(
             self.org.slug, qs_params={"projectId": self.project.id}
@@ -213,7 +380,9 @@ class OrganizationSentryAppComponentsTest(APITestCase):
                 "uuid": str(self.component1.uuid),
                 "type": self.component1.type,
                 "schema": self.component1.schema,
-                "error": True,
+                "error": {
+                    "detail": f"Something went wrong while trying to link issue for component: {self.component1.uuid}. Sentry error ID: {capture_exception.return_value}"
+                },
                 "sentryApp": {
                     "uuid": self.sentry_app1.uuid,
                     "slug": self.sentry_app1.slug,
@@ -225,7 +394,9 @@ class OrganizationSentryAppComponentsTest(APITestCase):
                 "uuid": str(self.component2.uuid),
                 "type": self.component2.type,
                 "schema": self.component2.schema,
-                "error": True,
+                "error": {
+                    "detail": f"Something went wrong while trying to link issue for component: {self.component2.uuid}. Sentry error ID: {capture_exception.return_value}"
+                },
                 "sentryApp": {
                     "uuid": self.sentry_app2.uuid,
                     "slug": self.sentry_app2.slug,
@@ -236,3 +407,20 @@ class OrganizationSentryAppComponentsTest(APITestCase):
         ]
 
         assert response.data == expected
+
+    def _get_component_uris(
+        self, component_field: str, component: SentryAppComponent
+    ) -> Sequence[str]:
+        fields = dict(**component.app_schema).get(component_field)
+        assert fields, "component field was not found in the schema"
+        uris = []
+
+        for field in fields.get("required_fields", []):
+            if "uri" in field:
+                uris.append(field.get("uri"))
+
+        for field in fields.get("optional_fields", []):
+            if "uri" in field:
+                uris.append(field.get("uri"))
+
+        return uris

+ 0 - 0
tests/sentry/sentry_apps/utils/__init__.py


+ 25 - 0
tests/sentry/sentry_apps/utils/test_errors.py

@@ -0,0 +1,25 @@
+from sentry.sentry_apps.utils.errors import SentryAppError, SentryAppIntegratorError
+from sentry.testutils.cases import TestCase
+
+
+class TestSentryAppBaseError(TestCase):
+
+    def test_to_public_dict(self):
+        error = SentryAppError(message="brooo", public_context={"omg": "omgggg"})
+        body = error.to_public_dict()
+
+        assert body == {"detail": "brooo", "context": {"omg": "omgggg"}}
+
+    def test_to_public_dict_no_webhook_context(self):
+        error = SentryAppIntegratorError(
+            message="brooo", public_context={"omg": "omgggg"}, webhook_context={"bruh": "bruhh"}
+        )
+        body = error.to_public_dict()
+
+        assert body == {"detail": "brooo", "context": {"omg": "omgggg"}}
+
+    def test_to_public_dict_no_context(self):
+        error = SentryAppIntegratorError(message="brooo")
+        body = error.to_public_dict()
+
+        assert body == {"detail": "brooo"}