From 74bb9a70328a6616601b60e14ac2acc4ce8c2043 Mon Sep 17 00:00:00 2001 From: Christinarlong Date: Thu, 5 Dec 2024 11:27:42 -0800 Subject: [PATCH 01/13] wrap errors in SAE --- src/sentry/exceptions.py | 4 ++++ .../installation_external_issue_actions.py | 5 ++--- .../external_issues/issue_link_creator.py | 3 ++- .../external_requests/issue_link_requester.py | 17 ++++++++++++----- 4 files changed, 20 insertions(+), 9 deletions(-) diff --git a/src/sentry/exceptions.py b/src/sentry/exceptions.py index 1c162069237a05..74b3ed9805417e 100644 --- a/src/sentry/exceptions.py +++ b/src/sentry/exceptions.py @@ -91,3 +91,7 @@ def __init__( class InvalidParams(Exception): pass + + +class SentryAppError(Exception): + pass diff --git a/src/sentry/sentry_apps/api/endpoints/installation_external_issue_actions.py b/src/sentry/sentry_apps/api/endpoints/installation_external_issue_actions.py index 13ae408d361ae1..8afb5c49d752e5 100644 --- a/src/sentry/sentry_apps/api/endpoints/installation_external_issue_actions.py +++ b/src/sentry/sentry_apps/api/endpoints/installation_external_issue_actions.py @@ -1,5 +1,4 @@ from django.utils.functional import empty -from jsonschema import ValidationError from rest_framework.request import Request from rest_framework.response import Response from sentry_sdk import capture_exception @@ -8,7 +7,7 @@ from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import region_silo_endpoint from sentry.api.serializers import serialize -from sentry.coreapi import APIError, APIUnauthorized +from sentry.exceptions import SentryAppError from sentry.models.group import Group from sentry.models.project import Project from sentry.sentry_apps.api.bases.sentryapps import SentryAppInstallationBaseEndpoint @@ -74,7 +73,7 @@ def post(self, request: Request, installation) -> Response: uri=uri, user=user, ).run() - except (APIError, ValidationError, APIUnauthorized) as e: + except SentryAppError as e: return Response({"error": str(e)}, status=400) except Exception as e: error_id = capture_exception(e) diff --git a/src/sentry/sentry_apps/external_issues/issue_link_creator.py b/src/sentry/sentry_apps/external_issues/issue_link_creator.py index 7c12f8608b801d..5884cb4deed742 100644 --- a/src/sentry/sentry_apps/external_issues/issue_link_creator.py +++ b/src/sentry/sentry_apps/external_issues/issue_link_creator.py @@ -4,6 +4,7 @@ from django.db import router, transaction from sentry.coreapi import APIUnauthorized +from sentry.exceptions import SentryAppError from sentry.models.group import Group from sentry.sentry_apps.external_issues.external_issue_creator import ExternalIssueCreator from sentry.sentry_apps.external_requests.issue_link_requester import IssueLinkRequester @@ -32,7 +33,7 @@ def run(self) -> PlatformExternalIssue: def _verify_action(self) -> None: if self.action not in VALID_ACTIONS: - raise APIUnauthorized(f"Invalid action '{self.action}'") + raise SentryAppError(APIUnauthorized(f"Invalid action '{self.action}'")) def _make_external_request(self) -> dict[str, Any]: response = IssueLinkRequester( diff --git a/src/sentry/sentry_apps/external_requests/issue_link_requester.py b/src/sentry/sentry_apps/external_requests/issue_link_requester.py index 50279b4d79f8ff..edd9aa31d6d558 100644 --- a/src/sentry/sentry_apps/external_requests/issue_link_requester.py +++ b/src/sentry/sentry_apps/external_requests/issue_link_requester.py @@ -8,6 +8,7 @@ from jsonschema import ValidationError from sentry.coreapi import APIError +from sentry.exceptions import SentryAppError from sentry.http import safe_urlread from sentry.models.group import Group from sentry.sentry_apps.external_requests.utils import send_and_save_sentry_app_request, validate @@ -74,7 +75,9 @@ def run(self) -> dict[str, Any]: body = safe_urlread(request) response = json.loads(body) except (json.JSONDecodeError, TypeError): - raise ValidationError(f"Unable to parse response from {self.sentry_app.slug}") + raise SentryAppError( + ValidationError(f"Unable to parse response from {self.sentry_app.slug}") + ) except Exception as e: logger.info( "issue-link-requester.error", @@ -87,13 +90,17 @@ def run(self) -> dict[str, Any]: "error_message": str(e), }, ) - raise APIError( - f"Issue occured while trying to contact {self.sentry_app.slug} to link issue" + raise SentryAppError( + APIError( + f"Issue occured while trying to contact {self.sentry_app.slug} to link issue" + ) ) if not self._validate_response(response): - raise ValidationError( - f"Invalid response format from sentry app {self.sentry_app} when linking issue" + raise SentryAppError( + ValidationError( + f"Invalid response format from sentry app {self.sentry_app} when linking issue" + ) ) return response From ce32a58a06b7bdbdc851edb98cc8e22b1e96f9b5 Mon Sep 17 00:00:00 2001 From: Christinarlong Date: Thu, 5 Dec 2024 14:07:32 -0800 Subject: [PATCH 02/13] add in sentryappintegratorerror --- src/sentry/exceptions.py | 6 ++++++ .../api/endpoints/installation_external_issue_actions.py | 4 ++-- .../sentry_apps/external_requests/issue_link_requester.py | 8 ++++---- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/sentry/exceptions.py b/src/sentry/exceptions.py index 74b3ed9805417e..d4bbe4010443bb 100644 --- a/src/sentry/exceptions.py +++ b/src/sentry/exceptions.py @@ -93,5 +93,11 @@ class InvalidParams(Exception): pass +# Represents a user/client error that occured during a Sentry App process class SentryAppError(Exception): pass + + +# Represents an error caused by a 3p integrator during a Sentry App process +class SentryAppIntegratorError(Exception): + pass diff --git a/src/sentry/sentry_apps/api/endpoints/installation_external_issue_actions.py b/src/sentry/sentry_apps/api/endpoints/installation_external_issue_actions.py index 8afb5c49d752e5..75a64cab0fc228 100644 --- a/src/sentry/sentry_apps/api/endpoints/installation_external_issue_actions.py +++ b/src/sentry/sentry_apps/api/endpoints/installation_external_issue_actions.py @@ -7,7 +7,7 @@ from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import region_silo_endpoint from sentry.api.serializers import serialize -from sentry.exceptions import SentryAppError +from sentry.exceptions import SentryAppError, SentryAppIntegratorError from sentry.models.group import Group from sentry.models.project import Project from sentry.sentry_apps.api.bases.sentryapps import SentryAppInstallationBaseEndpoint @@ -73,7 +73,7 @@ def post(self, request: Request, installation) -> Response: uri=uri, user=user, ).run() - except SentryAppError as e: + except (SentryAppError, SentryAppIntegratorError) as e: return Response({"error": str(e)}, status=400) except Exception as e: error_id = capture_exception(e) diff --git a/src/sentry/sentry_apps/external_requests/issue_link_requester.py b/src/sentry/sentry_apps/external_requests/issue_link_requester.py index edd9aa31d6d558..61964339ca9512 100644 --- a/src/sentry/sentry_apps/external_requests/issue_link_requester.py +++ b/src/sentry/sentry_apps/external_requests/issue_link_requester.py @@ -8,7 +8,7 @@ from jsonschema import ValidationError from sentry.coreapi import APIError -from sentry.exceptions import SentryAppError +from sentry.exceptions import SentryAppIntegratorError from sentry.http import safe_urlread from sentry.models.group import Group from sentry.sentry_apps.external_requests.utils import send_and_save_sentry_app_request, validate @@ -75,7 +75,7 @@ def run(self) -> dict[str, Any]: body = safe_urlread(request) response = json.loads(body) except (json.JSONDecodeError, TypeError): - raise SentryAppError( + raise SentryAppIntegratorError( ValidationError(f"Unable to parse response from {self.sentry_app.slug}") ) except Exception as e: @@ -90,14 +90,14 @@ def run(self) -> dict[str, Any]: "error_message": str(e), }, ) - raise SentryAppError( + raise SentryAppIntegratorError( APIError( f"Issue occured while trying to contact {self.sentry_app.slug} to link issue" ) ) if not self._validate_response(response): - raise SentryAppError( + raise SentryAppIntegratorError( ValidationError( f"Invalid response format from sentry app {self.sentry_app} when linking issue" ) From 5451caf3bd116601b9a08b87f273a3e74819939d Mon Sep 17 00:00:00 2001 From: Christinarlong Date: Sun, 8 Dec 2024 03:20:15 -0800 Subject: [PATCH 03/13] update errors for sentry apps and alert rule action --- .../api/endpoints/project_rule_details.py | 18 +++++++++++++++++- src/sentry/incidents/utils/sentry_apps.py | 4 +--- src/sentry/rules/actions/sentry_apps/utils.py | 8 ++------ .../sentry_apps/alert_rule_action_creator.py | 3 ++- .../installation_external_issue_details.py | 2 +- .../installation_external_requests.py | 18 ++++++++++-------- .../alert_rule_action_requester.py | 11 +++++------ .../external_requests/select_requester.py | 17 ++++++++++++----- src/sentry/sentry_apps/installations.py | 8 +++++--- src/sentry/sentry_apps/services/app/impl.py | 5 +---- 10 files changed, 56 insertions(+), 38 deletions(-) diff --git a/src/sentry/api/endpoints/project_rule_details.py b/src/sentry/api/endpoints/project_rule_details.py index 0559c04d52fe0f..76825d691548f8 100644 --- a/src/sentry/api/endpoints/project_rule_details.py +++ b/src/sentry/api/endpoints/project_rule_details.py @@ -2,6 +2,7 @@ import logging +import sentry_sdk from drf_spectacular.utils import extend_schema from rest_framework import serializers, status from rest_framework.request import Request @@ -27,6 +28,7 @@ from sentry.apidocs.parameters import GlobalParams, IssueAlertParams from sentry.constants import ObjectStatus from sentry.deletions.models.scheduleddeletion import RegionScheduledDeletion +from sentry.exceptions import SentryAppIntegratorError from sentry.integrations.jira.actions.create_ticket import JiraCreateTicketAction from sentry.integrations.jira_server.actions.create_ticket import JiraServerCreateTicketAction from sentry.integrations.slack.tasks.find_channel_id_for_rule import find_channel_id_for_rule @@ -285,7 +287,21 @@ def put(self, request: Request, project, rule) -> Response: context = {"uuid": client.uuid} return Response(context, status=202) - trigger_sentry_app_action_creators_for_issues(actions=kwargs["actions"]) + try: + trigger_sentry_app_action_creators_for_issues(actions=kwargs["actions"]) + except SentryAppIntegratorError as e: + return Response( + {"error": str(e)}, + status=status.HTTP_400_BAD_REQUEST, + ) + except Exception as e: + error_id = sentry_sdk.capture_exception(e) + return Response( + { + "error": f"Something went wrong while trying to create alert rule action. Sentry error ID: {error_id}" + }, + status=500, + ) updated_rule = ProjectRuleUpdater( rule=rule, diff --git a/src/sentry/incidents/utils/sentry_apps.py b/src/sentry/incidents/utils/sentry_apps.py index e7fd63be4f094f..01c262ec3676f2 100644 --- a/src/sentry/incidents/utils/sentry_apps.py +++ b/src/sentry/incidents/utils/sentry_apps.py @@ -28,9 +28,7 @@ def trigger_sentry_app_action_creators_for_incidents(alert_rule_data: Mapping[st if not action_serializer.is_valid(): raise serializers.ValidationError(action_serializer.errors) - result = app_service.trigger_sentry_app_action_creators( + app_service.trigger_sentry_app_action_creators( fields=action.get("sentry_app_config"), install_uuid=action.get("sentry_app_installation_uuid"), ) - if not result.success: - raise serializers.ValidationError({"sentry_app": result.message}) diff --git a/src/sentry/rules/actions/sentry_apps/utils.py b/src/sentry/rules/actions/sentry_apps/utils.py index fa23dda81fad38..cef2685597323f 100644 --- a/src/sentry/rules/actions/sentry_apps/utils.py +++ b/src/sentry/rules/actions/sentry_apps/utils.py @@ -3,8 +3,6 @@ from collections.abc import Mapping, Sequence from typing import Any -from rest_framework import serializers - from sentry.constants import SENTRY_APP_ACTIONS from sentry.sentry_apps.services.app import app_service @@ -18,11 +16,9 @@ def trigger_sentry_app_action_creators_for_issues( if not action.get("id") in SENTRY_APP_ACTIONS: continue - result = app_service.trigger_sentry_app_action_creators( + app_service.trigger_sentry_app_action_creators( fields=action["settings"], install_uuid=action.get("sentryAppInstallationUuid") ) - # Bubble up errors from Sentry App to the UI - if not result.success: - raise serializers.ValidationError({"actions": [result.message]}) + created = "alert-rule-action" return created diff --git a/src/sentry/sentry_apps/alert_rule_action_creator.py b/src/sentry/sentry_apps/alert_rule_action_creator.py index cd7e1632d297ec..4ac0779be6bd12 100644 --- a/src/sentry/sentry_apps/alert_rule_action_creator.py +++ b/src/sentry/sentry_apps/alert_rule_action_creator.py @@ -6,6 +6,7 @@ from django.utils.functional import cached_property from sentry.coreapi import APIError +from sentry.exceptions import SentryAppIntegratorError from sentry.sentry_apps.external_requests.alert_rule_action_requester import ( AlertRuleActionRequester, AlertRuleActionResult, @@ -34,7 +35,7 @@ def _fetch_sentry_app_uri(self): def _make_external_request(self, uri=None): if uri is None: - raise APIError("Sentry App request url not found") + raise SentryAppIntegratorError(APIError("Sentry App request url not found")) response = AlertRuleActionRequester( install=self.install, uri=uri, diff --git a/src/sentry/sentry_apps/api/endpoints/installation_external_issue_details.py b/src/sentry/sentry_apps/api/endpoints/installation_external_issue_details.py index 3bbd636b84623d..5743816371ff44 100644 --- a/src/sentry/sentry_apps/api/endpoints/installation_external_issue_details.py +++ b/src/sentry/sentry_apps/api/endpoints/installation_external_issue_details.py @@ -24,7 +24,7 @@ def delete(self, request: Request, installation, external_issue_id) -> Response: service_type=installation.sentry_app.slug, ) except PlatformExternalIssue.DoesNotExist: - return Response(status=404) + return Response({"error": "Could not find requested external issue"}, status=404) deletions.exec_sync(platform_external_issue) diff --git a/src/sentry/sentry_apps/api/endpoints/installation_external_requests.py b/src/sentry/sentry_apps/api/endpoints/installation_external_requests.py index d7b0d19592303a..57d13c912bf3f8 100644 --- a/src/sentry/sentry_apps/api/endpoints/installation_external_requests.py +++ b/src/sentry/sentry_apps/api/endpoints/installation_external_requests.py @@ -1,13 +1,13 @@ import logging -from jsonschema import ValidationError +import sentry_sdk 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.coreapi import APIError +from sentry.exceptions import SentryAppError, SentryAppIntegratorError 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 @@ -42,16 +42,18 @@ def get(self, request: Request, installation) -> Response: try: choices = SelectRequester(**kwargs).run() - except ValidationError as e: + except (SentryAppIntegratorError, SentryAppError) as e: return Response( - {"error": e.message}, + {"error": str(e)}, status=400, ) - except APIError: - message = f'Error retrieving select field options from {request.GET.get("uri")}' + except Exception as e: + error_id = sentry_sdk.capture_exception(e) return Response( - {"error": message}, - status=400, + { + "error": f"Something went wrong while trying to get Select FormField options. Sentry error ID: {error_id}" + }, + status=500, ) return Response(choices) diff --git a/src/sentry/sentry_apps/external_requests/alert_rule_action_requester.py b/src/sentry/sentry_apps/external_requests/alert_rule_action_requester.py index a9249af4849f63..e0d7d19ac04914 100644 --- a/src/sentry/sentry_apps/external_requests/alert_rule_action_requester.py +++ b/src/sentry/sentry_apps/external_requests/alert_rule_action_requester.py @@ -6,16 +6,15 @@ from uuid import uuid4 from django.utils.functional import cached_property -from requests import RequestException from requests.models import Response +from sentry.exceptions import SentryAppIntegratorError from sentry.sentry_apps.external_requests.utils import send_and_save_sentry_app_request from sentry.sentry_apps.models.sentry_app_installation import SentryAppInstallation from sentry.sentry_apps.services.app.model import RpcSentryAppInstallation from sentry.utils import json DEFAULT_SUCCESS_MESSAGE = "Success!" -DEFAULT_ERROR_MESSAGE = "Something went wrong!" logger = logging.getLogger("sentry.sentry_apps.external_requests") @@ -44,7 +43,7 @@ def run(self) -> AlertRuleActionResult: data=self.body, ) - except RequestException as e: + except Exception as e: logger.info( "alert_rule_action.error", extra={ @@ -55,9 +54,9 @@ def run(self) -> AlertRuleActionResult: }, ) - return AlertRuleActionResult( - success=False, message=self._get_response_message(e.response, DEFAULT_ERROR_MESSAGE) - ) + raise SentryAppIntegratorError( + f"Something went wrong while setting up alert for {self.sentry_app.slug}" + ) from e return AlertRuleActionResult( success=True, message=self._get_response_message(response, DEFAULT_SUCCESS_MESSAGE) ) diff --git a/src/sentry/sentry_apps/external_requests/select_requester.py b/src/sentry/sentry_apps/external_requests/select_requester.py index 9df1240653f150..8b4eb39d41e12f 100644 --- a/src/sentry/sentry_apps/external_requests/select_requester.py +++ b/src/sentry/sentry_apps/external_requests/select_requester.py @@ -9,6 +9,7 @@ from jsonschema import ValidationError from sentry.coreapi import APIError +from sentry.exceptions import SentryAppIntegratorError from sentry.http import safe_urlread from sentry.sentry_apps.external_requests.utils import send_and_save_sentry_app_request, validate from sentry.sentry_apps.services.app import RpcSentryAppInstallation @@ -78,8 +79,10 @@ def run(self) -> SelectRequesterResult: message = "select-requester.request-failed" logger.info(message, extra=extra) - raise APIError( - f"Something went wrong while getting SelectFields from {self.sentry_app.slug}" + raise SentryAppIntegratorError( + APIError( + f"Something went wrong while getting SelectFields from {self.sentry_app.slug}" + ) ) from e if not self._validate_response(response): @@ -93,8 +96,10 @@ def run(self) -> SelectRequesterResult: "url": url, }, ) - raise ValidationError( - f"Invalid response format for SelectField in {self.sentry_app.slug} from uri: {self.uri}" + raise SentryAppIntegratorError( + ValidationError( + f"Invalid response format for SelectField in {self.sentry_app.slug} from uri: {self.uri}" + ) ) return self._format_response(response) @@ -137,7 +142,9 @@ def _format_response(self, resp: Sequence[dict[str, Any]]) -> SelectRequesterRes "error_msg": "Missing `value` or `label` in option data for SelectField", }, ) - raise ValidationError("Missing `value` or `label` in option data for SelectField") + raise SentryAppIntegratorError( + ValidationError("Missing `value` or `label` in option data for SelectField") + ) choices.append([option["value"], option["label"]]) diff --git a/src/sentry/sentry_apps/installations.py b/src/sentry/sentry_apps/installations.py index 383618749509db..7e05e96144e7c0 100644 --- a/src/sentry/sentry_apps/installations.py +++ b/src/sentry/sentry_apps/installations.py @@ -11,7 +11,7 @@ from sentry.api.serializers import serialize from sentry.constants import INTERNAL_INTEGRATION_TOKEN_COUNT_MAX, SentryAppInstallationStatus from sentry.coreapi import APIUnauthorized -from sentry.exceptions import ApiTokenLimitError +from sentry.exceptions import ApiTokenLimitError, SentryAppError from sentry.models.apiapplication import ApiApplication from sentry.models.apigrant import ApiGrant from sentry.models.apitoken import ApiToken @@ -193,8 +193,10 @@ class SentryAppInstallationNotifier: def run(self) -> None: if self.action not in VALID_ACTIONS: - raise APIUnauthorized( - f"Invalid action '{self.action} for installation notifier for {self.sentry_app}" + raise SentryAppError( + APIUnauthorized( + f"Invalid action '{self.action} for installation notifier for {self.sentry_app}" + ) ) send_and_save_webhook_request(self.sentry_app, self.request) diff --git a/src/sentry/sentry_apps/services/app/impl.py b/src/sentry/sentry_apps/services/app/impl.py index e266283516b35e..2de7e92d25fcf3 100644 --- a/src/sentry/sentry_apps/services/app/impl.py +++ b/src/sentry/sentry_apps/services/app/impl.py @@ -251,10 +251,7 @@ def get_installation_token(self, *, organization_id: int, provider: str) -> str def trigger_sentry_app_action_creators( self, *, fields: list[Mapping[str, Any]], install_uuid: str | None ) -> RpcAlertRuleActionResult: - try: - install = SentryAppInstallation.objects.get(uuid=install_uuid) - except SentryAppInstallation.DoesNotExist: - return RpcAlertRuleActionResult(success=False, message="Installation does not exist") + install = SentryAppInstallation.objects.get(uuid=install_uuid) result = AlertRuleActionCreator(install=install, fields=fields).run() return RpcAlertRuleActionResult(success=result["success"], message=result["message"]) From 55455c8170bd9576884ac06761cd6d51055b86f0 Mon Sep 17 00:00:00 2001 From: Christinarlong Date: Mon, 9 Dec 2024 12:14:42 -0800 Subject: [PATCH 04/13] refactor alert rule creation for sentry apps --- .../api/endpoints/project_rule_details.py | 25 ++++----------- src/sentry/api/endpoints/project_rules.py | 31 +++++++++++++++++-- .../organization_alert_rule_details.py | 23 +++++++++++++- .../organization_alert_rule_index.py | 6 ++-- .../sentry_apps/alert_rule_action_creator.py | 12 +++---- .../alert_rule_action_requester.py | 12 +++---- src/sentry/sentry_apps/services/app/impl.py | 6 ++-- .../sentry_apps/services/app/service.py | 3 +- 8 files changed, 73 insertions(+), 45 deletions(-) diff --git a/src/sentry/api/endpoints/project_rule_details.py b/src/sentry/api/endpoints/project_rule_details.py index 76825d691548f8..89906024bc9cde 100644 --- a/src/sentry/api/endpoints/project_rule_details.py +++ b/src/sentry/api/endpoints/project_rule_details.py @@ -2,7 +2,6 @@ import logging -import sentry_sdk from drf_spectacular.utils import extend_schema from rest_framework import serializers, status from rest_framework.request import Request @@ -12,7 +11,11 @@ from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import region_silo_endpoint from sentry.api.bases.rule import RuleEndpoint -from sentry.api.endpoints.project_rules import find_duplicate_rule, send_confirmation_notification +from sentry.api.endpoints.project_rules import ( + create_sentry_app_alert_rule_issues_component, + find_duplicate_rule, + send_confirmation_notification, +) from sentry.api.fields.actor import ActorField from sentry.api.serializers import serialize from sentry.api.serializers.models.rule import RuleSerializer @@ -28,14 +31,12 @@ from sentry.apidocs.parameters import GlobalParams, IssueAlertParams from sentry.constants import ObjectStatus from sentry.deletions.models.scheduleddeletion import RegionScheduledDeletion -from sentry.exceptions import SentryAppIntegratorError from sentry.integrations.jira.actions.create_ticket import JiraCreateTicketAction from sentry.integrations.jira_server.actions.create_ticket import JiraServerCreateTicketAction from sentry.integrations.slack.tasks.find_channel_id_for_rule import find_channel_id_for_rule from sentry.integrations.slack.utils.rule_status import RedisRuleStatus from sentry.models.rule import NeglectedRule, RuleActivity, RuleActivityType from sentry.projects.project_rules.updater import ProjectRuleUpdater -from sentry.rules.actions import trigger_sentry_app_action_creators_for_issues from sentry.rules.actions.utils import get_changed_data, get_updated_rule_data from sentry.signals import alert_rule_edited from sentry.types.actor import Actor @@ -287,21 +288,7 @@ def put(self, request: Request, project, rule) -> Response: context = {"uuid": client.uuid} return Response(context, status=202) - try: - trigger_sentry_app_action_creators_for_issues(actions=kwargs["actions"]) - except SentryAppIntegratorError as e: - return Response( - {"error": str(e)}, - status=status.HTTP_400_BAD_REQUEST, - ) - except Exception as e: - error_id = sentry_sdk.capture_exception(e) - return Response( - { - "error": f"Something went wrong while trying to create alert rule action. Sentry error ID: {error_id}" - }, - status=500, - ) + create_sentry_app_alert_rule_issues_component(actions=kwargs["actions"]) updated_rule = ProjectRuleUpdater( rule=rule, diff --git a/src/sentry/api/endpoints/project_rules.py b/src/sentry/api/endpoints/project_rules.py index a7b86baf086b1f..efd6ee61ea9129 100644 --- a/src/sentry/api/endpoints/project_rules.py +++ b/src/sentry/api/endpoints/project_rules.py @@ -1,7 +1,8 @@ -from collections.abc import Callable +from collections.abc import Callable, Mapping, Sequence from dataclasses import dataclass from typing import Any, Literal +import sentry_sdk from django.conf import settings from django.db.models.signals import pre_save from django.dispatch import receiver @@ -25,6 +26,7 @@ from sentry.apidocs.parameters import GlobalParams from sentry.apidocs.utils import inline_sentry_response_serializer from sentry.constants import ObjectStatus +from sentry.exceptions import SentryAppError, SentryAppIntegratorError from sentry.integrations.slack.tasks.find_channel_id_for_rule import find_channel_id_for_rule from sentry.integrations.slack.utils.rule_status import RedisRuleStatus from sentry.models.rule import Rule, RuleActivity, RuleActivityType @@ -52,6 +54,29 @@ def clean_rule_data(data): del datum["name"] +def create_sentry_app_alert_rule_issues_component( + actions: Sequence[Mapping[str, Any]] +) -> str | None: + try: + created = trigger_sentry_app_action_creators_for_issues(actions) + except (SentryAppError, SentryAppIntegratorError) as e: + return Response( + {"actions": [str(e)]}, + status=status.HTTP_400_BAD_REQUEST, + ) + except Exception as e: + error_id = sentry_sdk.capture_exception(e) + return Response( + { + "actions": [ + f"Something went wrong while trying to create alert rule action. Sentry error ID: {error_id}" + ] + }, + status=500, + ) + return created + + @receiver(pre_save, sender=Rule) def pre_save_rule(instance, sender, *args, **kwargs): clean_rule_data(instance.data.get("conditions", [])) @@ -841,8 +866,8 @@ def post(self, request: Request, project) -> Response: find_channel_id_for_rule.apply_async(kwargs=kwargs) return Response(uuid_context, status=202) - created_alert_rule_ui_component = trigger_sentry_app_action_creators_for_issues( - kwargs["actions"] + created_alert_rule_ui_component = create_sentry_app_alert_rule_issues_component( + actions=kwargs["actions"] ) rule = ProjectRuleCreator( name=kwargs["name"], diff --git a/src/sentry/incidents/endpoints/organization_alert_rule_details.py b/src/sentry/incidents/endpoints/organization_alert_rule_details.py index 0aeea3a1fb0df0..9c8b42f5529d44 100644 --- a/src/sentry/incidents/endpoints/organization_alert_rule_details.py +++ b/src/sentry/incidents/endpoints/organization_alert_rule_details.py @@ -1,3 +1,7 @@ +from collections.abc import Mapping +from typing import Any + +import sentry_sdk from django.db.models import Q from drf_spectacular.utils import extend_schema, extend_schema_serializer from rest_framework import serializers, status @@ -18,6 +22,7 @@ ) from sentry.apidocs.examples.metric_alert_examples import MetricAlertExamples from sentry.apidocs.parameters import GlobalParams, MetricAlertParams +from sentry.exceptions import SentryAppError, SentryAppIntegratorError from sentry.incidents.endpoints.bases import OrganizationAlertRuleEndpoint from sentry.incidents.endpoints.serializers.alert_rule import ( AlertRuleSerializer, @@ -82,7 +87,7 @@ def update_alert_rule(request: Request, organization, alert_rule): partial=True, ) if serializer.is_valid(): - trigger_sentry_app_action_creators_for_incidents(serializer.validated_data) + create_sentry_app_alert_rule_component(serializer.validated_data) if get_slack_actions_with_async_lookups(organization, request.user, data): # need to kick off an async job for Slack client = RedisRuleStatus() @@ -111,6 +116,22 @@ def remove_alert_rule(request: Request, organization, alert_rule): return Response("This rule has already been deleted", status=status.HTTP_400_BAD_REQUEST) +def create_sentry_app_alert_rule_component(serialized_data: Mapping[str, Any]) -> None: + try: + trigger_sentry_app_action_creators_for_incidents(serialized_data) + except (SentryAppError, SentryAppIntegratorError) as e: + return Response( + str(e), + status=status.HTTP_400_BAD_REQUEST, + ) + except Exception as e: + error_id = sentry_sdk.capture_exception(e) + return Response( + f"Something went wrong while trying to create alert rule action. Sentry error ID: {error_id}", + status=500, + ) + + @extend_schema_serializer(exclude_fields=["excludedProjects", "thresholdPeriod"]) class OrganizationAlertRuleDetailsPutSerializer(serializers.Serializer): name = serializers.CharField(max_length=256, help_text="The name for the rule.") diff --git a/src/sentry/incidents/endpoints/organization_alert_rule_index.py b/src/sentry/incidents/endpoints/organization_alert_rule_index.py index db45347717d093..def593ffc93343 100644 --- a/src/sentry/incidents/endpoints/organization_alert_rule_index.py +++ b/src/sentry/incidents/endpoints/organization_alert_rule_index.py @@ -31,6 +31,9 @@ from sentry.apidocs.utils import inline_sentry_response_serializer from sentry.constants import ObjectStatus from sentry.exceptions import InvalidParams +from sentry.incidents.endpoints.organization_alert_rule_details import ( + create_sentry_app_alert_rule_component, +) from sentry.incidents.endpoints.serializers.alert_rule import ( AlertRuleSerializer, AlertRuleSerializerResponse, @@ -41,7 +44,6 @@ from sentry.incidents.models.alert_rule import AlertRule from sentry.incidents.models.incident import Incident, IncidentStatus from sentry.incidents.serializers import AlertRuleSerializer as DrfAlertRuleSerializer -from sentry.incidents.utils.sentry_apps import trigger_sentry_app_action_creators_for_incidents from sentry.integrations.slack.tasks.find_channel_id_for_alert_rule import ( find_channel_id_for_alert_rule, ) @@ -120,7 +122,7 @@ def create_metric_alert( if not serializer.is_valid(): raise ValidationError(serializer.errors) - trigger_sentry_app_action_creators_for_incidents(serializer.validated_data) + create_sentry_app_alert_rule_component(serializer.validated_data) if get_slack_actions_with_async_lookups(organization, request.user, request.data): # need to kick off an async job for Slack client = RedisRuleStatus() diff --git a/src/sentry/sentry_apps/alert_rule_action_creator.py b/src/sentry/sentry_apps/alert_rule_action_creator.py index 4ac0779be6bd12..efba993c77b0b0 100644 --- a/src/sentry/sentry_apps/alert_rule_action_creator.py +++ b/src/sentry/sentry_apps/alert_rule_action_creator.py @@ -9,7 +9,6 @@ from sentry.exceptions import SentryAppIntegratorError from sentry.sentry_apps.external_requests.alert_rule_action_requester import ( AlertRuleActionRequester, - AlertRuleActionResult, ) from sentry.sentry_apps.models.sentry_app_component import SentryAppComponent from sentry.sentry_apps.models.sentry_app_installation import SentryAppInstallation @@ -20,11 +19,10 @@ class AlertRuleActionCreator: install: SentryAppInstallation fields: list[Mapping[str, Any]] = field(default_factory=list) - def run(self) -> AlertRuleActionResult: + def run(self) -> None: with transaction.atomic(router.db_for_write(SentryAppComponent)): uri = self._fetch_sentry_app_uri() - response = self._make_external_request(uri) - return response + self._make_external_request(uri) def _fetch_sentry_app_uri(self): component = SentryAppComponent.objects.get( @@ -33,17 +31,15 @@ def _fetch_sentry_app_uri(self): settings = component.schema.get("settings", {}) return settings.get("uri") - def _make_external_request(self, uri=None): + def _make_external_request(self, uri=None) -> None: if uri is None: raise SentryAppIntegratorError(APIError("Sentry App request url not found")) - response = AlertRuleActionRequester( + AlertRuleActionRequester( install=self.install, uri=uri, fields=self.fields, ).run() - return response - @cached_property def sentry_app(self): return self.install.sentry_app diff --git a/src/sentry/sentry_apps/external_requests/alert_rule_action_requester.py b/src/sentry/sentry_apps/external_requests/alert_rule_action_requester.py index e0d7d19ac04914..bc84dd693be9f1 100644 --- a/src/sentry/sentry_apps/external_requests/alert_rule_action_requester.py +++ b/src/sentry/sentry_apps/external_requests/alert_rule_action_requester.py @@ -14,7 +14,7 @@ from sentry.sentry_apps.services.app.model import RpcSentryAppInstallation from sentry.utils import json -DEFAULT_SUCCESS_MESSAGE = "Success!" +DEFAULT_ERROR_MESSAGE = "Something went wrong while setting up alert for" logger = logging.getLogger("sentry.sentry_apps.external_requests") @@ -31,7 +31,7 @@ class AlertRuleActionRequester: fields: Sequence[Mapping[str, str]] = field(default_factory=list) http_method: str | None = "POST" - def run(self) -> AlertRuleActionResult: + def run(self) -> None: try: response = send_and_save_sentry_app_request( url=self._build_url(), @@ -51,15 +51,15 @@ def run(self) -> AlertRuleActionResult: "install_uuid": self.install.uuid, "uri": self.uri, "error_message": str(e), + "response": str(json.loads(response)), }, ) raise SentryAppIntegratorError( - f"Something went wrong while setting up alert for {self.sentry_app.slug}" + self._get_response_message( + e.response, f"{DEFAULT_ERROR_MESSAGE} {self.sentry_app.slug}" + ) ) from e - return AlertRuleActionResult( - success=True, message=self._get_response_message(response, DEFAULT_SUCCESS_MESSAGE) - ) def _build_url(self) -> str: urlparts = list(urlparse(self.sentry_app.webhook_url)) diff --git a/src/sentry/sentry_apps/services/app/impl.py b/src/sentry/sentry_apps/services/app/impl.py index 2de7e92d25fcf3..c6b575b262e88c 100644 --- a/src/sentry/sentry_apps/services/app/impl.py +++ b/src/sentry/sentry_apps/services/app/impl.py @@ -24,7 +24,6 @@ from sentry.sentry_apps.models.sentry_app_installation_token import SentryAppInstallationToken from sentry.sentry_apps.services.app import ( AppService, - RpcAlertRuleActionResult, RpcSentryApp, RpcSentryAppComponent, RpcSentryAppComponentContext, @@ -250,10 +249,9 @@ def get_installation_token(self, *, organization_id: int, provider: str) -> str def trigger_sentry_app_action_creators( self, *, fields: list[Mapping[str, Any]], install_uuid: str | None - ) -> RpcAlertRuleActionResult: + ) -> None: install = SentryAppInstallation.objects.get(uuid=install_uuid) - result = AlertRuleActionCreator(install=install, fields=fields).run() - return RpcAlertRuleActionResult(success=result["success"], message=result["message"]) + AlertRuleActionCreator(install=install, fields=fields).run() def find_service_hook_sentry_app(self, *, api_application_id: int) -> RpcSentryApp | None: try: diff --git a/src/sentry/sentry_apps/services/app/service.py b/src/sentry/sentry_apps/services/app/service.py index 7d8d8f466fcc60..d95756cc20a5a8 100644 --- a/src/sentry/sentry_apps/services/app/service.py +++ b/src/sentry/sentry_apps/services/app/service.py @@ -12,7 +12,6 @@ from sentry.hybridcloud.rpc.filter_query import OpaqueSerializedResponse from sentry.hybridcloud.rpc.service import RpcService, rpc_method from sentry.sentry_apps.services.app import ( - RpcAlertRuleActionResult, RpcSentryApp, RpcSentryAppComponent, RpcSentryAppEventData, @@ -166,7 +165,7 @@ def get_component_contexts( @abc.abstractmethod def trigger_sentry_app_action_creators( self, *, fields: list[Mapping[str, Any]], install_uuid: str | None - ) -> RpcAlertRuleActionResult: + ) -> None: pass @rpc_method From c20492fee9466c7cc8e95c23ef1458934e746d5c Mon Sep 17 00:00:00 2001 From: Christinarlong Date: Tue, 10 Dec 2024 14:38:48 -0800 Subject: [PATCH 05/13] dawg idk --- src/sentry/exceptions.py | 6 ++- src/sentry/incidents/utils/sentry_apps.py | 4 +- .../alert_rule_action_requester.py | 38 +++++++++++++------ src/sentry/sentry_apps/services/app/impl.py | 28 ++++++++++++-- src/sentry/sentry_apps/services/app/model.py | 2 + .../sentry_apps/services/app/service.py | 7 ++-- 6 files changed, 64 insertions(+), 21 deletions(-) diff --git a/src/sentry/exceptions.py b/src/sentry/exceptions.py index d4bbe4010443bb..e5ef9082d6be34 100644 --- a/src/sentry/exceptions.py +++ b/src/sentry/exceptions.py @@ -1,5 +1,7 @@ from django.core.exceptions import SuspiciousOperation +from sentry.sentry_apps.utils.errors import SentryAppErrorType + class InvalidData(Exception): pass @@ -95,9 +97,9 @@ class InvalidParams(Exception): # Represents a user/client error that occured during a Sentry App process class SentryAppError(Exception): - pass + error_type = SentryAppErrorType.CLIENT # Represents an error caused by a 3p integrator during a Sentry App process class SentryAppIntegratorError(Exception): - pass + error_type = SentryAppErrorType.INTEGRATOR diff --git a/src/sentry/incidents/utils/sentry_apps.py b/src/sentry/incidents/utils/sentry_apps.py index 01c262ec3676f2..e7fd63be4f094f 100644 --- a/src/sentry/incidents/utils/sentry_apps.py +++ b/src/sentry/incidents/utils/sentry_apps.py @@ -28,7 +28,9 @@ def trigger_sentry_app_action_creators_for_incidents(alert_rule_data: Mapping[st if not action_serializer.is_valid(): raise serializers.ValidationError(action_serializer.errors) - app_service.trigger_sentry_app_action_creators( + result = app_service.trigger_sentry_app_action_creators( fields=action.get("sentry_app_config"), install_uuid=action.get("sentry_app_installation_uuid"), ) + if not result.success: + raise serializers.ValidationError({"sentry_app": result.message}) diff --git a/src/sentry/sentry_apps/external_requests/alert_rule_action_requester.py b/src/sentry/sentry_apps/external_requests/alert_rule_action_requester.py index bc84dd693be9f1..f4993b999d503c 100644 --- a/src/sentry/sentry_apps/external_requests/alert_rule_action_requester.py +++ b/src/sentry/sentry_apps/external_requests/alert_rule_action_requester.py @@ -6,8 +6,10 @@ from uuid import uuid4 from django.utils.functional import cached_property +from requests.exceptions import RequestException from requests.models import Response +from sentry.coreapi import APIError from sentry.exceptions import SentryAppIntegratorError from sentry.sentry_apps.external_requests.utils import send_and_save_sentry_app_request from sentry.sentry_apps.models.sentry_app_installation import SentryAppInstallation @@ -42,25 +44,26 @@ def run(self) -> None: method=self.http_method, data=self.body, ) + except RequestException as e: + self._log_exceptions(e) + raise SentryAppIntegratorError( + self._get_response_message( + e.response, f"{DEFAULT_ERROR_MESSAGE} {self.sentry_app.slug}" + ) + ) from e except Exception as e: - logger.info( - "alert_rule_action.error", - extra={ - "sentry_app_slug": self.sentry_app.slug, - "install_uuid": self.install.uuid, - "uri": self.uri, - "error_message": str(e), - "response": str(json.loads(response)), - }, - ) - - raise SentryAppIntegratorError( + self._log_exceptions(e) + raise APIError( self._get_response_message( e.response, f"{DEFAULT_ERROR_MESSAGE} {self.sentry_app.slug}" ) ) from e + return AlertRuleActionResult( + success=True, message=self._get_response_message(response, "poggers") + ) + def _build_url(self) -> str: urlparts = list(urlparse(self.sentry_app.webhook_url)) urlparts[2] = self.uri @@ -91,6 +94,17 @@ def _get_response_message(self, response: Response | None, default_message: str) return f"{self.sentry_app.name}: {message}" + def _log_exceptions(self, error: Exception) -> None: + logger.info( + "alert_rule_action.error", + extra={ + "sentry_app_slug": self.sentry_app.slug, + "install_uuid": self.install.uuid, + "uri": self.uri, + "error_message": str(error), + }, + ) + @cached_property def body(self): return json.dumps( diff --git a/src/sentry/sentry_apps/services/app/impl.py b/src/sentry/sentry_apps/services/app/impl.py index c6b575b262e88c..bbac3a215b768c 100644 --- a/src/sentry/sentry_apps/services/app/impl.py +++ b/src/sentry/sentry_apps/services/app/impl.py @@ -9,6 +9,7 @@ from sentry.api.serializers import Serializer, serialize from sentry.auth.services.auth import AuthenticationContext from sentry.constants import SentryAppInstallationStatus, SentryAppStatus +from sentry.exceptions import SentryAppError, SentryAppIntegratorError from sentry.hybridcloud.rpc.filter_query import FilterQueryDatabaseImpl, OpaqueSerializedResponse from sentry.sentry_apps.alert_rule_action_creator import AlertRuleActionCreator from sentry.sentry_apps.api.serializers.sentry_app_component import ( @@ -32,11 +33,13 @@ RpcSentryAppService, SentryAppInstallationFilterArgs, ) +from sentry.sentry_apps.services.app.model import RpcAlertRuleActionResult from sentry.sentry_apps.services.app.serial import ( serialize_sentry_app, serialize_sentry_app_component, serialize_sentry_app_installation, ) +from sentry.sentry_apps.utils.errors import SentryAppErrorType from sentry.users.models.user import User from sentry.users.services.user import RpcUser @@ -249,9 +252,28 @@ def get_installation_token(self, *, organization_id: int, provider: str) -> str def trigger_sentry_app_action_creators( self, *, fields: list[Mapping[str, Any]], install_uuid: str | None - ) -> None: - install = SentryAppInstallation.objects.get(uuid=install_uuid) - AlertRuleActionCreator(install=install, fields=fields).run() + ) -> RpcAlertRuleActionResult: + try: + install = SentryAppInstallation.objects.get(uuid=install_uuid) + except SentryAppInstallation.DoesNotExist: + return RpcAlertRuleActionResult( + success=False, + error_type=SentryAppErrorType.SENTRY, + message="Installation does not exist", + ) + + try: + result = AlertRuleActionCreator(install=install, fields=fields).run() + except (SentryAppError, SentryAppIntegratorError) as e: + return RpcAlertRuleActionResult(success=False, error_type=e.error_type, message=str(e)) + except Exception as e: + return RpcAlertRuleActionResult( + success=False, + error_type=SentryAppErrorType.SENTRY, + message=str(e), + ) + + return RpcAlertRuleActionResult(success=result["success"], message=result["message"]) def find_service_hook_sentry_app(self, *, api_application_id: int) -> RpcSentryApp | None: try: diff --git a/src/sentry/sentry_apps/services/app/model.py b/src/sentry/sentry_apps/services/app/model.py index 20609618b50841..753a6f75b018d6 100644 --- a/src/sentry/sentry_apps/services/app/model.py +++ b/src/sentry/sentry_apps/services/app/model.py @@ -14,6 +14,7 @@ from sentry.constants import SentryAppInstallationStatus from sentry.hybridcloud.rpc import RpcModel, RpcModelProtocolMeta +from sentry.sentry_apps.utils.errors import SentryAppErrorType class RpcApiApplication(RpcModel): @@ -100,6 +101,7 @@ class RpcSentryAppComponentContext(RpcModel): class RpcAlertRuleActionResult(RpcModel): success: bool message: str + error_type: SentryAppErrorType class SentryAppEventDataInterface(Protocol): diff --git a/src/sentry/sentry_apps/services/app/service.py b/src/sentry/sentry_apps/services/app/service.py index d95756cc20a5a8..757e0ca41094fa 100644 --- a/src/sentry/sentry_apps/services/app/service.py +++ b/src/sentry/sentry_apps/services/app/service.py @@ -11,15 +11,16 @@ from sentry.hybridcloud.rpc.caching.service import back_with_silo_cache, back_with_silo_cache_list from sentry.hybridcloud.rpc.filter_query import OpaqueSerializedResponse from sentry.hybridcloud.rpc.service import RpcService, rpc_method -from sentry.sentry_apps.services.app import ( +from sentry.sentry_apps.services.app.model import ( + RpcAlertRuleActionResult, RpcSentryApp, RpcSentryAppComponent, + RpcSentryAppComponentContext, RpcSentryAppEventData, RpcSentryAppInstallation, RpcSentryAppService, SentryAppInstallationFilterArgs, ) -from sentry.sentry_apps.services.app.model import RpcSentryAppComponentContext from sentry.silo.base import SiloMode from sentry.users.services.user import RpcUser @@ -165,7 +166,7 @@ def get_component_contexts( @abc.abstractmethod def trigger_sentry_app_action_creators( self, *, fields: list[Mapping[str, Any]], install_uuid: str | None - ) -> None: + ) -> RpcAlertRuleActionResult: pass @rpc_method From 22e4ed9de988c9c35113d02f6689a263f76cb1ff Mon Sep 17 00:00:00 2001 From: Christinarlong Date: Tue, 10 Dec 2024 14:39:40 -0800 Subject: [PATCH 06/13] dawg idk --- src/sentry/sentry_apps/utils/errors.py | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 src/sentry/sentry_apps/utils/errors.py diff --git a/src/sentry/sentry_apps/utils/errors.py b/src/sentry/sentry_apps/utils/errors.py new file mode 100644 index 00000000000000..9c1fe0b62f1641 --- /dev/null +++ b/src/sentry/sentry_apps/utils/errors.py @@ -0,0 +1,7 @@ +from enum import Enum + + +class SentryAppErrorType(Enum): + CLIENT = "client" + INTEGRATOR = "integrator" + SENTRY = "sentry" From 50e427f11d65f24eccc10cde0e601539dce72e27 Mon Sep 17 00:00:00 2001 From: Christinarlong Date: Wed, 11 Dec 2024 09:51:56 -0800 Subject: [PATCH 07/13] switch back to alertruleactionresult --- .../sentry_apps/alert_rule_action_creator.py | 19 ++++++++++++------- .../alert_rule_action_requester.py | 15 +++++++-------- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/src/sentry/sentry_apps/alert_rule_action_creator.py b/src/sentry/sentry_apps/alert_rule_action_creator.py index efba993c77b0b0..5be1c9ea1868fb 100644 --- a/src/sentry/sentry_apps/alert_rule_action_creator.py +++ b/src/sentry/sentry_apps/alert_rule_action_creator.py @@ -9,7 +9,9 @@ from sentry.exceptions import SentryAppIntegratorError from sentry.sentry_apps.external_requests.alert_rule_action_requester import ( AlertRuleActionRequester, + AlertRuleActionResult, ) +from sentry.sentry_apps.models.sentry_app import SentryApp from sentry.sentry_apps.models.sentry_app_component import SentryAppComponent from sentry.sentry_apps.models.sentry_app_installation import SentryAppInstallation @@ -19,27 +21,30 @@ class AlertRuleActionCreator: install: SentryAppInstallation fields: list[Mapping[str, Any]] = field(default_factory=list) - def run(self) -> None: + def run(self) -> AlertRuleActionResult: with transaction.atomic(router.db_for_write(SentryAppComponent)): uri = self._fetch_sentry_app_uri() - self._make_external_request(uri) + response = self._make_external_request(uri) + return response - def _fetch_sentry_app_uri(self): + def _fetch_sentry_app_uri(self) -> str: component = SentryAppComponent.objects.get( type="alert-rule-action", sentry_app=self.sentry_app ) settings = component.schema.get("settings", {}) return settings.get("uri") - def _make_external_request(self, uri=None) -> None: + def _make_external_request(self, uri: str = None) -> AlertRuleActionResult: if uri is None: - raise SentryAppIntegratorError(APIError("Sentry App request url not found")) - AlertRuleActionRequester( + raise SentryAppIntegratorError(APIError("Sentry App request url not found on schema")) + response = AlertRuleActionRequester( install=self.install, uri=uri, fields=self.fields, ).run() + return response + @cached_property - def sentry_app(self): + def sentry_app(self) -> SentryApp: return self.install.sentry_app diff --git a/src/sentry/sentry_apps/external_requests/alert_rule_action_requester.py b/src/sentry/sentry_apps/external_requests/alert_rule_action_requester.py index f4993b999d503c..afc339f61dd309 100644 --- a/src/sentry/sentry_apps/external_requests/alert_rule_action_requester.py +++ b/src/sentry/sentry_apps/external_requests/alert_rule_action_requester.py @@ -9,13 +9,14 @@ from requests.exceptions import RequestException from requests.models import Response -from sentry.coreapi import APIError from sentry.exceptions import SentryAppIntegratorError from sentry.sentry_apps.external_requests.utils import send_and_save_sentry_app_request from sentry.sentry_apps.models.sentry_app_installation import SentryAppInstallation from sentry.sentry_apps.services.app.model import RpcSentryAppInstallation +from sentry.sentry_apps.utils.errors import SentryAppErrorType from sentry.utils import json +DEFAULT_SUCCESS_MESSAGE = "Success!" DEFAULT_ERROR_MESSAGE = "Something went wrong while setting up alert for" logger = logging.getLogger("sentry.sentry_apps.external_requests") @@ -24,6 +25,7 @@ class AlertRuleActionResult(TypedDict): success: bool message: str + error_type: SentryAppErrorType | None = None @dataclass @@ -33,7 +35,7 @@ class AlertRuleActionRequester: fields: Sequence[Mapping[str, str]] = field(default_factory=list) http_method: str | None = "POST" - def run(self) -> None: + def run(self) -> AlertRuleActionResult: try: response = send_and_save_sentry_app_request( url=self._build_url(), @@ -54,14 +56,11 @@ def run(self) -> None: except Exception as e: self._log_exceptions(e) - raise APIError( - self._get_response_message( - e.response, f"{DEFAULT_ERROR_MESSAGE} {self.sentry_app.slug}" - ) - ) from e + e.args = (f"{DEFAULT_ERROR_MESSAGE} {self.sentry_app.slug}",) + raise return AlertRuleActionResult( - success=True, message=self._get_response_message(response, "poggers") + success=True, message=self._get_response_message(response, DEFAULT_SUCCESS_MESSAGE) ) def _build_url(self) -> str: From c16edfdfb9aff7f3a126480ea4cdc01f6ec320f3 Mon Sep 17 00:00:00 2001 From: Christinarlong Date: Wed, 11 Dec 2024 13:13:47 -0800 Subject: [PATCH 08/13] update arc process to return alertruleactionresult --- src/sentry/incidents/utils/sentry_apps.py | 4 ++-- src/sentry/rules/actions/sentry_apps/utils.py | 5 +++-- .../alert_rule_action_requester.py | 4 +++- src/sentry/sentry_apps/services/app/impl.py | 4 +++- src/sentry/sentry_apps/services/app/model.py | 2 +- src/sentry/sentry_apps/utils/errors.py | 15 +++++++++++++++ 6 files changed, 27 insertions(+), 7 deletions(-) diff --git a/src/sentry/incidents/utils/sentry_apps.py b/src/sentry/incidents/utils/sentry_apps.py index e7fd63be4f094f..ebfabad4d095bb 100644 --- a/src/sentry/incidents/utils/sentry_apps.py +++ b/src/sentry/incidents/utils/sentry_apps.py @@ -8,6 +8,7 @@ from sentry.incidents.models.alert_rule import AlertRuleTriggerAction from sentry.incidents.serializers import AlertRuleTriggerActionSerializer from sentry.sentry_apps.services.app import app_service +from sentry.sentry_apps.utils.errors import raise_alert_rule_action_result_errors def trigger_sentry_app_action_creators_for_incidents(alert_rule_data: Mapping[str, Any]) -> None: @@ -32,5 +33,4 @@ def trigger_sentry_app_action_creators_for_incidents(alert_rule_data: Mapping[st fields=action.get("sentry_app_config"), install_uuid=action.get("sentry_app_installation_uuid"), ) - if not result.success: - raise serializers.ValidationError({"sentry_app": result.message}) + raise_alert_rule_action_result_errors(result) diff --git a/src/sentry/rules/actions/sentry_apps/utils.py b/src/sentry/rules/actions/sentry_apps/utils.py index cef2685597323f..54f92f7ea17241 100644 --- a/src/sentry/rules/actions/sentry_apps/utils.py +++ b/src/sentry/rules/actions/sentry_apps/utils.py @@ -5,6 +5,7 @@ from sentry.constants import SENTRY_APP_ACTIONS from sentry.sentry_apps.services.app import app_service +from sentry.sentry_apps.utils.errors import raise_alert_rule_action_result_errors def trigger_sentry_app_action_creators_for_issues( @@ -16,9 +17,9 @@ def trigger_sentry_app_action_creators_for_issues( if not action.get("id") in SENTRY_APP_ACTIONS: continue - app_service.trigger_sentry_app_action_creators( + result = app_service.trigger_sentry_app_action_creators( fields=action["settings"], install_uuid=action.get("sentryAppInstallationUuid") ) - + raise_alert_rule_action_result_errors(result=result) created = "alert-rule-action" return created diff --git a/src/sentry/sentry_apps/external_requests/alert_rule_action_requester.py b/src/sentry/sentry_apps/external_requests/alert_rule_action_requester.py index afc339f61dd309..771ebfb1570495 100644 --- a/src/sentry/sentry_apps/external_requests/alert_rule_action_requester.py +++ b/src/sentry/sentry_apps/external_requests/alert_rule_action_requester.py @@ -60,7 +60,9 @@ def run(self) -> AlertRuleActionResult: raise return AlertRuleActionResult( - success=True, message=self._get_response_message(response, DEFAULT_SUCCESS_MESSAGE) + success=True, + error_type=None, + message=self._get_response_message(response, DEFAULT_SUCCESS_MESSAGE), ) def _build_url(self) -> str: diff --git a/src/sentry/sentry_apps/services/app/impl.py b/src/sentry/sentry_apps/services/app/impl.py index bbac3a215b768c..624040e624d403 100644 --- a/src/sentry/sentry_apps/services/app/impl.py +++ b/src/sentry/sentry_apps/services/app/impl.py @@ -273,7 +273,9 @@ def trigger_sentry_app_action_creators( message=str(e), ) - return RpcAlertRuleActionResult(success=result["success"], message=result["message"]) + return RpcAlertRuleActionResult( + success=result["success"], error_type=None, message=result["message"] + ) def find_service_hook_sentry_app(self, *, api_application_id: int) -> RpcSentryApp | None: try: diff --git a/src/sentry/sentry_apps/services/app/model.py b/src/sentry/sentry_apps/services/app/model.py index 753a6f75b018d6..51aa759bf38306 100644 --- a/src/sentry/sentry_apps/services/app/model.py +++ b/src/sentry/sentry_apps/services/app/model.py @@ -101,7 +101,7 @@ class RpcSentryAppComponentContext(RpcModel): class RpcAlertRuleActionResult(RpcModel): success: bool message: str - error_type: SentryAppErrorType + error_type: SentryAppErrorType | None class SentryAppEventDataInterface(Protocol): diff --git a/src/sentry/sentry_apps/utils/errors.py b/src/sentry/sentry_apps/utils/errors.py index 9c1fe0b62f1641..0787677585102d 100644 --- a/src/sentry/sentry_apps/utils/errors.py +++ b/src/sentry/sentry_apps/utils/errors.py @@ -1,7 +1,22 @@ from enum import Enum +from sentry.exceptions import SentryAppError, SentryAppIntegratorError +from sentry.sentry_apps.services.app.model import RpcAlertRuleActionResult + class SentryAppErrorType(Enum): CLIENT = "client" INTEGRATOR = "integrator" SENTRY = "sentry" + + +def raise_alert_rule_action_result_errors(result: RpcAlertRuleActionResult) -> None: + match result.error_type: + case SentryAppErrorType.INTEGRATOR: + raise SentryAppIntegratorError(result.message) + case SentryAppErrorType.CLIENT: + raise SentryAppError(result.message) + case SentryAppErrorType.SENTRY: + raise Exception(result.message) + + return None From f6aaca7362e1553c837d32ae1cf3415ff5a3221f Mon Sep 17 00:00:00 2001 From: Christinarlong Date: Wed, 11 Dec 2024 13:43:33 -0800 Subject: [PATCH 09/13] update refs and arar tests --- src/sentry/api/endpoints/project_rules.py | 2 +- src/sentry/exceptions.py | 12 ----- .../organization_alert_rule_details.py | 2 +- src/sentry/incidents/utils/sentry_apps.py | 2 +- src/sentry/rules/actions/sentry_apps/utils.py | 2 +- .../installation_external_issue_actions.py | 2 +- .../installation_external_requests.py | 2 +- .../external_issues/issue_link_creator.py | 2 +- .../alert_rule_action_requester.py | 3 +- .../external_requests/issue_link_requester.py | 2 +- .../external_requests/select_requester.py | 2 +- src/sentry/sentry_apps/installations.py | 3 +- src/sentry/sentry_apps/services/app/impl.py | 7 ++- src/sentry/sentry_apps/services/app/model.py | 3 +- .../sentry_apps/utils/alert_rule_action.py | 18 +++++++ src/sentry/sentry_apps/utils/errors.py | 19 +++----- .../test_alert_rule_action_requester.py | 47 ++++++++++--------- 17 files changed, 70 insertions(+), 60 deletions(-) create mode 100644 src/sentry/sentry_apps/utils/alert_rule_action.py diff --git a/src/sentry/api/endpoints/project_rules.py b/src/sentry/api/endpoints/project_rules.py index efd6ee61ea9129..21b5876fa73568 100644 --- a/src/sentry/api/endpoints/project_rules.py +++ b/src/sentry/api/endpoints/project_rules.py @@ -26,7 +26,6 @@ from sentry.apidocs.parameters import GlobalParams from sentry.apidocs.utils import inline_sentry_response_serializer from sentry.constants import ObjectStatus -from sentry.exceptions import SentryAppError, SentryAppIntegratorError from sentry.integrations.slack.tasks.find_channel_id_for_rule import find_channel_id_for_rule from sentry.integrations.slack.utils.rule_status import RedisRuleStatus from sentry.models.rule import Rule, RuleActivity, RuleActivityType @@ -34,6 +33,7 @@ from sentry.rules.actions import trigger_sentry_app_action_creators_for_issues from sentry.rules.actions.base import instantiate_action from sentry.rules.processing.processor import is_condition_slow +from sentry.sentry_apps.utils.errors import SentryAppError, SentryAppIntegratorError from sentry.signals import alert_rule_created from sentry.utils import metrics diff --git a/src/sentry/exceptions.py b/src/sentry/exceptions.py index e5ef9082d6be34..1c162069237a05 100644 --- a/src/sentry/exceptions.py +++ b/src/sentry/exceptions.py @@ -1,7 +1,5 @@ from django.core.exceptions import SuspiciousOperation -from sentry.sentry_apps.utils.errors import SentryAppErrorType - class InvalidData(Exception): pass @@ -93,13 +91,3 @@ def __init__( class InvalidParams(Exception): pass - - -# Represents a user/client error that occured during a Sentry App process -class SentryAppError(Exception): - error_type = SentryAppErrorType.CLIENT - - -# Represents an error caused by a 3p integrator during a Sentry App process -class SentryAppIntegratorError(Exception): - error_type = SentryAppErrorType.INTEGRATOR diff --git a/src/sentry/incidents/endpoints/organization_alert_rule_details.py b/src/sentry/incidents/endpoints/organization_alert_rule_details.py index 9c8b42f5529d44..1ea443fca8bbef 100644 --- a/src/sentry/incidents/endpoints/organization_alert_rule_details.py +++ b/src/sentry/incidents/endpoints/organization_alert_rule_details.py @@ -22,7 +22,6 @@ ) from sentry.apidocs.examples.metric_alert_examples import MetricAlertExamples from sentry.apidocs.parameters import GlobalParams, MetricAlertParams -from sentry.exceptions import SentryAppError, SentryAppIntegratorError from sentry.incidents.endpoints.bases import OrganizationAlertRuleEndpoint from sentry.incidents.endpoints.serializers.alert_rule import ( AlertRuleSerializer, @@ -41,6 +40,7 @@ from sentry.integrations.slack.utils.rule_status import RedisRuleStatus from sentry.models.rulesnooze import RuleSnooze from sentry.sentry_apps.services.app import app_service +from sentry.sentry_apps.utils.errors import SentryAppError, SentryAppIntegratorError from sentry.users.services.user.service import user_service diff --git a/src/sentry/incidents/utils/sentry_apps.py b/src/sentry/incidents/utils/sentry_apps.py index ebfabad4d095bb..5b849fa8255985 100644 --- a/src/sentry/incidents/utils/sentry_apps.py +++ b/src/sentry/incidents/utils/sentry_apps.py @@ -8,7 +8,7 @@ from sentry.incidents.models.alert_rule import AlertRuleTriggerAction from sentry.incidents.serializers import AlertRuleTriggerActionSerializer from sentry.sentry_apps.services.app import app_service -from sentry.sentry_apps.utils.errors import raise_alert_rule_action_result_errors +from sentry.sentry_apps.utils.alert_rule_action import raise_alert_rule_action_result_errors def trigger_sentry_app_action_creators_for_incidents(alert_rule_data: Mapping[str, Any]) -> None: diff --git a/src/sentry/rules/actions/sentry_apps/utils.py b/src/sentry/rules/actions/sentry_apps/utils.py index 54f92f7ea17241..e98dbaeb9005a5 100644 --- a/src/sentry/rules/actions/sentry_apps/utils.py +++ b/src/sentry/rules/actions/sentry_apps/utils.py @@ -5,7 +5,7 @@ from sentry.constants import SENTRY_APP_ACTIONS from sentry.sentry_apps.services.app import app_service -from sentry.sentry_apps.utils.errors import raise_alert_rule_action_result_errors +from sentry.sentry_apps.utils.alert_rule_action import raise_alert_rule_action_result_errors def trigger_sentry_app_action_creators_for_issues( diff --git a/src/sentry/sentry_apps/api/endpoints/installation_external_issue_actions.py b/src/sentry/sentry_apps/api/endpoints/installation_external_issue_actions.py index 75a64cab0fc228..4b80531450544f 100644 --- a/src/sentry/sentry_apps/api/endpoints/installation_external_issue_actions.py +++ b/src/sentry/sentry_apps/api/endpoints/installation_external_issue_actions.py @@ -7,7 +7,6 @@ from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import region_silo_endpoint from sentry.api.serializers import serialize -from sentry.exceptions import SentryAppError, SentryAppIntegratorError from sentry.models.group import Group from sentry.models.project import Project from sentry.sentry_apps.api.bases.sentryapps import SentryAppInstallationBaseEndpoint @@ -15,6 +14,7 @@ PlatformExternalIssueSerializer, ) from sentry.sentry_apps.external_issues.issue_link_creator import IssueLinkCreator +from sentry.sentry_apps.utils.errors import SentryAppError, SentryAppIntegratorError from sentry.users.models.user import User from sentry.users.services.user.serial import serialize_rpc_user diff --git a/src/sentry/sentry_apps/api/endpoints/installation_external_requests.py b/src/sentry/sentry_apps/api/endpoints/installation_external_requests.py index 57d13c912bf3f8..1dbe7e81f4f2af 100644 --- a/src/sentry/sentry_apps/api/endpoints/installation_external_requests.py +++ b/src/sentry/sentry_apps/api/endpoints/installation_external_requests.py @@ -7,10 +7,10 @@ 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.exceptions import SentryAppError, SentryAppIntegratorError 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 +from sentry.sentry_apps.utils.errors import SentryAppError, SentryAppIntegratorError logger = logging.getLogger("sentry.sentry-apps") diff --git a/src/sentry/sentry_apps/external_issues/issue_link_creator.py b/src/sentry/sentry_apps/external_issues/issue_link_creator.py index 5884cb4deed742..7807e4abf68db8 100644 --- a/src/sentry/sentry_apps/external_issues/issue_link_creator.py +++ b/src/sentry/sentry_apps/external_issues/issue_link_creator.py @@ -4,12 +4,12 @@ from django.db import router, transaction from sentry.coreapi import APIUnauthorized -from sentry.exceptions import SentryAppError from sentry.models.group import Group from sentry.sentry_apps.external_issues.external_issue_creator import ExternalIssueCreator from sentry.sentry_apps.external_requests.issue_link_requester import IssueLinkRequester from sentry.sentry_apps.models.platformexternalissue import PlatformExternalIssue from sentry.sentry_apps.services.app import RpcSentryAppInstallation +from sentry.sentry_apps.utils.errors import SentryAppError from sentry.users.services.user import RpcUser VALID_ACTIONS = ["link", "create"] diff --git a/src/sentry/sentry_apps/external_requests/alert_rule_action_requester.py b/src/sentry/sentry_apps/external_requests/alert_rule_action_requester.py index 771ebfb1570495..d9716fa361174c 100644 --- a/src/sentry/sentry_apps/external_requests/alert_rule_action_requester.py +++ b/src/sentry/sentry_apps/external_requests/alert_rule_action_requester.py @@ -9,11 +9,10 @@ from requests.exceptions import RequestException from requests.models import Response -from sentry.exceptions import SentryAppIntegratorError from sentry.sentry_apps.external_requests.utils import send_and_save_sentry_app_request from sentry.sentry_apps.models.sentry_app_installation import SentryAppInstallation from sentry.sentry_apps.services.app.model import RpcSentryAppInstallation -from sentry.sentry_apps.utils.errors import SentryAppErrorType +from sentry.sentry_apps.utils.errors import SentryAppErrorType, SentryAppIntegratorError from sentry.utils import json DEFAULT_SUCCESS_MESSAGE = "Success!" diff --git a/src/sentry/sentry_apps/external_requests/issue_link_requester.py b/src/sentry/sentry_apps/external_requests/issue_link_requester.py index 61964339ca9512..38a0c5b5672587 100644 --- a/src/sentry/sentry_apps/external_requests/issue_link_requester.py +++ b/src/sentry/sentry_apps/external_requests/issue_link_requester.py @@ -8,11 +8,11 @@ from jsonschema import ValidationError from sentry.coreapi import APIError -from sentry.exceptions import SentryAppIntegratorError from sentry.http import safe_urlread from sentry.models.group import Group from sentry.sentry_apps.external_requests.utils import send_and_save_sentry_app_request, validate from sentry.sentry_apps.services.app import RpcSentryAppInstallation +from sentry.sentry_apps.utils.errors import SentryAppIntegratorError from sentry.users.models.user import User from sentry.users.services.user import RpcUser from sentry.utils import json diff --git a/src/sentry/sentry_apps/external_requests/select_requester.py b/src/sentry/sentry_apps/external_requests/select_requester.py index 8b4eb39d41e12f..2565f403af1412 100644 --- a/src/sentry/sentry_apps/external_requests/select_requester.py +++ b/src/sentry/sentry_apps/external_requests/select_requester.py @@ -9,11 +9,11 @@ from jsonschema import ValidationError from sentry.coreapi import APIError -from sentry.exceptions import SentryAppIntegratorError from sentry.http import safe_urlread from sentry.sentry_apps.external_requests.utils import send_and_save_sentry_app_request, validate from sentry.sentry_apps.services.app import RpcSentryAppInstallation from sentry.sentry_apps.services.app.model import RpcSentryApp +from sentry.sentry_apps.utils.errors import SentryAppIntegratorError from sentry.utils import json logger = logging.getLogger("sentry.sentry_apps.external_requests") diff --git a/src/sentry/sentry_apps/installations.py b/src/sentry/sentry_apps/installations.py index 7e05e96144e7c0..837a4dab9b53e6 100644 --- a/src/sentry/sentry_apps/installations.py +++ b/src/sentry/sentry_apps/installations.py @@ -11,7 +11,7 @@ from sentry.api.serializers import serialize from sentry.constants import INTERNAL_INTEGRATION_TOKEN_COUNT_MAX, SentryAppInstallationStatus from sentry.coreapi import APIUnauthorized -from sentry.exceptions import ApiTokenLimitError, SentryAppError +from sentry.exceptions import ApiTokenLimitError from sentry.models.apiapplication import ApiApplication from sentry.models.apigrant import ApiGrant from sentry.models.apitoken import ApiToken @@ -24,6 +24,7 @@ from sentry.sentry_apps.models.sentry_app_installation_token import SentryAppInstallationToken from sentry.sentry_apps.services.hook import hook_service from sentry.sentry_apps.tasks.sentry_apps import installation_webhook +from sentry.sentry_apps.utils.errors import SentryAppError from sentry.users.models.user import User from sentry.users.services.user.model import RpcUser from sentry.utils import metrics diff --git a/src/sentry/sentry_apps/services/app/impl.py b/src/sentry/sentry_apps/services/app/impl.py index 624040e624d403..d33fbd5845174c 100644 --- a/src/sentry/sentry_apps/services/app/impl.py +++ b/src/sentry/sentry_apps/services/app/impl.py @@ -9,7 +9,6 @@ from sentry.api.serializers import Serializer, serialize from sentry.auth.services.auth import AuthenticationContext from sentry.constants import SentryAppInstallationStatus, SentryAppStatus -from sentry.exceptions import SentryAppError, SentryAppIntegratorError from sentry.hybridcloud.rpc.filter_query import FilterQueryDatabaseImpl, OpaqueSerializedResponse from sentry.sentry_apps.alert_rule_action_creator import AlertRuleActionCreator from sentry.sentry_apps.api.serializers.sentry_app_component import ( @@ -39,7 +38,11 @@ serialize_sentry_app_component, serialize_sentry_app_installation, ) -from sentry.sentry_apps.utils.errors import SentryAppErrorType +from sentry.sentry_apps.utils.errors import ( + SentryAppError, + SentryAppErrorType, + SentryAppIntegratorError, +) from sentry.users.models.user import User from sentry.users.services.user import RpcUser diff --git a/src/sentry/sentry_apps/services/app/model.py b/src/sentry/sentry_apps/services/app/model.py index 51aa759bf38306..3a873a083c5558 100644 --- a/src/sentry/sentry_apps/services/app/model.py +++ b/src/sentry/sentry_apps/services/app/model.py @@ -14,7 +14,6 @@ from sentry.constants import SentryAppInstallationStatus from sentry.hybridcloud.rpc import RpcModel, RpcModelProtocolMeta -from sentry.sentry_apps.utils.errors import SentryAppErrorType class RpcApiApplication(RpcModel): @@ -99,6 +98,8 @@ class RpcSentryAppComponentContext(RpcModel): class RpcAlertRuleActionResult(RpcModel): + from sentry.sentry_apps.utils.errors import SentryAppErrorType + success: bool message: str error_type: SentryAppErrorType | None diff --git a/src/sentry/sentry_apps/utils/alert_rule_action.py b/src/sentry/sentry_apps/utils/alert_rule_action.py new file mode 100644 index 00000000000000..829242ee336a59 --- /dev/null +++ b/src/sentry/sentry_apps/utils/alert_rule_action.py @@ -0,0 +1,18 @@ +from sentry.sentry_apps.services.app.model import RpcAlertRuleActionResult +from sentry.sentry_apps.utils.errors import ( + SentryAppError, + SentryAppErrorType, + SentryAppIntegratorError, +) + + +def raise_alert_rule_action_result_errors(result: RpcAlertRuleActionResult) -> None: + match result.error_type: + case SentryAppErrorType.INTEGRATOR: + raise SentryAppIntegratorError(result.message) + case SentryAppErrorType.CLIENT: + raise SentryAppError(result.message) + case SentryAppErrorType.SENTRY: + raise Exception(result.message) + + return None diff --git a/src/sentry/sentry_apps/utils/errors.py b/src/sentry/sentry_apps/utils/errors.py index 0787677585102d..68aba2dcaba5b1 100644 --- a/src/sentry/sentry_apps/utils/errors.py +++ b/src/sentry/sentry_apps/utils/errors.py @@ -1,8 +1,5 @@ from enum import Enum -from sentry.exceptions import SentryAppError, SentryAppIntegratorError -from sentry.sentry_apps.services.app.model import RpcAlertRuleActionResult - class SentryAppErrorType(Enum): CLIENT = "client" @@ -10,13 +7,11 @@ class SentryAppErrorType(Enum): SENTRY = "sentry" -def raise_alert_rule_action_result_errors(result: RpcAlertRuleActionResult) -> None: - match result.error_type: - case SentryAppErrorType.INTEGRATOR: - raise SentryAppIntegratorError(result.message) - case SentryAppErrorType.CLIENT: - raise SentryAppError(result.message) - case SentryAppErrorType.SENTRY: - raise Exception(result.message) +# Represents a user/client error that occured during a Sentry App process +class SentryAppError(Exception): + error_type = SentryAppErrorType.CLIENT + - return None +# Represents an error caused by a 3p integrator during a Sentry App process +class SentryAppIntegratorError(Exception): + error_type = SentryAppErrorType.INTEGRATOR diff --git a/tests/sentry/sentry_apps/external_requests/test_alert_rule_action_requester.py b/tests/sentry/sentry_apps/external_requests/test_alert_rule_action_requester.py index b728a3d78309b1..c381c781f30408 100644 --- a/tests/sentry/sentry_apps/external_requests/test_alert_rule_action_requester.py +++ b/tests/sentry/sentry_apps/external_requests/test_alert_rule_action_requester.py @@ -1,5 +1,6 @@ from collections.abc import Mapping +import pytest import responses from sentry.sentry_apps.external_requests.alert_rule_action_requester import ( @@ -7,6 +8,7 @@ DEFAULT_SUCCESS_MESSAGE, AlertRuleActionRequester, ) +from sentry.sentry_apps.utils.errors import SentryAppIntegratorError from sentry.testutils.cases import TestCase from sentry.testutils.silo import control_silo_test from sentry.utils import json @@ -122,13 +124,14 @@ def test_makes_failed_request(self): status=401, ) - result = AlertRuleActionRequester( - install=self.install, - uri="/sentry/alert-rule", - fields=self.fields, - ).run() - assert not result["success"] - assert result["message"] == f"{self.sentry_app.name}: {DEFAULT_ERROR_MESSAGE}" + with pytest.raises(SentryAppIntegratorError) as e: + AlertRuleActionRequester( + install=self.install, + uri="/sentry/alert-rule", + fields=self.fields, + ).run() + assert str(e) == f"{DEFAULT_ERROR_MESSAGE} {self.sentry_app.slug}" + request = responses.calls[0].request data = { @@ -164,13 +167,14 @@ def test_makes_failed_request_with_message(self): status=401, json={"message": self.error_message}, ) - result = AlertRuleActionRequester( - install=self.install, - uri="/sentry/alert-rule", - fields=self.fields, - ).run() - assert not result["success"] - assert result["message"] == f"{self.sentry_app.name}: {self.error_message}" + with pytest.raises(SentryAppIntegratorError) as e: + AlertRuleActionRequester( + install=self.install, + uri="/sentry/alert-rule", + fields=self.fields, + ).run() + + assert str(e) == f"{self.sentry_app.name}: {self.error_message}" @responses.activate def test_makes_failed_request_with_malformed_message(self): @@ -180,10 +184,11 @@ def test_makes_failed_request_with_malformed_message(self): status=401, body=self.error_message.encode(), ) - result = AlertRuleActionRequester( - install=self.install, - uri="/sentry/alert-rule", - fields=self.fields, - ).run() - assert not result["success"] - assert result["message"] == f"{self.sentry_app.name}: {DEFAULT_ERROR_MESSAGE}" + + with pytest.raises(SentryAppIntegratorError) as e: + AlertRuleActionRequester( + install=self.install, + uri="/sentry/alert-rule", + fields=self.fields, + ).run() + assert str(e) == f"{DEFAULT_ERROR_MESSAGE} {self.sentry_app.slug}" From d18f463468ecaad8ceabe32c6ea36872c9e23448 Mon Sep 17 00:00:00 2001 From: Christinarlong Date: Wed, 11 Dec 2024 14:19:28 -0800 Subject: [PATCH 10/13] fix sentry apps tests --- .../sentry_apps/alert_rule_action_creator.py | 2 +- src/sentry/sentry_apps/installations.py | 7 ++----- .../external_issues/test_issue_link_creator.py | 4 ++-- .../test_issue_link_requester.py | 7 +++---- .../external_requests/test_select_requester.py | 15 +++++++-------- 5 files changed, 15 insertions(+), 20 deletions(-) diff --git a/src/sentry/sentry_apps/alert_rule_action_creator.py b/src/sentry/sentry_apps/alert_rule_action_creator.py index 5be1c9ea1868fb..4a14f9c334e6e8 100644 --- a/src/sentry/sentry_apps/alert_rule_action_creator.py +++ b/src/sentry/sentry_apps/alert_rule_action_creator.py @@ -6,7 +6,6 @@ from django.utils.functional import cached_property from sentry.coreapi import APIError -from sentry.exceptions import SentryAppIntegratorError from sentry.sentry_apps.external_requests.alert_rule_action_requester import ( AlertRuleActionRequester, AlertRuleActionResult, @@ -14,6 +13,7 @@ from sentry.sentry_apps.models.sentry_app import SentryApp 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 SentryAppIntegratorError @dataclass diff --git a/src/sentry/sentry_apps/installations.py b/src/sentry/sentry_apps/installations.py index 837a4dab9b53e6..383618749509db 100644 --- a/src/sentry/sentry_apps/installations.py +++ b/src/sentry/sentry_apps/installations.py @@ -24,7 +24,6 @@ from sentry.sentry_apps.models.sentry_app_installation_token import SentryAppInstallationToken from sentry.sentry_apps.services.hook import hook_service from sentry.sentry_apps.tasks.sentry_apps import installation_webhook -from sentry.sentry_apps.utils.errors import SentryAppError from sentry.users.models.user import User from sentry.users.services.user.model import RpcUser from sentry.utils import metrics @@ -194,10 +193,8 @@ class SentryAppInstallationNotifier: def run(self) -> None: if self.action not in VALID_ACTIONS: - raise SentryAppError( - APIUnauthorized( - f"Invalid action '{self.action} for installation notifier for {self.sentry_app}" - ) + raise APIUnauthorized( + f"Invalid action '{self.action} for installation notifier for {self.sentry_app}" ) send_and_save_webhook_request(self.sentry_app, self.request) diff --git a/tests/sentry/sentry_apps/external_issues/test_issue_link_creator.py b/tests/sentry/sentry_apps/external_issues/test_issue_link_creator.py index 6ca2418e606b2d..e12aba398b50ba 100644 --- a/tests/sentry/sentry_apps/external_issues/test_issue_link_creator.py +++ b/tests/sentry/sentry_apps/external_issues/test_issue_link_creator.py @@ -1,10 +1,10 @@ import pytest import responses -from sentry.coreapi import APIUnauthorized from sentry.sentry_apps.external_issues.issue_link_creator import IssueLinkCreator from sentry.sentry_apps.models.platformexternalissue import PlatformExternalIssue from sentry.sentry_apps.services.app import app_service +from sentry.sentry_apps.utils.errors import SentryAppError from sentry.testutils.cases import TestCase from sentry.users.services.user.serial import serialize_rpc_user @@ -60,7 +60,7 @@ def test_creates_external_issue(self): assert external_issue.display_name == "Projectname#issue-1" def test_invalid_action(self): - with pytest.raises(APIUnauthorized): + with pytest.raises(SentryAppError): IssueLinkCreator( install=self.install, group=self.group, diff --git a/tests/sentry/sentry_apps/external_requests/test_issue_link_requester.py b/tests/sentry/sentry_apps/external_requests/test_issue_link_requester.py index 082a40e84cdd6c..a7943c7a079b00 100644 --- a/tests/sentry/sentry_apps/external_requests/test_issue_link_requester.py +++ b/tests/sentry/sentry_apps/external_requests/test_issue_link_requester.py @@ -1,10 +1,9 @@ import pytest import responses -from jsonschema import ValidationError -from sentry.coreapi import APIError from sentry.sentry_apps.external_requests.issue_link_requester import IssueLinkRequester from sentry.sentry_apps.services.app import app_service +from sentry.sentry_apps.utils.errors import SentryAppIntegratorError from sentry.testutils.cases import TestCase from sentry.users.services.user.serial import serialize_rpc_user from sentry.utils import json @@ -95,7 +94,7 @@ def test_invalid_response_format(self): status=200, content_type="application/json", ) - with pytest.raises(ValidationError): + with pytest.raises(SentryAppIntegratorError): IssueLinkRequester( install=self.install, group=self.group, @@ -114,7 +113,7 @@ def test_500_response(self): status=500, ) - with pytest.raises(APIError): + with pytest.raises(SentryAppIntegratorError): IssueLinkRequester( install=self.install, group=self.group, 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 45ad13df13145d..58ae7036ecad4b 100644 --- a/tests/sentry/sentry_apps/external_requests/test_select_requester.py +++ b/tests/sentry/sentry_apps/external_requests/test_select_requester.py @@ -1,10 +1,9 @@ import pytest import responses -from jsonschema import ValidationError -from sentry.coreapi import APIError from sentry.sentry_apps.external_requests.select_requester import SelectRequester from sentry.sentry_apps.services.app import app_service +from sentry.sentry_apps.utils.errors import SentryAppIntegratorError from sentry.testutils.cases import TestCase from sentry.utils.sentry_apps import SentryAppWebhookRequestsBuffer @@ -70,7 +69,7 @@ def test_invalid_response_missing_label(self): content_type="application/json", ) - with pytest.raises(ValidationError): + with pytest.raises(SentryAppIntegratorError): SelectRequester( install=self.install, project_slug=self.project.slug, @@ -91,7 +90,7 @@ def test_invalid_response_missing_value(self): content_type="application/json", ) - with pytest.raises(ValidationError): + with pytest.raises(SentryAppIntegratorError): SelectRequester( install=self.install, project_slug=self.project.slug, @@ -107,7 +106,7 @@ def test_500_response(self): status=500, ) - with pytest.raises(APIError): + with pytest.raises(SentryAppIntegratorError): SelectRequester( install=self.install, project_slug=self.project.slug, @@ -130,7 +129,7 @@ def test_api_error_message(self): status=500, ) - with pytest.raises(APIError) as exception_info: + with pytest.raises(SentryAppIntegratorError) as exception_info: SelectRequester( install=self.install, project_slug=self.project.slug, @@ -152,7 +151,7 @@ def test_validation_error_message_validator(self): status=200, ) - with pytest.raises(ValidationError) as exception_info: + with pytest.raises(SentryAppIntegratorError) as exception_info: SelectRequester( install=self.install, project_slug=self.project.slug, @@ -173,7 +172,7 @@ def test_validation_error_message_missing_field(self): status=200, ) - with pytest.raises(ValidationError) as exception_info: + with pytest.raises(SentryAppIntegratorError) as exception_info: SelectRequester( install=self.install, project_slug=self.project.slug, From a316a623c47b723ca70156ba6c59d04ff1ff6150 Mon Sep 17 00:00:00 2001 From: Christinarlong Date: Wed, 11 Dec 2024 14:42:31 -0800 Subject: [PATCH 11/13] fix typing --- src/sentry/api/endpoints/project_rule_details.py | 4 +++- src/sentry/api/endpoints/project_rules.py | 5 ++++- .../endpoints/organization_alert_rule_details.py | 7 +++++-- .../incidents/endpoints/organization_alert_rule_index.py | 5 ++++- src/sentry/sentry_apps/alert_rule_action_creator.py | 9 +++++---- .../external_requests/alert_rule_action_requester.py | 2 +- 6 files changed, 22 insertions(+), 10 deletions(-) diff --git a/src/sentry/api/endpoints/project_rule_details.py b/src/sentry/api/endpoints/project_rule_details.py index 89906024bc9cde..b1d5a15a33a38b 100644 --- a/src/sentry/api/endpoints/project_rule_details.py +++ b/src/sentry/api/endpoints/project_rule_details.py @@ -288,7 +288,9 @@ def put(self, request: Request, project, rule) -> Response: context = {"uuid": client.uuid} return Response(context, status=202) - create_sentry_app_alert_rule_issues_component(actions=kwargs["actions"]) + result = create_sentry_app_alert_rule_issues_component(actions=kwargs["actions"]) + if isinstance(result, Response): + return result updated_rule = ProjectRuleUpdater( rule=rule, diff --git a/src/sentry/api/endpoints/project_rules.py b/src/sentry/api/endpoints/project_rules.py index 21b5876fa73568..cdd2252642e4d1 100644 --- a/src/sentry/api/endpoints/project_rules.py +++ b/src/sentry/api/endpoints/project_rules.py @@ -56,7 +56,7 @@ def clean_rule_data(data): def create_sentry_app_alert_rule_issues_component( actions: Sequence[Mapping[str, Any]] -) -> str | None: +) -> str | Response: try: created = trigger_sentry_app_action_creators_for_issues(actions) except (SentryAppError, SentryAppIntegratorError) as e: @@ -869,6 +869,9 @@ def post(self, request: Request, project) -> Response: created_alert_rule_ui_component = create_sentry_app_alert_rule_issues_component( actions=kwargs["actions"] ) + if isinstance(created_alert_rule_ui_component, Response): + return created_alert_rule_ui_component + rule = ProjectRuleCreator( name=kwargs["name"], project=project, diff --git a/src/sentry/incidents/endpoints/organization_alert_rule_details.py b/src/sentry/incidents/endpoints/organization_alert_rule_details.py index 1ea443fca8bbef..82656227fca0b8 100644 --- a/src/sentry/incidents/endpoints/organization_alert_rule_details.py +++ b/src/sentry/incidents/endpoints/organization_alert_rule_details.py @@ -87,7 +87,10 @@ def update_alert_rule(request: Request, organization, alert_rule): partial=True, ) if serializer.is_valid(): - create_sentry_app_alert_rule_component(serializer.validated_data) + raised_error = create_sentry_app_alert_rule_component(serializer.validated_data) + if raised_error: + return raised_error + if get_slack_actions_with_async_lookups(organization, request.user, data): # need to kick off an async job for Slack client = RedisRuleStatus() @@ -116,7 +119,7 @@ def remove_alert_rule(request: Request, organization, alert_rule): return Response("This rule has already been deleted", status=status.HTTP_400_BAD_REQUEST) -def create_sentry_app_alert_rule_component(serialized_data: Mapping[str, Any]) -> None: +def create_sentry_app_alert_rule_component(serialized_data: Mapping[str, Any]) -> Response | None: try: trigger_sentry_app_action_creators_for_incidents(serialized_data) except (SentryAppError, SentryAppIntegratorError) as e: diff --git a/src/sentry/incidents/endpoints/organization_alert_rule_index.py b/src/sentry/incidents/endpoints/organization_alert_rule_index.py index def593ffc93343..cbd1d1287c995d 100644 --- a/src/sentry/incidents/endpoints/organization_alert_rule_index.py +++ b/src/sentry/incidents/endpoints/organization_alert_rule_index.py @@ -122,7 +122,10 @@ def create_metric_alert( if not serializer.is_valid(): raise ValidationError(serializer.errors) - create_sentry_app_alert_rule_component(serializer.validated_data) + raised_error = create_sentry_app_alert_rule_component(serializer.validated_data) + if raised_error: + return raised_error + if get_slack_actions_with_async_lookups(organization, request.user, request.data): # need to kick off an async job for Slack client = RedisRuleStatus() diff --git a/src/sentry/sentry_apps/alert_rule_action_creator.py b/src/sentry/sentry_apps/alert_rule_action_creator.py index 4a14f9c334e6e8..9652294bc2181e 100644 --- a/src/sentry/sentry_apps/alert_rule_action_creator.py +++ b/src/sentry/sentry_apps/alert_rule_action_creator.py @@ -32,11 +32,12 @@ def _fetch_sentry_app_uri(self) -> str: type="alert-rule-action", sentry_app=self.sentry_app ) settings = component.schema.get("settings", {}) - return settings.get("uri") - - def _make_external_request(self, uri: str = None) -> AlertRuleActionResult: - if uri is None: + uri = settings.get("uri", None) + if not uri: raise SentryAppIntegratorError(APIError("Sentry App request url not found on schema")) + return uri + + def _make_external_request(self, uri: str) -> AlertRuleActionResult: response = AlertRuleActionRequester( install=self.install, uri=uri, diff --git a/src/sentry/sentry_apps/external_requests/alert_rule_action_requester.py b/src/sentry/sentry_apps/external_requests/alert_rule_action_requester.py index d9716fa361174c..82f73c1729bbb2 100644 --- a/src/sentry/sentry_apps/external_requests/alert_rule_action_requester.py +++ b/src/sentry/sentry_apps/external_requests/alert_rule_action_requester.py @@ -24,7 +24,7 @@ class AlertRuleActionResult(TypedDict): success: bool message: str - error_type: SentryAppErrorType | None = None + error_type: SentryAppErrorType | None @dataclass From 138812f3260ead251ed3812e8d07467f0fce76e0 Mon Sep 17 00:00:00 2001 From: Christinarlong Date: Thu, 12 Dec 2024 11:02:37 -0800 Subject: [PATCH 12/13] circluar imports :dies and adding tests for metric alerts --- src/sentry/api/endpoints/project_rules.py | 2 +- .../organization_alert_rule_details.py | 29 +--- .../organization_alert_rule_index.py | 10 +- src/sentry/incidents/utils/sentry_apps.py | 36 ----- src/sentry/rules/actions/__init__.py | 2 - .../rules/actions/sentry_apps/__init__.py | 2 - .../models/sentry_app_installation.py | 3 +- src/sentry/sentry_apps/services/app/impl.py | 8 -- .../sentry_apps/utils/alert_rule_action.py | 60 +++++++- .../test_organization_alert_rule_details.py | 131 +++++++++++++++++- .../test_organization_alert_rule_index.py | 2 +- 11 files changed, 204 insertions(+), 81 deletions(-) delete mode 100644 src/sentry/incidents/utils/sentry_apps.py diff --git a/src/sentry/api/endpoints/project_rules.py b/src/sentry/api/endpoints/project_rules.py index cdd2252642e4d1..84a256dbdc0fe4 100644 --- a/src/sentry/api/endpoints/project_rules.py +++ b/src/sentry/api/endpoints/project_rules.py @@ -30,8 +30,8 @@ from sentry.integrations.slack.utils.rule_status import RedisRuleStatus from sentry.models.rule import Rule, RuleActivity, RuleActivityType from sentry.projects.project_rules.creator import ProjectRuleCreator -from sentry.rules.actions import trigger_sentry_app_action_creators_for_issues from sentry.rules.actions.base import instantiate_action +from sentry.rules.actions.sentry_apps.utils import trigger_sentry_app_action_creators_for_issues from sentry.rules.processing.processor import is_condition_slow from sentry.sentry_apps.utils.errors import SentryAppError, SentryAppIntegratorError from sentry.signals import alert_rule_created diff --git a/src/sentry/incidents/endpoints/organization_alert_rule_details.py b/src/sentry/incidents/endpoints/organization_alert_rule_details.py index 82656227fca0b8..5e956fbd1d939b 100644 --- a/src/sentry/incidents/endpoints/organization_alert_rule_details.py +++ b/src/sentry/incidents/endpoints/organization_alert_rule_details.py @@ -1,7 +1,3 @@ -from collections.abc import Mapping -from typing import Any - -import sentry_sdk from django.db.models import Q from drf_spectacular.utils import extend_schema, extend_schema_serializer from rest_framework import serializers, status @@ -33,14 +29,15 @@ get_slack_actions_with_async_lookups, ) from sentry.incidents.serializers import AlertRuleSerializer as DrfAlertRuleSerializer -from sentry.incidents.utils.sentry_apps import trigger_sentry_app_action_creators_for_incidents from sentry.integrations.slack.tasks.find_channel_id_for_alert_rule import ( find_channel_id_for_alert_rule, ) from sentry.integrations.slack.utils.rule_status import RedisRuleStatus from sentry.models.rulesnooze import RuleSnooze from sentry.sentry_apps.services.app import app_service -from sentry.sentry_apps.utils.errors import SentryAppError, SentryAppIntegratorError +from sentry.sentry_apps.utils.alert_rule_action import ( + create_sentry_app_alert_rule_component_for_incidents, +) from sentry.users.services.user.service import user_service @@ -87,7 +84,9 @@ def update_alert_rule(request: Request, organization, alert_rule): partial=True, ) if serializer.is_valid(): - raised_error = create_sentry_app_alert_rule_component(serializer.validated_data) + raised_error = create_sentry_app_alert_rule_component_for_incidents( + serializer.validated_data + ) if raised_error: return raised_error @@ -119,22 +118,6 @@ def remove_alert_rule(request: Request, organization, alert_rule): return Response("This rule has already been deleted", status=status.HTTP_400_BAD_REQUEST) -def create_sentry_app_alert_rule_component(serialized_data: Mapping[str, Any]) -> Response | None: - try: - trigger_sentry_app_action_creators_for_incidents(serialized_data) - except (SentryAppError, SentryAppIntegratorError) as e: - return Response( - str(e), - status=status.HTTP_400_BAD_REQUEST, - ) - except Exception as e: - error_id = sentry_sdk.capture_exception(e) - return Response( - f"Something went wrong while trying to create alert rule action. Sentry error ID: {error_id}", - status=500, - ) - - @extend_schema_serializer(exclude_fields=["excludedProjects", "thresholdPeriod"]) class OrganizationAlertRuleDetailsPutSerializer(serializers.Serializer): name = serializers.CharField(max_length=256, help_text="The name for the rule.") diff --git a/src/sentry/incidents/endpoints/organization_alert_rule_index.py b/src/sentry/incidents/endpoints/organization_alert_rule_index.py index cbd1d1287c995d..eb512447984899 100644 --- a/src/sentry/incidents/endpoints/organization_alert_rule_index.py +++ b/src/sentry/incidents/endpoints/organization_alert_rule_index.py @@ -31,9 +31,6 @@ from sentry.apidocs.utils import inline_sentry_response_serializer from sentry.constants import ObjectStatus from sentry.exceptions import InvalidParams -from sentry.incidents.endpoints.organization_alert_rule_details import ( - create_sentry_app_alert_rule_component, -) from sentry.incidents.endpoints.serializers.alert_rule import ( AlertRuleSerializer, AlertRuleSerializerResponse, @@ -54,6 +51,9 @@ from sentry.models.rule import Rule, RuleSource from sentry.models.team import Team from sentry.sentry_apps.services.app import app_service +from sentry.sentry_apps.utils.alert_rule_action import ( + create_sentry_app_alert_rule_component_for_incidents, +) from sentry.snuba.dataset import Dataset from sentry.snuba.models import SnubaQuery from sentry.uptime.models import ( @@ -122,7 +122,9 @@ def create_metric_alert( if not serializer.is_valid(): raise ValidationError(serializer.errors) - raised_error = create_sentry_app_alert_rule_component(serializer.validated_data) + raised_error = create_sentry_app_alert_rule_component_for_incidents( + serializer.validated_data + ) if raised_error: return raised_error diff --git a/src/sentry/incidents/utils/sentry_apps.py b/src/sentry/incidents/utils/sentry_apps.py deleted file mode 100644 index 5b849fa8255985..00000000000000 --- a/src/sentry/incidents/utils/sentry_apps.py +++ /dev/null @@ -1,36 +0,0 @@ -from collections.abc import Mapping -from typing import Any - -from rest_framework import serializers - -from sentry.auth.access import NoAccess -from sentry.incidents.logic import get_filtered_actions -from sentry.incidents.models.alert_rule import AlertRuleTriggerAction -from sentry.incidents.serializers import AlertRuleTriggerActionSerializer -from sentry.sentry_apps.services.app import app_service -from sentry.sentry_apps.utils.alert_rule_action import raise_alert_rule_action_result_errors - - -def trigger_sentry_app_action_creators_for_incidents(alert_rule_data: Mapping[str, Any]) -> None: - sentry_app_actions = get_filtered_actions( - alert_rule_data=alert_rule_data, - action_type=AlertRuleTriggerAction.Type.SENTRY_APP, - ) - # We're doing this so that Sentry Apps without alert-rule-action schemas still get saved - sentry_app_actions_with_components = list( - filter(lambda x: x.get("sentry_app_config"), sentry_app_actions) - ) - - for action in sentry_app_actions_with_components: - action_serializer = AlertRuleTriggerActionSerializer( - context={"access": NoAccess()}, - data=action, - ) - if not action_serializer.is_valid(): - raise serializers.ValidationError(action_serializer.errors) - - result = app_service.trigger_sentry_app_action_creators( - fields=action.get("sentry_app_config"), - install_uuid=action.get("sentry_app_installation_uuid"), - ) - raise_alert_rule_action_result_errors(result) diff --git a/src/sentry/rules/actions/__init__.py b/src/sentry/rules/actions/__init__.py index 7cee5b8db4a817..2f567f43ef91d1 100644 --- a/src/sentry/rules/actions/__init__.py +++ b/src/sentry/rules/actions/__init__.py @@ -4,12 +4,10 @@ IntegrationNotifyServiceForm, TicketEventAction, ) -from sentry.rules.actions.sentry_apps import trigger_sentry_app_action_creators_for_issues __all__ = ( "EventAction", "IntegrationEventAction", "IntegrationNotifyServiceForm", "TicketEventAction", - "trigger_sentry_app_action_creators_for_issues", ) diff --git a/src/sentry/rules/actions/sentry_apps/__init__.py b/src/sentry/rules/actions/sentry_apps/__init__.py index aaa63e984888dd..97adb9e9201ced 100644 --- a/src/sentry/rules/actions/sentry_apps/__init__.py +++ b/src/sentry/rules/actions/sentry_apps/__init__.py @@ -1,9 +1,7 @@ from .base import SentryAppEventAction from .notify_event import NotifyEventSentryAppAction -from .utils import trigger_sentry_app_action_creators_for_issues __all__ = ( "NotifyEventSentryAppAction", "SentryAppEventAction", - "trigger_sentry_app_action_creators_for_issues", ) diff --git a/src/sentry/sentry_apps/models/sentry_app_installation.py b/src/sentry/sentry_apps/models/sentry_app_installation.py index 62cfb87dac734f..55a04c45b4de96 100644 --- a/src/sentry/sentry_apps/models/sentry_app_installation.py +++ b/src/sentry/sentry_apps/models/sentry_app_installation.py @@ -19,6 +19,7 @@ from sentry.hybridcloud.outbox.category import OutboxCategory from sentry.projects.services.project import RpcProject from sentry.sentry_apps.services.app.model import RpcSentryAppComponent, RpcSentryAppInstallation +from sentry.sentry_apps.utils.errors import SentryAppError, SentryAppIntegratorError from sentry.types.region import find_regions_for_orgs if TYPE_CHECKING: @@ -240,6 +241,6 @@ def prepare_ui_component( component=component, install=installation, project_slug=project_slug, values=values ).run() return component - except (APIError, ValidationError): + except (APIError, ValidationError, SentryAppError, SentryAppIntegratorError): # TODO(nisanthan): For now, skip showing the UI Component if the API requests fail return None diff --git a/src/sentry/sentry_apps/services/app/impl.py b/src/sentry/sentry_apps/services/app/impl.py index d33fbd5845174c..443939243897d9 100644 --- a/src/sentry/sentry_apps/services/app/impl.py +++ b/src/sentry/sentry_apps/services/app/impl.py @@ -258,14 +258,6 @@ def trigger_sentry_app_action_creators( ) -> RpcAlertRuleActionResult: try: install = SentryAppInstallation.objects.get(uuid=install_uuid) - except SentryAppInstallation.DoesNotExist: - return RpcAlertRuleActionResult( - success=False, - error_type=SentryAppErrorType.SENTRY, - message="Installation does not exist", - ) - - try: result = AlertRuleActionCreator(install=install, fields=fields).run() except (SentryAppError, SentryAppIntegratorError) as e: return RpcAlertRuleActionResult(success=False, error_type=e.error_type, message=str(e)) diff --git a/src/sentry/sentry_apps/utils/alert_rule_action.py b/src/sentry/sentry_apps/utils/alert_rule_action.py index 829242ee336a59..517b74d0475b2e 100644 --- a/src/sentry/sentry_apps/utils/alert_rule_action.py +++ b/src/sentry/sentry_apps/utils/alert_rule_action.py @@ -1,3 +1,15 @@ +from collections.abc import Mapping +from typing import Any + +import sentry_sdk +from rest_framework import serializers, status +from rest_framework.response import Response + +from sentry.auth.access import NoAccess +from sentry.incidents.logic import get_filtered_actions +from sentry.incidents.models.alert_rule import AlertRuleTriggerAction +from sentry.incidents.serializers import AlertRuleTriggerActionSerializer +from sentry.sentry_apps.services.app import app_service from sentry.sentry_apps.services.app.model import RpcAlertRuleActionResult from sentry.sentry_apps.utils.errors import ( SentryAppError, @@ -7,7 +19,11 @@ def raise_alert_rule_action_result_errors(result: RpcAlertRuleActionResult) -> None: - match result.error_type: + if result.error_type is None: + return None + + error_type = SentryAppErrorType(result.error_type) + match error_type: case SentryAppErrorType.INTEGRATOR: raise SentryAppIntegratorError(result.message) case SentryAppErrorType.CLIENT: @@ -15,4 +31,44 @@ def raise_alert_rule_action_result_errors(result: RpcAlertRuleActionResult) -> N case SentryAppErrorType.SENTRY: raise Exception(result.message) - return None + +def create_sentry_app_alert_rule_component_for_incidents( + serialized_data: Mapping[str, Any] +) -> Response | None: + try: + trigger_sentry_app_action_creators_for_incidents(serialized_data) + except (SentryAppError, SentryAppIntegratorError) as e: + return Response( + str(e), + status=status.HTTP_400_BAD_REQUEST, + ) + except Exception as e: + error_id = sentry_sdk.capture_exception(e) + return Response( + f"Something went wrong while trying to create alert rule action. Sentry error ID: {error_id}", + status=500, + ) + + +def trigger_sentry_app_action_creators_for_incidents(alert_rule_data: Mapping[str, Any]) -> None: + sentry_app_actions = get_filtered_actions( + alert_rule_data=alert_rule_data, + action_type=AlertRuleTriggerAction.Type.SENTRY_APP, + ) + # We're doing this so that Sentry Apps without alert-rule-action schemas still get saved + sentry_app_actions_with_components = list( + filter(lambda x: x.get("sentry_app_config"), sentry_app_actions) + ) + + for action in sentry_app_actions_with_components: + action_serializer = AlertRuleTriggerActionSerializer( + context={"access": NoAccess()}, + data=action, + ) + if not action_serializer.is_valid(): + raise serializers.ValidationError(action_serializer.errors) + result = app_service.trigger_sentry_app_action_creators( + fields=action.get("sentry_app_config"), + install_uuid=action.get("sentry_app_installation_uuid"), + ) + raise_alert_rule_action_result_errors(result) diff --git a/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py b/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py index edf682e7591127..c4f60376189422 100644 --- a/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py +++ b/tests/sentry/incidents/endpoints/test_organization_alert_rule_details.py @@ -50,6 +50,10 @@ from sentry.seer.anomaly_detection.store_data import seer_anomaly_detection_connection_pool from sentry.seer.anomaly_detection.types import StoreDataResponse from sentry.sentry_apps.services.app import app_service +from sentry.sentry_apps.utils.alert_rule_action import ( + trigger_sentry_app_action_creators_for_incidents, +) +from sentry.sentry_apps.utils.errors import SentryAppIntegratorError from sentry.silo.base import SiloMode from sentry.testutils.abstract import Abstract from sentry.testutils.helpers.features import with_feature @@ -1692,7 +1696,132 @@ def test_error_response_from_sentry_app(self): resp = self.get_response(self.organization.slug, self.alert_rule.id, **test_params) assert resp.status_code == 400 - assert error_message in resp.data["sentry_app"] + assert error_message in resp.data + + @responses.activate + def test_trigger_sentry_app_action_creators_for_incidents(self): + self.create_member( + user=self.user, organization=self.organization, role="owner", teams=[self.team] + ) + self.login_as(self.user) + error_message = "Everything is broken!" + responses.add( + method=responses.POST, + url="https://example.com/sentry/alert-rule", + status=500, + json={"message": error_message}, + ) + + sentry_app = self.create_sentry_app( + name="foo", + organization=self.organization, + schema={ + "elements": [ + self.create_alert_rule_action_schema(), + ] + }, + ) + install = self.create_sentry_app_installation( + slug="foo", organization=self.organization, user=self.user + ) + + sentry_app_settings = [ + {"name": "title", "value": "test title"}, + {"name": "description", "value": "test description"}, + ] + + test_params = self.valid_params.copy() + test_params["triggers"] = [ + { + "actions": [ + { + "type": "sentry_app", + "targetType": "sentry_app", + "targetIdentifier": sentry_app.id, + "hasSchemaFormConfig": True, + "sentryAppId": sentry_app.id, + "sentryAppInstallationUuid": install.uuid, + "settings": sentry_app_settings, + } + ], + "alertThreshold": 300, + "label": "critical", + } + ] + + serializer = AlertRuleSerializer( + context={ + "organization": self.organization, + "access": OrganizationGlobalAccess(self.organization, settings.SENTRY_SCOPES), + "user": self.user, + "installations": app_service.installations_for_organization( + organization_id=self.organization.id + ), + }, + data=test_params, + ) + assert serializer.is_valid() + + with pytest.raises(SentryAppIntegratorError) as e: + trigger_sentry_app_action_creators_for_incidents(serializer.validated_data) + assert str(e) == f"{sentry_app.slug}: {error_message}" + + @patch("sentry_sdk.capture_exception") + @patch( + "sentry.sentry_apps.utils.alert_rule_action.trigger_sentry_app_action_creators_for_incidents" + ) + def test_error_response_from_sentry_app_500(self, error, sentry): + self.create_member( + user=self.user, organization=self.organization, role="owner", teams=[self.team] + ) + self.login_as(self.user) + sentry_app = self.create_sentry_app( + name="foo", + organization=self.organization, + schema={ + "elements": [ + self.create_alert_rule_action_schema(), + ] + }, + ) + install = self.create_sentry_app_installation( + slug="foo", organization=self.organization, user=self.user + ) + + sentry_app_settings = [ + {"name": "title", "value": "test title"}, + {"name": "description", "value": "test description"}, + ] + + test_params = self.valid_params.copy() + test_params["triggers"] = [ + { + "actions": [ + { + "type": "sentry_app", + "targetType": "sentry_app", + "targetIdentifier": sentry_app.id, + "hasSchemaFormConfig": True, + "sentryAppId": sentry_app.id, + "sentryAppInstallationUuid": install.uuid, + "settings": sentry_app_settings, + } + ], + "alertThreshold": 300, + "label": "critical", + } + ] + + error.side_effect = Exception("wowzers") + sentry.return_value = 1 + with self.feature(["organizations:incidents", "organizations:performance-view"]): + resp = self.get_response(self.organization.slug, self.alert_rule.id, **test_params) + + assert resp.status_code == 500 + assert ( + resp.data + == f"Something went wrong while trying to create alert rule action. Sentry error ID: {sentry.return_value}" + ) class AlertRuleDetailsDeleteEndpointTest(AlertRuleDetailsBase): diff --git a/tests/sentry/incidents/endpoints/test_organization_alert_rule_index.py b/tests/sentry/incidents/endpoints/test_organization_alert_rule_index.py index 444d9bb6648b0c..f3de456356e7a8 100644 --- a/tests/sentry/incidents/endpoints/test_organization_alert_rule_index.py +++ b/tests/sentry/incidents/endpoints/test_organization_alert_rule_index.py @@ -807,7 +807,7 @@ def test_error_response_from_sentry_app(self): resp = self.get_response(self.organization.slug, **alert_rule) assert resp.status_code == 400 - assert error_message in resp.data["sentry_app"] + assert error_message in resp.data def test_no_label(self): rule_one_trigger_no_label = { From 261d0810ee8e65105ae4d6ab4b9533546740de46 Mon Sep 17 00:00:00 2001 From: Christinarlong Date: Thu, 12 Dec 2024 11:51:48 -0800 Subject: [PATCH 13/13] add tests for project rules --- .../api/endpoints/project_rule_details.py | 7 +-- src/sentry/api/endpoints/project_rules.py | 29 +----------- src/sentry/rules/actions/sentry_apps/utils.py | 25 ----------- .../sentry_apps/utils/alert_rule_action.py | 44 ++++++++++++++++++- .../api/endpoints/test_project_rules.py | 37 ++++++++++++++++ 5 files changed, 84 insertions(+), 58 deletions(-) delete mode 100644 src/sentry/rules/actions/sentry_apps/utils.py diff --git a/src/sentry/api/endpoints/project_rule_details.py b/src/sentry/api/endpoints/project_rule_details.py index b1d5a15a33a38b..f1950b2c85a94c 100644 --- a/src/sentry/api/endpoints/project_rule_details.py +++ b/src/sentry/api/endpoints/project_rule_details.py @@ -11,11 +11,7 @@ from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import region_silo_endpoint from sentry.api.bases.rule import RuleEndpoint -from sentry.api.endpoints.project_rules import ( - create_sentry_app_alert_rule_issues_component, - find_duplicate_rule, - send_confirmation_notification, -) +from sentry.api.endpoints.project_rules import find_duplicate_rule, send_confirmation_notification from sentry.api.fields.actor import ActorField from sentry.api.serializers import serialize from sentry.api.serializers.models.rule import RuleSerializer @@ -38,6 +34,7 @@ from sentry.models.rule import NeglectedRule, RuleActivity, RuleActivityType from sentry.projects.project_rules.updater import ProjectRuleUpdater from sentry.rules.actions.utils import get_changed_data, get_updated_rule_data +from sentry.sentry_apps.utils.alert_rule_action import create_sentry_app_alert_rule_issues_component from sentry.signals import alert_rule_edited from sentry.types.actor import Actor from sentry.utils import metrics diff --git a/src/sentry/api/endpoints/project_rules.py b/src/sentry/api/endpoints/project_rules.py index 84a256dbdc0fe4..b45c6f444c9921 100644 --- a/src/sentry/api/endpoints/project_rules.py +++ b/src/sentry/api/endpoints/project_rules.py @@ -1,8 +1,7 @@ -from collections.abc import Callable, Mapping, Sequence +from collections.abc import Callable from dataclasses import dataclass from typing import Any, Literal -import sentry_sdk from django.conf import settings from django.db.models.signals import pre_save from django.dispatch import receiver @@ -31,9 +30,8 @@ from sentry.models.rule import Rule, RuleActivity, RuleActivityType from sentry.projects.project_rules.creator import ProjectRuleCreator from sentry.rules.actions.base import instantiate_action -from sentry.rules.actions.sentry_apps.utils import trigger_sentry_app_action_creators_for_issues from sentry.rules.processing.processor import is_condition_slow -from sentry.sentry_apps.utils.errors import SentryAppError, SentryAppIntegratorError +from sentry.sentry_apps.utils.alert_rule_action import create_sentry_app_alert_rule_issues_component from sentry.signals import alert_rule_created from sentry.utils import metrics @@ -54,29 +52,6 @@ def clean_rule_data(data): del datum["name"] -def create_sentry_app_alert_rule_issues_component( - actions: Sequence[Mapping[str, Any]] -) -> str | Response: - try: - created = trigger_sentry_app_action_creators_for_issues(actions) - except (SentryAppError, SentryAppIntegratorError) as e: - return Response( - {"actions": [str(e)]}, - status=status.HTTP_400_BAD_REQUEST, - ) - except Exception as e: - error_id = sentry_sdk.capture_exception(e) - return Response( - { - "actions": [ - f"Something went wrong while trying to create alert rule action. Sentry error ID: {error_id}" - ] - }, - status=500, - ) - return created - - @receiver(pre_save, sender=Rule) def pre_save_rule(instance, sender, *args, **kwargs): clean_rule_data(instance.data.get("conditions", [])) diff --git a/src/sentry/rules/actions/sentry_apps/utils.py b/src/sentry/rules/actions/sentry_apps/utils.py deleted file mode 100644 index e98dbaeb9005a5..00000000000000 --- a/src/sentry/rules/actions/sentry_apps/utils.py +++ /dev/null @@ -1,25 +0,0 @@ -from __future__ import annotations - -from collections.abc import Mapping, Sequence -from typing import Any - -from sentry.constants import SENTRY_APP_ACTIONS -from sentry.sentry_apps.services.app import app_service -from sentry.sentry_apps.utils.alert_rule_action import raise_alert_rule_action_result_errors - - -def trigger_sentry_app_action_creators_for_issues( - actions: Sequence[Mapping[str, Any]] -) -> str | None: - created = None - for action in actions: - # Only call creator for Sentry Apps with UI Components for alert rules. - if not action.get("id") in SENTRY_APP_ACTIONS: - continue - - result = app_service.trigger_sentry_app_action_creators( - fields=action["settings"], install_uuid=action.get("sentryAppInstallationUuid") - ) - raise_alert_rule_action_result_errors(result=result) - created = "alert-rule-action" - return created diff --git a/src/sentry/sentry_apps/utils/alert_rule_action.py b/src/sentry/sentry_apps/utils/alert_rule_action.py index 517b74d0475b2e..adb71e54cc3002 100644 --- a/src/sentry/sentry_apps/utils/alert_rule_action.py +++ b/src/sentry/sentry_apps/utils/alert_rule_action.py @@ -1,4 +1,4 @@ -from collections.abc import Mapping +from collections.abc import Mapping, Sequence from typing import Any import sentry_sdk @@ -6,6 +6,7 @@ from rest_framework.response import Response from sentry.auth.access import NoAccess +from sentry.constants import SENTRY_APP_ACTIONS from sentry.incidents.logic import get_filtered_actions from sentry.incidents.models.alert_rule import AlertRuleTriggerAction from sentry.incidents.serializers import AlertRuleTriggerActionSerializer @@ -50,6 +51,30 @@ def create_sentry_app_alert_rule_component_for_incidents( ) +def create_sentry_app_alert_rule_issues_component( + actions: Sequence[Mapping[str, Any]] +) -> str | Response: + try: + created = trigger_sentry_app_action_creators_for_issues(actions) + + except (SentryAppError, SentryAppIntegratorError) as e: + return Response( + {"actions": [str(e)]}, + status=status.HTTP_400_BAD_REQUEST, + ) + except Exception as e: + error_id = sentry_sdk.capture_exception(e) + return Response( + { + "actions": [ + f"Something went wrong while trying to create alert rule action. Sentry error ID: {error_id}" + ] + }, + status=500, + ) + return created + + def trigger_sentry_app_action_creators_for_incidents(alert_rule_data: Mapping[str, Any]) -> None: sentry_app_actions = get_filtered_actions( alert_rule_data=alert_rule_data, @@ -72,3 +97,20 @@ def trigger_sentry_app_action_creators_for_incidents(alert_rule_data: Mapping[st install_uuid=action.get("sentry_app_installation_uuid"), ) raise_alert_rule_action_result_errors(result) + + +def trigger_sentry_app_action_creators_for_issues( + actions: Sequence[Mapping[str, Any]] +) -> str | None: + created = None + for action in actions: + # Only call creator for Sentry Apps with UI Components for alert rules. + if not action.get("id") in SENTRY_APP_ACTIONS: + continue + + result = app_service.trigger_sentry_app_action_creators( + fields=action["settings"], install_uuid=action.get("sentryAppInstallationUuid") + ) + raise_alert_rule_action_result_errors(result=result) + created = "alert-rule-action" + return created diff --git a/tests/sentry/api/endpoints/test_project_rules.py b/tests/sentry/api/endpoints/test_project_rules.py index 3f3772445a2018..b126d885ffa63e 100644 --- a/tests/sentry/api/endpoints/test_project_rules.py +++ b/tests/sentry/api/endpoints/test_project_rules.py @@ -1013,6 +1013,43 @@ def test_create_sentry_app_action_failure(self): assert len(responses.calls) == 1 assert error_message in response.json().get("actions")[0] + @patch("sentry_sdk.capture_exception") + @patch( + "sentry.sentry_apps.utils.alert_rule_action.trigger_sentry_app_action_creators_for_issues" + ) + def test_create_sentry_app_action_failure_500(self, error, exc_id): + actions = [ + { + "id": "sentry.rules.actions.notify_event_sentry_app.NotifyEventSentryAppAction", + "settings": self.sentry_app_settings_payload, + "sentryAppInstallationUuid": self.sentry_app_installation.uuid, + "hasSchemaFormConfig": True, + }, + ] + payload = { + "name": "my super cool rule", + "owner": f"user:{self.user.id}", + "conditions": [], + "filters": [], + "actions": actions, + "filterMatch": "any", + "actionMatch": "any", + "frequency": 30, + } + exc_id.return_value = 1 + error.side_effect = Exception("ZOINKS") + response = self.get_error_response( + self.organization.slug, + self.project.slug, + **payload, + status_code=500, + ) + + assert ( + f"Something went wrong while trying to create alert rule action. Sentry error ID: {exc_id.return_value}" + in response.json().get("actions")[0] + ) + def test_post_rule_256_char_name(self): char_256_name = "wOOFmsWY80o0RPrlsrrqDp2Ylpr5K2unBWbsrqvuNb4Fy3vzawkNAyFJdqeFLlXNWF2kMfgMT9EQmFF3u3MqW3CTI7L2SLsmS9uSDQtcinjlZrr8BT4v8Q6ySrVY5HmiFO97w3awe4lA8uyVikeaSwPjt8MD5WSjdTI0RRXYeK3qnHTpVswBe9AIcQVMLKQXHgjulpsrxHc0DI0Vb8hKA4BhmzQXhYmAvKK26ZwCSjJurAODJB6mgIdlV7tigsFO" response = self.get_success_response(