diff --git a/src/sentry/sentry_apps/api/endpoints/installation_external_requests.py b/src/sentry/sentry_apps/api/endpoints/installation_external_requests.py index 0565b1b592aec9..d7b0d19592303a 100644 --- a/src/sentry/sentry_apps/api/endpoints/installation_external_requests.py +++ b/src/sentry/sentry_apps/api/endpoints/installation_external_requests.py @@ -1,13 +1,19 @@ +import logging + +from jsonschema import ValidationError from rest_framework.request import Request from rest_framework.response import Response from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import region_silo_endpoint +from sentry.coreapi import APIError from sentry.models.project import Project from sentry.sentry_apps.api.bases.sentryapps import SentryAppInstallationBaseEndpoint from sentry.sentry_apps.external_requests.select_requester import SelectRequester +logger = logging.getLogger("sentry.sentry-apps") + @region_silo_endpoint class SentryAppInstallationExternalRequestsEndpoint(SentryAppInstallationBaseEndpoint): @@ -36,7 +42,16 @@ def get(self, request: Request, installation) -> Response: try: choices = SelectRequester(**kwargs).run() - except Exception: - return Response({"error": "Error communicating with Sentry App service"}, status=400) + except ValidationError as e: + return Response( + {"error": e.message}, + status=400, + ) + except APIError: + message = f'Error retrieving select field options from {request.GET.get("uri")}' + return Response( + {"error": message}, + status=400, + ) return Response(choices) diff --git a/src/sentry/sentry_apps/external_requests/select_requester.py b/src/sentry/sentry_apps/external_requests/select_requester.py index 14f9c6ba6b8634..67f11b3b16c72e 100644 --- a/src/sentry/sentry_apps/external_requests/select_requester.py +++ b/src/sentry/sentry_apps/external_requests/select_requester.py @@ -5,6 +5,7 @@ from uuid import uuid4 from django.utils.functional import cached_property +from jsonschema import ValidationError from sentry.coreapi import APIError from sentry.http import safe_urlread @@ -36,9 +37,10 @@ class SelectRequester: def run(self) -> dict[str, Any]: response: list[dict[str, str]] = [] try: + url = self._build_url() body = safe_urlread( send_and_save_sentry_app_request( - self._build_url(), + url, self.sentry_app, self.install.organization_id, "select_options.requested", @@ -54,13 +56,24 @@ def run(self) -> dict[str, Any]: "sentry_app_slug": self.sentry_app.slug, "install_uuid": self.install.uuid, "project_slug": self.project_slug, - "uri": self.uri, "error_message": str(e), + "url": url, }, ) + raise APIError from e - if not self._validate_response(response) or not response: - raise APIError( + if not self._validate_response(response): + logger.info( + "select-requester.invalid-response", + extra={ + "response": response, + "sentry_app_slug": self.sentry_app.slug, + "install_uuid": self.install.uuid, + "project_slug": self.project_slug, + "url": url, + }, + ) + raise ValidationError( f"Invalid response format for SelectField in {self.sentry_app} from uri: {self.uri}" ) return self._format_response(response) @@ -95,7 +108,14 @@ def _format_response(self, resp: list[dict[str, Any]]) -> dict[str, Any]: for option in resp: if not ("value" in option and "label" in option): - raise APIError("Missing `value` or `label` in option data for SelectField") + logger.info( + "select-requester.invalid-response", + extra={ + "resposnse": resp, + "error_msg": "Missing `value` or `label` in option data for SelectField", + }, + ) + raise ValidationError("Missing `value` or `label` in option data for SelectField") choices.append([option["value"], option["label"]]) diff --git a/src/sentry/sentry_apps/models/sentry_app_installation.py b/src/sentry/sentry_apps/models/sentry_app_installation.py index 55b126bd2e7689..c0eb232c0b6f99 100644 --- a/src/sentry/sentry_apps/models/sentry_app_installation.py +++ b/src/sentry/sentry_apps/models/sentry_app_installation.py @@ -7,6 +7,7 @@ from django.db import models from django.db.models import QuerySet from django.utils import timezone +from jsonschema import ValidationError from sentry.auth.services.auth import AuthenticatedToken from sentry.backup.scopes import RelocationScope @@ -241,6 +242,6 @@ def prepare_ui_component( component=component, install=installation, project_slug=project_slug, values=values ).run() return component - except APIError: + except (APIError, ValidationError): # TODO(nisanthan): For now, skip showing the UI Component if the API requests fail return None diff --git a/tests/sentry/sentry_apps/external_requests/test_select_requester.py b/tests/sentry/sentry_apps/external_requests/test_select_requester.py index f620f0a2b16218..63a726515e5b58 100644 --- a/tests/sentry/sentry_apps/external_requests/test_select_requester.py +++ b/tests/sentry/sentry_apps/external_requests/test_select_requester.py @@ -1,5 +1,6 @@ import pytest import responses +from jsonschema import ValidationError from sentry.coreapi import APIError from sentry.sentry_apps.external_requests.select_requester import SelectRequester @@ -69,7 +70,7 @@ def test_invalid_response_missing_label(self): content_type="application/json", ) - with pytest.raises(APIError): + with pytest.raises(ValidationError): SelectRequester( install=self.install, project_slug=self.project.slug, @@ -90,7 +91,7 @@ def test_invalid_response_missing_value(self): content_type="application/json", ) - with pytest.raises(APIError): + with pytest.raises(ValidationError): SelectRequester( install=self.install, project_slug=self.project.slug,