Skip to content

Commit

Permalink
update tests and update returned errors to be a bit more acc
Browse files Browse the repository at this point in the history
  • Loading branch information
Christinarlong committed Oct 19, 2024
1 parent 7d8052e commit 133aa4b
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 12 deletions.
Original file line number Diff line number Diff line change
@@ -1,14 +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.api.client import ApiError
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):
Expand Down Expand Up @@ -37,13 +42,12 @@ def get(self, request: Request, installation) -> Response:

try:
choices = SelectRequester(**kwargs).run()
except ApiError:
message = f"Error validating response options from {installation.sentry_app.slug} for {request.GET.get('uri')}"
except ValidationError as e:
return Response(
{"error": message},
{"error": e.message},
status=400,
)
except Exception:
except APIError:
message = f'Error retrieving select field options from {request.GET.get("uri")}'
return Response(
{"error": message},
Expand Down
14 changes: 11 additions & 3 deletions src/sentry/sentry_apps/external_requests/select_requester.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -60,7 +61,7 @@ def run(self) -> dict[str, Any]:
"url": url,
},
)
raise
raise APIError from e

if not self._validate_response(response) or not response:
logger.info(
Expand All @@ -74,7 +75,7 @@ def run(self) -> dict[str, Any]:
"url": url,
},
)
raise APIError(
raise ValidationError(
f"Invalid response format for SelectField in {self.sentry_app} from uri: {self.uri}"
)
return self._format_response(response)
Expand Down Expand Up @@ -109,7 +110,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"]])

Expand Down
3 changes: 2 additions & 1 deletion src/sentry/sentry_apps/models/sentry_app_installation.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -106,7 +107,7 @@ def test_500_response(self):
status=500,
)

with pytest.raises(Exception):
with pytest.raises(APIError):
SelectRequester(
install=self.install,
project_slug=self.project.slug,
Expand Down

0 comments on commit 133aa4b

Please sign in to comment.