Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ref(sentry apps): Improve Sentry App Component endpoint errors #81167

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 15 additions & 5 deletions src/sentry/sentry_apps/api/endpoints/sentry_app_components.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import sentry_sdk
from jsonschema import ValidationError
from rest_framework.request import Request
from rest_framework.response import Response

Expand Down Expand Up @@ -39,7 +40,7 @@ def get(self, request: Request, sentry_app) -> Response:
queryset=sentry_app.components.all(),
paginator_cls=OffsetPaginator,
on_results=lambda x: serialize(
x, request.user, errors=[], serializer=SentryAppComponentSerializer()
x, request.user, errors={}, serializer=SentryAppComponentSerializer()
),
)

Expand All @@ -59,7 +60,7 @@ def get(
organization: RpcOrganization,
) -> Response:
components = []
errors = []
errors = {}

with sentry_sdk.start_transaction(name="sentry.api.sentry_app_components.get"):
with sentry_sdk.start_span(op="sentry-app-components.get_installs"):
Expand All @@ -80,11 +81,20 @@ def get(
with sentry_sdk.start_span(op="sentry-app-components.prepare_components"):
try:
SentryAppComponentPreparer(component=component, install=install).run()
except APIError:
errors.append(str(component.uuid))

components.append(component)
# APIError is for webhook request fails
except (APIError, ValidationError) as e:
errors[str(component.uuid)] = (
f"Encountered error: {str(e)}, while preparing component: {str(component.uuid)}"
)

except Exception as e:
error_id = sentry_sdk.capture_exception(e)
errors[str(component.uuid)] = (
f"Something went wrong while trying to link issue for component: {str(component.uuid)}. Sentry error ID: {error_id}"
)

components.append(component)
return self.paginate(
request=request,
queryset=components,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def serialize(self, obj, attrs, user, **kwargs):
"uuid": str(obj.uuid),
"type": obj.type,
"schema": obj.schema,
"error": True if str(obj.uuid) in errors else False,
"error": errors.get(str(obj.uuid), None) or "",
"sentryApp": {
"uuid": obj.sentry_app.uuid,
"slug": obj.sentry_app.slug,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ def run(self) -> SelectRequesterResult:
},
)
raise ValidationError(
f"Invalid response format for SelectField in {self.sentry_app.slug} from uri: {self.uri}"
f"Invalid response format for SelectField in {self.sentry_app.slug.slug} from uri: {self.uri}"
)
return self._format_response(response)

Expand Down
178 changes: 172 additions & 6 deletions tests/sentry/sentry_apps/api/endpoints/test_sentry_app_components.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
from collections.abc import Sequence
from unittest.mock import patch

import responses

from sentry.api.serializers.base import serialize
from sentry.constants import SentryAppInstallationStatus
from sentry.coreapi import APIError
from sentry.sentry_apps.models.sentry_app import SentryApp
from sentry.sentry_apps.models.sentry_app_component import SentryAppComponent
from sentry.testutils.cases import APITestCase
from sentry.testutils.silo import control_silo_test

Expand Down Expand Up @@ -39,7 +43,7 @@ def test_retrieves_all_components(self):
"uuid": str(self.component.uuid),
"type": "issue-link",
"schema": self.component.schema,
"error": False,
"error": "",
"sentryApp": {
"uuid": self.sentry_app.uuid,
"slug": self.sentry_app.slug,
Expand Down Expand Up @@ -103,7 +107,7 @@ def test_retrieves_all_components_for_installed_apps(self, run):
"uuid": str(self.component1.uuid),
"type": "issue-link",
"schema": self.component1.schema,
"error": False,
"error": "",
"sentryApp": {
"uuid": self.sentry_app1.uuid,
"slug": self.sentry_app1.slug,
Expand All @@ -116,7 +120,7 @@ def test_retrieves_all_components_for_installed_apps(self, run):
"uuid": str(self.component2.uuid),
"type": "issue-link",
"schema": self.component2.schema,
"error": False,
"error": "",
"sentryApp": {
"uuid": self.sentry_app2.uuid,
"slug": self.sentry_app2.slug,
Expand All @@ -142,7 +146,7 @@ def test_filter_by_type(self, run):
"uuid": str(component.uuid),
"type": "alert-rule",
"schema": component.schema,
"error": False,
"error": "",
"sentryApp": {
"uuid": sentry_app.uuid,
"slug": sentry_app.slug,
Expand Down Expand Up @@ -173,7 +177,153 @@ def test_component_prep_errors_are_isolated(self, run):
"uuid": str(self.component1.uuid),
"type": self.component1.type,
"schema": self.component1.schema,
"error": True,
"error": f"Encountered error: Invalid request, while preparing component: {str(self.component1.uuid)}",
"sentryApp": {
"uuid": self.sentry_app1.uuid,
"slug": self.sentry_app1.slug,
"name": self.sentry_app1.name,
"avatars": get_sentry_app_avatars(self.sentry_app1),
},
},
{
"uuid": str(self.component2.uuid),
"type": self.component2.type,
"schema": self.component2.schema,
"error": "",
"sentryApp": {
"uuid": self.sentry_app2.uuid,
"slug": self.sentry_app2.slug,
"name": self.sentry_app2.name,
"avatars": get_sentry_app_avatars(self.sentry_app2),
},
},
]

assert response.data == expected

@responses.activate
def test_component_prep_api_error(self):
responses.add(
method=responses.GET,
url="https://example.com/",
json={"error": "the dumpsters on fire!!!"},
status=500,
content_type="application/json",
)

responses.add(
method=responses.GET,
url="https://example.com/",
json={"error": "couldnt find the dumpsters :C"},
status=404,
content_type="application/json",
)

response = self.get_success_response(
self.org.slug, qs_params={"projectId": self.project.id}
)
expected = [
{
"uuid": str(self.component1.uuid),
"type": self.component1.type,
"schema": self.component1.schema,
"error": f"Encountered error: Something went wrong while getting SelectFields from {self.sentry_app1.slug}, while preparing component: {str(self.component1.uuid)}",
"sentryApp": {
"uuid": self.sentry_app1.uuid,
"slug": self.sentry_app1.slug,
"name": self.sentry_app1.name,
"avatars": get_sentry_app_avatars(self.sentry_app1),
},
},
{
"uuid": str(self.component2.uuid),
"type": self.component2.type,
"schema": self.component2.schema,
"error": f"Encountered error: Something went wrong while getting SelectFields from {self.sentry_app2.slug}, while preparing component: {str(self.component2.uuid)}",
"sentryApp": {
"uuid": self.sentry_app2.uuid,
"slug": self.sentry_app2.slug,
"name": self.sentry_app2.name,
"avatars": get_sentry_app_avatars(self.sentry_app2),
},
},
]

assert response.data == expected

@responses.activate
def test_component_prep_validation_error(self):
component1_uris = self._get_component_uris(
component_field="link", component=self.component1
)

component2_uris = self._get_component_uris(
component_field="link", component=self.component2
)

# We only get the first uri since the SentryAppComponentPreparer will short circuit after getting the first error
responses.add(
method=responses.GET,
url=f"https://example.com{component1_uris[0]}?installationId={self.install1.uuid}",
json=[{"bruh": "the dumpsters on fire!!!"}],
status=200,
content_type="application/json",
)

responses.add(
method=responses.GET,
url=f"https://example.com{component2_uris[0]}?installationId={self.install2.uuid}",
json={},
status=200,
content_type="application/json",
)

response = self.get_success_response(
self.org.slug, qs_params={"projectId": self.project.id}
)
expected = [
{
"uuid": str(self.component1.uuid),
"type": self.component1.type,
"schema": self.component1.schema,
"error": f"Encountered error: Missing `value` or `label` in option data for SelectField, while preparing component: {str(self.component1.uuid)}",
"sentryApp": {
"uuid": self.sentry_app1.uuid,
"slug": self.sentry_app1.slug,
"name": self.sentry_app1.name,
"avatars": get_sentry_app_avatars(self.sentry_app1),
},
},
{
"uuid": str(self.component2.uuid),
"type": self.component2.type,
"schema": self.component2.schema,
"error": f"Encountered error: Invalid response format for SelectField in {self.sentry_app2.slug} from uri: {component2_uris[0]}, while preparing component: {str(self.component2.uuid)}",
"sentryApp": {
"uuid": self.sentry_app2.uuid,
"slug": self.sentry_app2.slug,
"name": self.sentry_app2.name,
"avatars": get_sentry_app_avatars(self.sentry_app2),
},
},
]

assert response.data == expected

@patch("sentry_sdk.capture_exception")
@patch("sentry.sentry_apps.components.SentryAppComponentPreparer.run")
def test_component_prep_general_error(self, run, capture_exception):
run.side_effect = [Exception(":dead:"), KeyError("oh shit swip split snip")]
capture_exception.return_value = 1
response = self.get_success_response(
self.org.slug, qs_params={"projectId": self.project.id}
)
expected = [
{
"uuid": str(self.component1.uuid),
"type": self.component1.type,
"schema": self.component1.schema,
"error": f"Something went wrong while trying to link issue for component: {str(self.component1.uuid)}. Sentry error ID: {capture_exception.return_value}",
"sentryApp": {
"uuid": self.sentry_app1.uuid,
"slug": self.sentry_app1.slug,
Expand All @@ -185,7 +335,7 @@ def test_component_prep_errors_are_isolated(self, run):
"uuid": str(self.component2.uuid),
"type": self.component2.type,
"schema": self.component2.schema,
"error": False,
"error": f"Something went wrong while trying to link issue for component: {str(self.component2.uuid)}. Sentry error ID: {capture_exception.return_value}",
"sentryApp": {
"uuid": self.sentry_app2.uuid,
"slug": self.sentry_app2.slug,
Expand All @@ -196,3 +346,19 @@ def test_component_prep_errors_are_isolated(self, run):
]

assert response.data == expected

def _get_component_uris(
self, component_field: str, component: SentryAppComponent
) -> Sequence[str]:
fields = dict(**component.app_schema).get(component_field)
uris = []

for field in fields.get("required_fields", []):
if "uri" in field:
uris.append(field.get("uri"))

for field in fields.get("optional_fields", []):
if "uri" in field:
uris.append(field.get("uri"))

return uris
Loading