diff --git a/src/sentry/sentry_apps/api/endpoints/installation_external_issue_actions.py b/src/sentry/sentry_apps/api/endpoints/installation_external_issue_actions.py index 13ae408d361ae1..67c5fdaeb96d12 100644 --- a/src/sentry/sentry_apps/api/endpoints/installation_external_issue_actions.py +++ b/src/sentry/sentry_apps/api/endpoints/installation_external_issue_actions.py @@ -1,14 +1,12 @@ from django.utils.functional import empty -from jsonschema import ValidationError +from rest_framework import serializers from rest_framework.request import Request from rest_framework.response import Response -from sentry_sdk import capture_exception 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.api.serializers import serialize -from sentry.coreapi import APIError, APIUnauthorized from sentry.models.group import Group from sentry.models.project import Project from sentry.sentry_apps.api.bases.sentryapps import SentryAppInstallationBaseEndpoint @@ -16,6 +14,7 @@ PlatformExternalIssueSerializer, ) from sentry.sentry_apps.external_issues.issue_link_creator import IssueLinkCreator +from sentry.sentry_apps.utils.errors import SentryAppError from sentry.users.models.user import User from sentry.users.services.user.serial import serialize_rpc_user @@ -34,6 +33,12 @@ def _extract_lazy_object(lo): return lo._wrapped +class SentryAppInstallationExternalIssueActionsSerializer(serializers.Serializer): + groupId = serializers.CharField(required=True, allow_null=False) + action = serializers.CharField(required=True, allow_null=False) + uri = serializers.CharField(required=True, allow_null=False) + + @region_silo_endpoint class SentryAppInstallationExternalIssueActionsEndpoint(SentryAppInstallationBaseEndpoint): owner = ApiOwner.INTEGRATIONS @@ -44,8 +49,12 @@ class SentryAppInstallationExternalIssueActionsEndpoint(SentryAppInstallationBas def post(self, request: Request, installation) -> Response: data = request.data.copy() - if not {"groupId", "action", "uri"}.issubset(data.keys()): - return Response(status=400) + external_issue_action_serializer = SentryAppInstallationExternalIssueActionsSerializer( + data=data + ) + + if not external_issue_action_serializer.is_valid(): + return Response(external_issue_action_serializer.errors, status=400) group_id = data.get("groupId") del data["groupId"] @@ -56,34 +65,26 @@ def post(self, request: Request, installation) -> Response: project_id__in=Project.objects.filter(organization_id=installation.organization_id), ) except Group.DoesNotExist: - return Response(status=404) + raise SentryAppError( + message="Could not find the corresponding issue for the given groupId", + status_code=404, + ) action = data.pop("action") uri = data.pop("uri") - try: - user = _extract_lazy_object(request.user) - if isinstance(user, User): - user = serialize_rpc_user(user) - - external_issue = IssueLinkCreator( - install=installation, - group=group, - action=action, - fields=data, - uri=uri, - user=user, - ).run() - except (APIError, ValidationError, APIUnauthorized) as e: - return Response({"error": str(e)}, status=400) - except Exception as e: - error_id = capture_exception(e) - return Response( - { - "error": f"Something went wrong while trying to link issue. Sentry error ID: {error_id}" - }, - status=500, - ) + user = _extract_lazy_object(request.user) + if isinstance(user, User): + user = serialize_rpc_user(user) + + external_issue = IssueLinkCreator( + install=installation, + group=group, + action=action, + fields=data, + uri=uri, + user=user, + ).run() return Response( serialize(objects=external_issue, serializer=PlatformExternalIssueSerializer()) diff --git a/src/sentry/sentry_apps/api/endpoints/installation_external_issue_details.py b/src/sentry/sentry_apps/api/endpoints/installation_external_issue_details.py index 3bbd636b84623d..702e709ffa9427 100644 --- a/src/sentry/sentry_apps/api/endpoints/installation_external_issue_details.py +++ b/src/sentry/sentry_apps/api/endpoints/installation_external_issue_details.py @@ -8,6 +8,7 @@ SentryAppInstallationExternalIssueBaseEndpoint as ExternalIssueBaseEndpoint, ) from sentry.sentry_apps.models.platformexternalissue import PlatformExternalIssue +from sentry.sentry_apps.utils.errors import SentryAppError @region_silo_endpoint @@ -24,7 +25,10 @@ def delete(self, request: Request, installation, external_issue_id) -> Response: service_type=installation.sentry_app.slug, ) except PlatformExternalIssue.DoesNotExist: - return Response(status=404) + raise SentryAppError( + message="Could not find the corresponding external issue from given external_issue_id", + status_code=404, + ) deletions.exec_sync(platform_external_issue) diff --git a/src/sentry/sentry_apps/api/endpoints/installation_external_issues.py b/src/sentry/sentry_apps/api/endpoints/installation_external_issues.py index 8f4f6073b761b9..6aef63a5562bc1 100644 --- a/src/sentry/sentry_apps/api/endpoints/installation_external_issues.py +++ b/src/sentry/sentry_apps/api/endpoints/installation_external_issues.py @@ -16,6 +16,7 @@ PlatformExternalIssueSerializer as ResponsePlatformExternalIssueSerializer, ) from sentry.sentry_apps.external_issues.external_issue_creator import ExternalIssueCreator +from sentry.sentry_apps.utils.errors import SentryAppError class PlatformExternalIssueSerializer(serializers.Serializer): @@ -40,7 +41,10 @@ def post(self, request: Request, installation) -> Response: project_id__in=Project.objects.filter(organization_id=installation.organization_id), ) except Group.DoesNotExist: - return Response(status=404) + raise SentryAppError( + message="Could not find the corresponding issue for the given issueId", + status_code=404, + ) serializer = PlatformExternalIssueSerializer(data=request.data) if serializer.is_valid(): 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 d7b0d19592303a..3842e3c39c817e 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,11 @@ 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 @@ -40,18 +38,6 @@ def get(self, request: Request, installation) -> Response: if project: kwargs.update({"project_slug": project.slug}) - try: - choices = SelectRequester(**kwargs).run() - 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, - ) + choices = SelectRequester(**kwargs).run() return Response(choices) diff --git a/src/sentry/sentry_apps/external_issues/external_issue_creator.py b/src/sentry/sentry_apps/external_issues/external_issue_creator.py index 428a2fc92246c4..797134093b2eb2 100644 --- a/src/sentry/sentry_apps/external_issues/external_issue_creator.py +++ b/src/sentry/sentry_apps/external_issues/external_issue_creator.py @@ -38,15 +38,14 @@ def run(self) -> PlatformExternalIssue: return self.external_issue[0] except Exception as e: logger.info( - "create-failed", + "platform-external-issue.create-failed", + exc_info=e, extra={ - "error": str(e), - "installtion_id": self.install.uuid, + "installation_id": self.install.uuid, "group_id": self.group.id, "sentry_app_slug": self.install.sentry_app.slug, }, ) raise SentryAppSentryError( message="Failed to create external issue obj", - webhook_context={"error": str(e)}, ) from e diff --git a/src/sentry/sentry_apps/external_requests/issue_link_requester.py b/src/sentry/sentry_apps/external_requests/issue_link_requester.py index 50279b4d79f8ff..659d554709faf9 100644 --- a/src/sentry/sentry_apps/external_requests/issue_link_requester.py +++ b/src/sentry/sentry_apps/external_requests/issue_link_requester.py @@ -5,13 +5,13 @@ from uuid import uuid4 from django.utils.functional import cached_property -from jsonschema import ValidationError +from requests import RequestException -from sentry.coreapi import APIError from sentry.http import safe_urlread from sentry.models.group import Group from sentry.sentry_apps.external_requests.utils import send_and_save_sentry_app_request, validate from sentry.sentry_apps.services.app import RpcSentryAppInstallation +from sentry.sentry_apps.utils.errors import SentryAppIntegratorError from sentry.users.models.user import User from sentry.users.services.user import RpcUser from sentry.utils import json @@ -74,26 +74,43 @@ def run(self) -> dict[str, Any]: body = safe_urlread(request) response = json.loads(body) except (json.JSONDecodeError, TypeError): - raise ValidationError(f"Unable to parse response from {self.sentry_app.slug}") - except Exception as e: - logger.info( - "issue-link-requester.error", - extra={ - "sentry_app": self.sentry_app.slug, - "install": self.install.uuid, - "project": self.group.project.slug, - "group": self.group.id, + raise SentryAppIntegratorError( + message=f"Unable to parse response from {self.sentry_app.slug}", + webhook_context={ + "error_type": "issue-link-requester.invalid-json", "uri": self.uri, - "error_message": str(e), + "response": body, + "installation_uuid": self.install.uuid, }, + status_code=500, ) - raise APIError( - f"Issue occured while trying to contact {self.sentry_app.slug} to link issue" + except RequestException as e: + error_type = "issue-link-requester.error" + extras = { + "sentry_app": self.sentry_app.slug, + "installation_uuid": self.install.uuid, + "project": self.group.project.slug, + "group": self.group.id, + "uri": self.uri, + "error_message": str(e), + } + logger.info(error_type, extra=extras) + raise SentryAppIntegratorError( + message=f"Issue occured while trying to contact {self.sentry_app.slug} to link issue", + webhook_context={"error_type": error_type, **extras}, + status_code=500, ) if not self._validate_response(response): - raise ValidationError( - f"Invalid response format from sentry app {self.sentry_app} when linking issue" + raise SentryAppIntegratorError( + message=f"Invalid response format from sentry app {self.sentry_app} when linking issue", + webhook_context={ + "error_type": "issue-link-requester.invalid-response", + "response": response, + "installation_uuid": self.install.uuid, + "uri": self.uri, + }, + status_code=500, ) return response diff --git a/src/sentry/sentry_apps/external_requests/select_requester.py b/src/sentry/sentry_apps/external_requests/select_requester.py index 9df1240653f150..9cb0ff5d0275c4 100644 --- a/src/sentry/sentry_apps/external_requests/select_requester.py +++ b/src/sentry/sentry_apps/external_requests/select_requester.py @@ -6,13 +6,12 @@ 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 from sentry.sentry_apps.external_requests.utils import send_and_save_sentry_app_request, validate from sentry.sentry_apps.services.app import RpcSentryAppInstallation from sentry.sentry_apps.services.app.model import RpcSentryApp +from sentry.sentry_apps.utils.errors import SentryAppIntegratorError from sentry.utils import json logger = logging.getLogger("sentry.sentry_apps.external_requests") @@ -61,7 +60,6 @@ def run(self) -> SelectRequesterResult: "sentry_app_slug": self.sentry_app.slug, "install_uuid": self.install.uuid, "project_slug": self.project_slug, - "error_message": str(e), } if not url: @@ -77,25 +75,30 @@ def run(self) -> SelectRequesterResult: extra.update({"url": url}) message = "select-requester.request-failed" - logger.info(message, extra=extra) - raise APIError( - f"Something went wrong while getting SelectFields from {self.sentry_app.slug}" + logger.info(message, exc_info=e, extra=extra) + raise SentryAppIntegratorError( + message=f"Something went wrong while getting options for Select FormField from {self.sentry_app.slug}", + webhook_context={"error_type": message, **extra}, + status_code=500, ) from e 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, + extras = { + "response": response, + "sentry_app_slug": self.sentry_app.slug, + "install_uuid": self.install.uuid, + "project_slug": self.project_slug, + "url": url, + } + logger.info("select-requester.invalid-response", extra=extras) + + raise SentryAppIntegratorError( + message=f"Invalid response format for Select FormField in {self.sentry_app.slug} from uri: {self.uri}", + webhook_context={ + "error_type": "select-requester.invalid-integrator-response", + **extras, }, ) - raise ValidationError( - f"Invalid response format for SelectField in {self.sentry_app.slug} from uri: {self.uri}" - ) return self._format_response(response) def _build_url(self) -> str: @@ -130,14 +133,14 @@ def _format_response(self, resp: Sequence[dict[str, Any]]) -> SelectRequesterRes for option in resp: if not ("value" in option and "label" in option): - logger.info( - "select-requester.invalid-response", - extra={ - "resposnse": resp, - "error_msg": "Missing `value` or `label` in option data for SelectField", + raise SentryAppIntegratorError( + message="Missing `value` or `label` in option data for Select FormField", + webhook_context={ + "error_type": "select-requester.missing-fields", + "response": resp, }, + status_code=500, ) - 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 62cfb87dac734f..989c3a6c43e348 100644 --- a/src/sentry/sentry_apps/models/sentry_app_installation.py +++ b/src/sentry/sentry_apps/models/sentry_app_installation.py @@ -19,6 +19,11 @@ from sentry.hybridcloud.outbox.category import OutboxCategory from sentry.projects.services.project import RpcProject from sentry.sentry_apps.services.app.model import RpcSentryAppComponent, RpcSentryAppInstallation +from sentry.sentry_apps.utils.errors import ( + SentryAppError, + SentryAppIntegratorError, + SentryAppSentryError, +) from sentry.types.region import find_regions_for_orgs if TYPE_CHECKING: @@ -240,6 +245,12 @@ def prepare_ui_component( component=component, install=installation, project_slug=project_slug, values=values ).run() return component - except (APIError, ValidationError): + except ( + APIError, + ValidationError, + SentryAppIntegratorError, + SentryAppError, + SentryAppSentryError, + ): # TODO(nisanthan): For now, skip showing the UI Component if the API requests fail return None diff --git a/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_issue_actions.py b/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_issue_actions.py index a0e80b7f710d4c..3972207fbccf8f 100644 --- a/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_issue_actions.py +++ b/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_issue_actions.py @@ -75,9 +75,9 @@ def test_external_issue_doesnt_get_created(self): ) response = self.client.post(self.url, data=data, format="json") - assert response.status_code == 400 + assert response.status_code == 500 assert ( response.content - == b'{"error":"Issue occured while trying to contact testin to link issue"}' + == b'{"detail":"Issue occured while trying to contact testin to link issue"}' ) assert not PlatformExternalIssue.objects.all() diff --git a/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_requests.py b/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_requests.py index d46f07773dff49..1f73c0e52107de 100644 --- a/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_requests.py +++ b/tests/sentry/sentry_apps/api/endpoints/test_sentry_app_installation_external_requests.py @@ -90,4 +90,4 @@ def test_external_request_fails(self): ) url = self.url + f"?uri={self.project.id}" response = self.client.get(url, format="json") - assert response.status_code == 400 + assert response.status_code == 500 diff --git a/tests/sentry/sentry_apps/external_requests/test_issue_link_requester.py b/tests/sentry/sentry_apps/external_requests/test_issue_link_requester.py index 082a40e84cdd6c..a7943c7a079b00 100644 --- a/tests/sentry/sentry_apps/external_requests/test_issue_link_requester.py +++ b/tests/sentry/sentry_apps/external_requests/test_issue_link_requester.py @@ -1,10 +1,9 @@ import pytest import responses -from jsonschema import ValidationError -from sentry.coreapi import APIError from sentry.sentry_apps.external_requests.issue_link_requester import IssueLinkRequester from sentry.sentry_apps.services.app import app_service +from sentry.sentry_apps.utils.errors import SentryAppIntegratorError from sentry.testutils.cases import TestCase from sentry.users.services.user.serial import serialize_rpc_user from sentry.utils import json @@ -95,7 +94,7 @@ def test_invalid_response_format(self): status=200, content_type="application/json", ) - with pytest.raises(ValidationError): + with pytest.raises(SentryAppIntegratorError): IssueLinkRequester( install=self.install, group=self.group, @@ -114,7 +113,7 @@ def test_500_response(self): status=500, ) - with pytest.raises(APIError): + with pytest.raises(SentryAppIntegratorError): IssueLinkRequester( install=self.install, group=self.group, 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 45ad13df13145d..982f2b85dce162 100644 --- a/tests/sentry/sentry_apps/external_requests/test_select_requester.py +++ b/tests/sentry/sentry_apps/external_requests/test_select_requester.py @@ -1,10 +1,9 @@ import pytest import responses -from jsonschema import ValidationError -from sentry.coreapi import APIError from sentry.sentry_apps.external_requests.select_requester import SelectRequester from sentry.sentry_apps.services.app import app_service +from sentry.sentry_apps.utils.errors import SentryAppIntegratorError from sentry.testutils.cases import TestCase from sentry.utils.sentry_apps import SentryAppWebhookRequestsBuffer @@ -61,22 +60,38 @@ def test_makes_request(self): @responses.activate def test_invalid_response_missing_label(self): # missing 'label' + url = f"https://example.com/get-issues?installationId={self.install.uuid}&projectSlug={self.project.slug}" + uri = "/get-issues" + invalid_format = {"value": "12345"} responses.add( method=responses.GET, - url=f"https://example.com/get-issues?installationId={self.install.uuid}&projectSlug={self.project.slug}", + url=url, json=invalid_format, status=200, content_type="application/json", ) - with pytest.raises(ValidationError): + with pytest.raises(SentryAppIntegratorError) as exception_info: SelectRequester( install=self.install, project_slug=self.project.slug, - uri="/get-issues", + uri=uri, ).run() + assert ( + exception_info.value.message + == f"Invalid response format for Select FormField in {self.sentry_app.slug} from uri: {uri}" + ) + assert exception_info.value.webhook_context == { + "error_type": "select-requester.invalid-integrator-response", + "response": invalid_format, + "sentry_app_slug": self.sentry_app.slug, + "install_uuid": self.install.uuid, + "project_slug": self.project.slug, + "url": url, + } + @responses.activate def test_invalid_response_missing_value(self): # missing 'label' and 'value' @@ -91,13 +106,22 @@ def test_invalid_response_missing_value(self): content_type="application/json", ) - with pytest.raises(ValidationError): + with pytest.raises(SentryAppIntegratorError) as exception_info: SelectRequester( install=self.install, project_slug=self.project.slug, uri="/get-issues", ).run() + assert ( + exception_info.value.message + == "Missing `value` or `label` in option data for Select FormField" + ) + assert exception_info.value.webhook_context == { + "error_type": "select-requester.missing-fields", + "response": invalid_format, + } + @responses.activate def test_500_response(self): responses.add( @@ -107,7 +131,7 @@ def test_500_response(self): status=500, ) - with pytest.raises(APIError): + with pytest.raises(SentryAppIntegratorError): SelectRequester( install=self.install, project_slug=self.project.slug, @@ -123,63 +147,28 @@ def test_500_response(self): @responses.activate def test_api_error_message(self): + url = f"https://example.com/get-issues?installationId={self.install.uuid}&projectSlug={self.project.slug}" responses.add( method=responses.GET, - url=f"https://example.com/get-issues?installationId={self.install.uuid}&projectSlug={self.project.slug}", + url=url, body="Something failed", status=500, ) - with pytest.raises(APIError) as exception_info: + with pytest.raises(SentryAppIntegratorError) as exception_info: SelectRequester( install=self.install, project_slug=self.project.slug, uri="/get-issues", ).run() assert ( - str(exception_info.value) - == f"Something went wrong while getting SelectFields from {self.sentry_app.slug}" - ) - - @responses.activate - def test_validation_error_message_validator(self): - uri = "/get-issues" - - responses.add( - method=responses.GET, - url=f"https://example.com/get-issues?installationId={self.install.uuid}&projectSlug={self.project.slug}", - json={}, - status=200, - ) - - with pytest.raises(ValidationError) as exception_info: - SelectRequester( - install=self.install, - project_slug=self.project.slug, - uri=uri, - ).run() - - assert ( - str(exception_info.value) - == f"Invalid response format for SelectField in {self.sentry_app.slug} from uri: {uri}" - ) - - @responses.activate - def test_validation_error_message_missing_field(self): - responses.add( - method=responses.GET, - url=f"https://example.com/get-issues?installationId={self.install.uuid}&projectSlug={self.project.slug}", - json=[{"bruh": "bruhhhhh"}], - status=200, - ) - - with pytest.raises(ValidationError) as exception_info: - SelectRequester( - install=self.install, - project_slug=self.project.slug, - uri="/get-issues", - ).run() - - assert ( - str(exception_info.value) == "Missing `value` or `label` in option data for SelectField" + exception_info.value.message + == f"Something went wrong while getting options for Select FormField from {self.sentry_app.slug}" ) + assert exception_info.value.webhook_context == { + "error_type": "select-requester.request-failed", + "sentry_app_slug": self.sentry_app.slug, + "install_uuid": self.install.uuid, + "project_slug": self.project.slug, + "url": url, + }