From 3ee2817539ddd52c1f75e116bf3e03b5672682b0 Mon Sep 17 00:00:00 2001 From: Christinarlong Date: Wed, 20 Nov 2024 01:03:07 -0800 Subject: [PATCH 1/3] improve select requester typing and uh dont serialize the whole sentry app --- .../external_requests/select_requester.py | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/sentry/sentry_apps/external_requests/select_requester.py b/src/sentry/sentry_apps/external_requests/select_requester.py index b7e746811f3e7b..308a84a2fc625c 100644 --- a/src/sentry/sentry_apps/external_requests/select_requester.py +++ b/src/sentry/sentry_apps/external_requests/select_requester.py @@ -1,6 +1,7 @@ import logging +from collections.abc import Sequence from dataclasses import dataclass, field -from typing import Any +from typing import Annotated, Any, TypedDict from urllib.parse import urlencode, urlparse, urlunparse from uuid import uuid4 @@ -17,6 +18,12 @@ logger = logging.getLogger("sentry.sentry_apps.external_requests") +class SelectRequesterResult(TypedDict, total=False): + # Each contained Sequence of strings is of length 2 i.e ["label", "value"] + choices: Sequence[Annotated[Sequence[str], 2]] + defaultValue: str + + @dataclass class SelectRequester: """ @@ -34,7 +41,7 @@ class SelectRequester: query: str | None = field(default=None) dependent_data: str | None = field(default=None) - def run(self) -> dict[str, Any]: + def run(self) -> SelectRequesterResult: response: list[dict[str, str]] = [] try: url = self._build_url() @@ -85,7 +92,7 @@ def run(self) -> dict[str, Any]: }, ) raise ValidationError( - f"Invalid response format for SelectField in {self.sentry_app} from uri: {self.uri}" + f"Invalid response format for SelectField in {self.sentry_app.slug} from uri: {self.uri}" ) return self._format_response(response) @@ -107,14 +114,16 @@ def _build_url(self) -> str: urlparts[4] = urlencode(query) return str(urlunparse(urlparts)) - def _validate_response(self, resp: list[dict[str, Any]]) -> bool: + # response format must be: + # https://docs.sentry.io/organization/integrations/integration-platform/ui-components/formfield/#uri-response-format + def _validate_response(self, resp: Sequence[dict[str, Any]]) -> bool: return validate(instance=resp, schema_type="select") - def _format_response(self, resp: list[dict[str, Any]]) -> dict[str, Any]: + def _format_response(self, resp: Sequence[dict[str, Any]]) -> SelectRequesterResult: # the UI expects the following form: # choices: [[label, value]] # default: [label, value] - response: dict[str, Any] = {} + response: SelectRequesterResult = {} choices: list[list[str]] = [] for option in resp: From b1ea76a3555d9a3cf3a05532234947854a31d4c9 Mon Sep 17 00:00:00 2001 From: Christinarlong Date: Wed, 20 Nov 2024 02:56:22 -0800 Subject: [PATCH 2/3] get initial tests for messages in --- .../test_select_requester.py | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) 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 63a726515e5b58..551c98fd76b04e 100644 --- a/tests/sentry/sentry_apps/external_requests/test_select_requester.py +++ b/tests/sentry/sentry_apps/external_requests/test_select_requester.py @@ -120,3 +120,56 @@ def test_500_response(self): assert len(requests) == 1 assert requests[0]["response_code"] == 500 assert requests[0]["event_type"] == "select_options.requested" + + @responses.activate + def test_api_error_message(self): + responses.add( + method=responses.GET, + url=f"https://example.com/get-issues?installationId={self.install.uuid}&projectSlug={self.project.slug}", + body="Something failed", + status=500, + ) + + with pytest.raises(APIError) as exception_info: + SelectRequester( + install=self.install, + project_slug=self.project.slug, + uri="/get-issues", + ).run() + assert str(exception_info.value) == "hi" + + @responses.activate + def test_validation_error_message_validator(self): + responses.add( + method=responses.GET, + url=f"https://example.com/get-issues?installationId={self.install.uuid}&projectSlug={self.project.slug}", + body={}, + 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) == "hi" + + @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}", + body={"bruh": "bruhhhh"}, + 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) == "hi" From c10b09ec6d33385a8ff7e5c2734cf7acf2e88419 Mon Sep 17 00:00:00 2001 From: Christinarlong Date: Wed, 20 Nov 2024 03:04:29 -0800 Subject: [PATCH 3/3] update error msg and add tests to amke sure we only show msg --- .../external_requests/select_requester.py | 4 +++- .../test_select_requester.py | 22 ++++++++++++++----- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/sentry/sentry_apps/external_requests/select_requester.py b/src/sentry/sentry_apps/external_requests/select_requester.py index 308a84a2fc625c..9df1240653f150 100644 --- a/src/sentry/sentry_apps/external_requests/select_requester.py +++ b/src/sentry/sentry_apps/external_requests/select_requester.py @@ -78,7 +78,9 @@ def run(self) -> SelectRequesterResult: message = "select-requester.request-failed" logger.info(message, extra=extra) - raise APIError from e + raise APIError( + f"Something went wrong while getting SelectFields from {self.sentry_app.slug}" + ) from e if not self._validate_response(response): logger.info( 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 551c98fd76b04e..45ad13df13145d 100644 --- a/tests/sentry/sentry_apps/external_requests/test_select_requester.py +++ b/tests/sentry/sentry_apps/external_requests/test_select_requester.py @@ -136,14 +136,19 @@ def test_api_error_message(self): project_slug=self.project.slug, uri="/get-issues", ).run() - assert str(exception_info.value) == "hi" + 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}", - body={}, + json={}, status=200, ) @@ -151,17 +156,20 @@ def test_validation_error_message_validator(self): SelectRequester( install=self.install, project_slug=self.project.slug, - uri="/get-issues", + uri=uri, ).run() - assert str(exception_info.value) == "hi" + 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}", - body={"bruh": "bruhhhh"}, + json=[{"bruh": "bruhhhhh"}], status=200, ) @@ -172,4 +180,6 @@ def test_validation_error_message_missing_field(self): uri="/get-issues", ).run() - assert str(exception_info.value) == "hi" + assert ( + str(exception_info.value) == "Missing `value` or `label` in option data for SelectField" + )