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

fix(sentry apps): Catch all errors so components doesn't blow up #84204

Merged
merged 3 commits into from
Jan 29, 2025
Merged
Changes from 1 commit
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
Next Next commit
backstop, so that if one integration fails we still load other UI com…
…ponents
  • Loading branch information
Christinarlong committed Jan 28, 2025

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
commit e16018df0f7526e820a86ff8b6bbab9e3cc38fc6
23 changes: 21 additions & 2 deletions src/sentry/sentry_apps/api/endpoints/sentry_app_components.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import logging

import sentry_sdk
from rest_framework.request import Request
from rest_framework.response import Response
@@ -8,7 +10,6 @@
from sentry.api.bases.organization import ControlSiloOrganizationEndpoint
from sentry.api.paginator import OffsetPaginator
from sentry.api.serializers import serialize
from sentry.coreapi import APIError
from sentry.organizations.services.organization.model import (
RpcOrganization,
RpcUserOrganizationContext,
@@ -18,6 +19,13 @@
from sentry.sentry_apps.components import SentryAppComponentPreparer
from sentry.sentry_apps.models.sentry_app_component import SentryAppComponent
from sentry.sentry_apps.models.sentry_app_installation import SentryAppInstallation
from sentry.sentry_apps.utils.errors import (
SentryAppError,
SentryAppIntegratorError,
SentryAppSentryError,
)

logger = logging.getLogger("sentry.sentry_apps.components")


# TODO(mgaeta): These endpoints are doing the same thing, but one takes a
@@ -76,7 +84,18 @@ def get(
with sentry_sdk.start_span(op="sentry-app-components.prepare_components"):
try:
SentryAppComponentPreparer(component=component, install=install).run()
except APIError:
except (SentryAppIntegratorError, SentryAppError):
errors.append(str(component.uuid))
except (SentryAppSentryError, Exception) as e:
logger.info(
"component-preperation-error",
Christinarlong marked this conversation as resolved.
Show resolved Hide resolved
exc_info=e,
extra={
"component_uuid": component.uuid,
"sentry_app": install.sentry_app.slug,
"installation_uuid": install.uuid,
},
)
errors.append(str(component.uuid))

components.append(component)
Original file line number Diff line number Diff line change
@@ -4,6 +4,7 @@
from sentry.constants import SentryAppInstallationStatus
from sentry.coreapi import APIError
from sentry.sentry_apps.models.sentry_app import SentryApp
from sentry.sentry_apps.utils.errors import SentryAppIntegratorError, SentryAppSentryError
from sentry.testutils.cases import APITestCase
from sentry.testutils.silo import control_silo_test

@@ -160,7 +161,7 @@ def test_prepares_each_component(self, run):

@patch("sentry.sentry_apps.components.SentryAppComponentPreparer.run")
def test_component_prep_errors_are_isolated(self, run):
run.side_effect = [APIError(), self.component2]
run.side_effect = [SentryAppIntegratorError(message="whoops"), self.component2]

response = self.get_success_response(
self.org.slug, qs_params={"projectId": self.project.id}
@@ -196,3 +197,42 @@ def test_component_prep_errors_are_isolated(self, run):
]

assert response.data == expected

@patch("sentry.sentry_apps.components.SentryAppComponentPreparer.run")
def test_component_prep_errors_dont_bring_down_everything(self, run):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol

run.side_effect = [APIError(), SentryAppSentryError(message="kewl")]

response = self.get_success_response(
self.org.slug, qs_params={"projectId": self.project.id}
)

# self.component1 data contains an error, because it raised an exception
# during preparation.
expected = [
{
"uuid": str(self.component1.uuid),
"type": self.component1.type,
"schema": self.component1.schema,
"error": True,
"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": True,
"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