Skip to content

Commit e38ab01

Browse files
chore(sentryapps): better logging and errors for select requester (#79377)
1 parent 66a70aa commit e38ab01

File tree

4 files changed

+47
-10
lines changed

4 files changed

+47
-10
lines changed

src/sentry/sentry_apps/api/endpoints/installation_external_requests.py

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,19 @@
1+
import logging
2+
3+
from jsonschema import ValidationError
14
from rest_framework.request import Request
25
from rest_framework.response import Response
36

47
from sentry.api.api_owners import ApiOwner
58
from sentry.api.api_publish_status import ApiPublishStatus
69
from sentry.api.base import region_silo_endpoint
10+
from sentry.coreapi import APIError
711
from sentry.models.project import Project
812
from sentry.sentry_apps.api.bases.sentryapps import SentryAppInstallationBaseEndpoint
913
from sentry.sentry_apps.external_requests.select_requester import SelectRequester
1014

15+
logger = logging.getLogger("sentry.sentry-apps")
16+
1117

1218
@region_silo_endpoint
1319
class SentryAppInstallationExternalRequestsEndpoint(SentryAppInstallationBaseEndpoint):
@@ -36,7 +42,16 @@ def get(self, request: Request, installation) -> Response:
3642

3743
try:
3844
choices = SelectRequester(**kwargs).run()
39-
except Exception:
40-
return Response({"error": "Error communicating with Sentry App service"}, status=400)
45+
except ValidationError as e:
46+
return Response(
47+
{"error": e.message},
48+
status=400,
49+
)
50+
except APIError:
51+
message = f'Error retrieving select field options from {request.GET.get("uri")}'
52+
return Response(
53+
{"error": message},
54+
status=400,
55+
)
4156

4257
return Response(choices)

src/sentry/sentry_apps/external_requests/select_requester.py

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
from uuid import uuid4
66

77
from django.utils.functional import cached_property
8+
from jsonschema import ValidationError
89

910
from sentry.coreapi import APIError
1011
from sentry.http import safe_urlread
@@ -36,9 +37,10 @@ class SelectRequester:
3637
def run(self) -> dict[str, Any]:
3738
response: list[dict[str, str]] = []
3839
try:
40+
url = self._build_url()
3941
body = safe_urlread(
4042
send_and_save_sentry_app_request(
41-
self._build_url(),
43+
url,
4244
self.sentry_app,
4345
self.install.organization_id,
4446
"select_options.requested",
@@ -54,13 +56,24 @@ def run(self) -> dict[str, Any]:
5456
"sentry_app_slug": self.sentry_app.slug,
5557
"install_uuid": self.install.uuid,
5658
"project_slug": self.project_slug,
57-
"uri": self.uri,
5859
"error_message": str(e),
60+
"url": url,
5961
},
6062
)
63+
raise APIError from e
6164

62-
if not self._validate_response(response) or not response:
63-
raise APIError(
65+
if not self._validate_response(response):
66+
logger.info(
67+
"select-requester.invalid-response",
68+
extra={
69+
"response": response,
70+
"sentry_app_slug": self.sentry_app.slug,
71+
"install_uuid": self.install.uuid,
72+
"project_slug": self.project_slug,
73+
"url": url,
74+
},
75+
)
76+
raise ValidationError(
6477
f"Invalid response format for SelectField in {self.sentry_app} from uri: {self.uri}"
6578
)
6679
return self._format_response(response)
@@ -95,7 +108,14 @@ def _format_response(self, resp: list[dict[str, Any]]) -> dict[str, Any]:
95108

96109
for option in resp:
97110
if not ("value" in option and "label" in option):
98-
raise APIError("Missing `value` or `label` in option data for SelectField")
111+
logger.info(
112+
"select-requester.invalid-response",
113+
extra={
114+
"resposnse": resp,
115+
"error_msg": "Missing `value` or `label` in option data for SelectField",
116+
},
117+
)
118+
raise ValidationError("Missing `value` or `label` in option data for SelectField")
99119

100120
choices.append([option["value"], option["label"]])
101121

src/sentry/sentry_apps/models/sentry_app_installation.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
from django.db import models
88
from django.db.models import QuerySet
99
from django.utils import timezone
10+
from jsonschema import ValidationError
1011

1112
from sentry.auth.services.auth import AuthenticatedToken
1213
from sentry.backup.scopes import RelocationScope
@@ -241,6 +242,6 @@ def prepare_ui_component(
241242
component=component, install=installation, project_slug=project_slug, values=values
242243
).run()
243244
return component
244-
except APIError:
245+
except (APIError, ValidationError):
245246
# TODO(nisanthan): For now, skip showing the UI Component if the API requests fail
246247
return None

tests/sentry/sentry_apps/external_requests/test_select_requester.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import pytest
22
import responses
3+
from jsonschema import ValidationError
34

45
from sentry.coreapi import APIError
56
from sentry.sentry_apps.external_requests.select_requester import SelectRequester
@@ -69,7 +70,7 @@ def test_invalid_response_missing_label(self):
6970
content_type="application/json",
7071
)
7172

72-
with pytest.raises(APIError):
73+
with pytest.raises(ValidationError):
7374
SelectRequester(
7475
install=self.install,
7576
project_slug=self.project.slug,
@@ -90,7 +91,7 @@ def test_invalid_response_missing_value(self):
9091
content_type="application/json",
9192
)
9293

93-
with pytest.raises(APIError):
94+
with pytest.raises(ValidationError):
9495
SelectRequester(
9596
install=self.install,
9697
project_slug=self.project.slug,

0 commit comments

Comments
 (0)