From aeb7176c58cb54b91cb784b994999404fd4f0ac8 Mon Sep 17 00:00:00 2001 From: Christinarlong Date: Tue, 10 Dec 2024 14:38:48 -0800 Subject: [PATCH 01/12] dawg idk --- src/sentry/exceptions.py | 12 +++++ .../alert_rule_action_requester.py | 44 ++++++++++++------- src/sentry/sentry_apps/services/app/impl.py | 23 ++++++++-- src/sentry/sentry_apps/services/app/model.py | 2 + .../sentry_apps/services/app/service.py | 4 +- 5 files changed, 65 insertions(+), 20 deletions(-) diff --git a/src/sentry/exceptions.py b/src/sentry/exceptions.py index 1c162069237a05..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 @@ -91,3 +93,13 @@ 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/sentry_apps/external_requests/alert_rule_action_requester.py b/src/sentry/sentry_apps/external_requests/alert_rule_action_requester.py index a9249af4849f63..edbbccf5f47c61 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,9 +6,11 @@ from uuid import uuid4 from django.utils.functional import cached_property -from requests import RequestException +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 @@ -43,23 +45,24 @@ def run(self) -> AlertRuleActionResult: method=self.http_method, data=self.body, ) - except RequestException 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), - }, - ) + 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: + 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=False, message=self._get_response_message(e.response, DEFAULT_ERROR_MESSAGE) - ) return AlertRuleActionResult( - success=True, message=self._get_response_message(response, DEFAULT_SUCCESS_MESSAGE) + success=True, message=self._get_response_message(response, "poggers") ) def _build_url(self) -> str: @@ -92,6 +95,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 e266283516b35e..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 ( @@ -24,7 +25,6 @@ from sentry.sentry_apps.models.sentry_app_installation_token import SentryAppInstallationToken from sentry.sentry_apps.services.app import ( AppService, - RpcAlertRuleActionResult, RpcSentryApp, RpcSentryAppComponent, RpcSentryAppComponentContext, @@ -33,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 @@ -254,8 +256,23 @@ def trigger_sentry_app_action_creators( try: install = SentryAppInstallation.objects.get(uuid=install_uuid) except SentryAppInstallation.DoesNotExist: - return RpcAlertRuleActionResult(success=False, message="Installation does not exist") - result = AlertRuleActionCreator(install=install, fields=fields).run() + 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: 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 7d8d8f466fcc60..757e0ca41094fa 100644 --- a/src/sentry/sentry_apps/services/app/service.py +++ b/src/sentry/sentry_apps/services/app/service.py @@ -11,16 +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 From 3002ef58aa2fdd033f398fd3381bd5aeec17c4ec Mon Sep 17 00:00:00 2001 From: Christinarlong Date: Tue, 10 Dec 2024 14:39:40 -0800 Subject: [PATCH 02/12] 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 afd18d8dcea45290c99a5d53769f4320fbf94cd6 Mon Sep 17 00:00:00 2001 From: Christinarlong Date: Wed, 11 Dec 2024 09:51:56 -0800 Subject: [PATCH 03/12] switch back to alertruleactionresult --- .../sentry_apps/alert_rule_action_creator.py | 10 ++++++---- .../alert_rule_action_requester.py | 14 ++++++-------- src/sentry/sentry_apps/utils/errors.py | 10 ++++++++++ 3 files changed, 22 insertions(+), 12 deletions(-) diff --git a/src/sentry/sentry_apps/alert_rule_action_creator.py b/src/sentry/sentry_apps/alert_rule_action_creator.py index cd7e1632d297ec..4a14f9c334e6e8 100644 --- a/src/sentry/sentry_apps/alert_rule_action_creator.py +++ b/src/sentry/sentry_apps/alert_rule_action_creator.py @@ -10,8 +10,10 @@ 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 +from sentry.sentry_apps.utils.errors import SentryAppIntegratorError @dataclass @@ -25,16 +27,16 @@ def run(self) -> AlertRuleActionResult: 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): + def _make_external_request(self, uri: str = None) -> AlertRuleActionResult: if uri is None: - raise APIError("Sentry App request url not found") + raise SentryAppIntegratorError(APIError("Sentry App request url not found on schema")) response = AlertRuleActionRequester( install=self.install, uri=uri, @@ -44,5 +46,5 @@ def _make_external_request(self, uri=None): 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 edbbccf5f47c61..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,15 +9,15 @@ 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!" +DEFAULT_ERROR_MESSAGE = "Something went wrong while setting up alert for" logger = logging.getLogger("sentry.sentry_apps.external_requests") @@ -25,6 +25,7 @@ class AlertRuleActionResult(TypedDict): success: bool message: str + error_type: SentryAppErrorType | None = None @dataclass @@ -55,14 +56,11 @@ def run(self) -> AlertRuleActionResult: 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: diff --git a/src/sentry/sentry_apps/utils/errors.py b/src/sentry/sentry_apps/utils/errors.py index 9c1fe0b62f1641..68aba2dcaba5b1 100644 --- a/src/sentry/sentry_apps/utils/errors.py +++ b/src/sentry/sentry_apps/utils/errors.py @@ -5,3 +5,13 @@ class SentryAppErrorType(Enum): CLIENT = "client" INTEGRATOR = "integrator" SENTRY = "sentry" + + +# 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 From 67bc16ff0b0ac59edc572634fdd43b19e5c45458 Mon Sep 17 00:00:00 2001 From: Christinarlong Date: Wed, 11 Dec 2024 13:13:47 -0800 Subject: [PATCH 04/12] update arc process to return alertruleactionresult --- src/sentry/incidents/utils/sentry_apps.py | 4 ++-- src/sentry/rules/actions/sentry_apps/utils.py | 7 ++----- .../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 | 14 ++++++++++++++ 6 files changed, 25 insertions(+), 10 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 fa23dda81fad38..54f92f7ea17241 100644 --- a/src/sentry/rules/actions/sentry_apps/utils.py +++ b/src/sentry/rules/actions/sentry_apps/utils.py @@ -3,10 +3,9 @@ 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 +from sentry.sentry_apps.utils.errors import raise_alert_rule_action_result_errors def trigger_sentry_app_action_creators_for_issues( @@ -21,8 +20,6 @@ def trigger_sentry_app_action_creators_for_issues( result = 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]}) + 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 68aba2dcaba5b1..997ff34e2420aa 100644 --- a/src/sentry/sentry_apps/utils/errors.py +++ b/src/sentry/sentry_apps/utils/errors.py @@ -1,5 +1,7 @@ from enum import Enum +from sentry.sentry_apps.services.app.model import RpcAlertRuleActionResult + class SentryAppErrorType(Enum): CLIENT = "client" @@ -15,3 +17,15 @@ class SentryAppError(Exception): # Represents an error caused by a 3p integrator during a Sentry App process class SentryAppIntegratorError(Exception): error_type = SentryAppErrorType.INTEGRATOR + + +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 2f0c14f31b6580013ad3a0ab5a48b1fa99b67807 Mon Sep 17 00:00:00 2001 From: Christinarlong Date: Wed, 11 Dec 2024 13:43:33 -0800 Subject: [PATCH 05/12] update refs and arar tests --- src/sentry/exceptions.py | 12 ----- src/sentry/incidents/utils/sentry_apps.py | 2 +- src/sentry/rules/actions/sentry_apps/utils.py | 2 +- .../alert_rule_action_requester.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 | 14 ------ .../test_alert_rule_action_requester.py | 47 ++++++++++--------- 9 files changed, 54 insertions(+), 54 deletions(-) create mode 100644 src/sentry/sentry_apps/utils/alert_rule_action.py 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/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/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/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 997ff34e2420aa..68aba2dcaba5b1 100644 --- a/src/sentry/sentry_apps/utils/errors.py +++ b/src/sentry/sentry_apps/utils/errors.py @@ -1,7 +1,5 @@ from enum import Enum -from sentry.sentry_apps.services.app.model import RpcAlertRuleActionResult - class SentryAppErrorType(Enum): CLIENT = "client" @@ -17,15 +15,3 @@ class SentryAppError(Exception): # Represents an error caused by a 3p integrator during a Sentry App process class SentryAppIntegratorError(Exception): error_type = SentryAppErrorType.INTEGRATOR - - -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/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 223ff0e49c678b9a54eb06ddee326fa5c54e247f Mon Sep 17 00:00:00 2001 From: Christinarlong Date: Wed, 11 Dec 2024 14:19:28 -0800 Subject: [PATCH 06/12] fix sentry apps tests --- .../external_issues/test_issue_link_creator.py | 4 ++-- .../test_issue_link_requester.py | 7 +++---- .../external_requests/test_select_requester.py | 15 +++++++-------- 3 files changed, 12 insertions(+), 14 deletions(-) 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 a0b764ab11c3b47359f3f9c096fd585a885412f0 Mon Sep 17 00:00:00 2001 From: Christinarlong Date: Wed, 11 Dec 2024 14:42:31 -0800 Subject: [PATCH 07/12] fix typing --- .../api/endpoints/project_rule_details.py | 11 +++++-- src/sentry/api/endpoints/project_rules.py | 30 ++++++++++++++++++- .../organization_alert_rule_details.py | 26 +++++++++++++++- .../organization_alert_rule_index.py | 9 ++++-- .../sentry_apps/alert_rule_action_creator.py | 9 +++--- .../alert_rule_action_requester.py | 2 +- 6 files changed, 75 insertions(+), 12 deletions(-) diff --git a/src/sentry/api/endpoints/project_rule_details.py b/src/sentry/api/endpoints/project_rule_details.py index 0559c04d52fe0f..b1d5a15a33a38b 100644 --- a/src/sentry/api/endpoints/project_rule_details.py +++ b/src/sentry/api/endpoints/project_rule_details.py @@ -11,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 @@ -33,7 +37,6 @@ 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 @@ -285,7 +288,9 @@ 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"]) + 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 a7b86baf086b1f..8385b0147d894f 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 @@ -32,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 @@ -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 | 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", [])) @@ -844,6 +869,9 @@ def post(self, request: Request, project) -> Response: created_alert_rule_ui_component = trigger_sentry_app_action_creators_for_issues( 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 0aeea3a1fb0df0..82656227fca0b8 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 @@ -36,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 @@ -82,7 +87,10 @@ 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) + 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() @@ -111,6 +119,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]) -> 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 db45347717d093..cbd1d1287c995d 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,10 @@ def create_metric_alert( if not serializer.is_valid(): raise ValidationError(serializer.errors) - trigger_sentry_app_action_creators_for_incidents(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 b8331a95ec5dbfbcccd1a76b869f25b982f692d0 Mon Sep 17 00:00:00 2001 From: Christinarlong Date: Thu, 12 Dec 2024 11:02:37 -0800 Subject: [PATCH 08/12] 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 8385b0147d894f..0d37f44f6ecbf8 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 9e41d24ff3a11229e920f65457d8383d637cbe49 Mon Sep 17 00:00:00 2001 From: Christinarlong Date: Thu, 12 Dec 2024 11:51:48 -0800 Subject: [PATCH 09/12] 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 0d37f44f6ecbf8..ee5d8c4c72d033 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( From 72a683cee8280c42942785ab1f2204eff2aa062d Mon Sep 17 00:00:00 2001 From: Christinarlong Date: Thu, 12 Dec 2024 15:36:33 -0800 Subject: [PATCH 10/12] revert errors for non alert rule processes --- .../external_issues/test_issue_link_creator.py | 4 ++-- .../test_issue_link_requester.py | 7 ++++--- .../external_requests/test_select_requester.py | 15 ++++++++------- 3 files changed, 14 insertions(+), 12 deletions(-) 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 e12aba398b50ba..6ca2418e606b2d 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(SentryAppError): + with pytest.raises(APIUnauthorized): 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 a7943c7a079b00..082a40e84cdd6c 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,9 +1,10 @@ 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 @@ -94,7 +95,7 @@ def test_invalid_response_format(self): status=200, content_type="application/json", ) - with pytest.raises(SentryAppIntegratorError): + with pytest.raises(ValidationError): IssueLinkRequester( install=self.install, group=self.group, @@ -113,7 +114,7 @@ def test_500_response(self): status=500, ) - with pytest.raises(SentryAppIntegratorError): + with pytest.raises(APIError): 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 58ae7036ecad4b..45ad13df13145d 100644 --- a/tests/sentry/sentry_apps/external_requests/test_select_requester.py +++ b/tests/sentry/sentry_apps/external_requests/test_select_requester.py @@ -1,9 +1,10 @@ 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 @@ -69,7 +70,7 @@ def test_invalid_response_missing_label(self): content_type="application/json", ) - with pytest.raises(SentryAppIntegratorError): + with pytest.raises(ValidationError): SelectRequester( install=self.install, project_slug=self.project.slug, @@ -90,7 +91,7 @@ def test_invalid_response_missing_value(self): content_type="application/json", ) - with pytest.raises(SentryAppIntegratorError): + with pytest.raises(ValidationError): SelectRequester( install=self.install, project_slug=self.project.slug, @@ -106,7 +107,7 @@ def test_500_response(self): status=500, ) - with pytest.raises(SentryAppIntegratorError): + with pytest.raises(APIError): SelectRequester( install=self.install, project_slug=self.project.slug, @@ -129,7 +130,7 @@ def test_api_error_message(self): status=500, ) - with pytest.raises(SentryAppIntegratorError) as exception_info: + with pytest.raises(APIError) as exception_info: SelectRequester( install=self.install, project_slug=self.project.slug, @@ -151,7 +152,7 @@ def test_validation_error_message_validator(self): status=200, ) - with pytest.raises(SentryAppIntegratorError) as exception_info: + with pytest.raises(ValidationError) as exception_info: SelectRequester( install=self.install, project_slug=self.project.slug, @@ -172,7 +173,7 @@ def test_validation_error_message_missing_field(self): status=200, ) - with pytest.raises(SentryAppIntegratorError) as exception_info: + with pytest.raises(ValidationError) as exception_info: SelectRequester( install=self.install, project_slug=self.project.slug, From 2c946f411e9174d798460148067c5582ab6ec679 Mon Sep 17 00:00:00 2001 From: Christinarlong Date: Thu, 12 Dec 2024 15:48:58 -0800 Subject: [PATCH 11/12] bruh why did this pass --- src/sentry/api/endpoints/project_rules.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry/api/endpoints/project_rules.py b/src/sentry/api/endpoints/project_rules.py index ee5d8c4c72d033..a7a2a7072b541f 100644 --- a/src/sentry/api/endpoints/project_rules.py +++ b/src/sentry/api/endpoints/project_rules.py @@ -841,7 +841,7 @@ 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( + created_alert_rule_ui_component = create_sentry_app_alert_rule_issues_component( kwargs["actions"] ) if isinstance(created_alert_rule_ui_component, Response): From b84d81389a85775e82bc857cd51ba679d825b5ac Mon Sep 17 00:00:00 2001 From: Christinarlong Date: Thu, 12 Dec 2024 16:15:21 -0800 Subject: [PATCH 12/12] fix typing --- src/sentry/sentry_apps/utils/alert_rule_action.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/sentry/sentry_apps/utils/alert_rule_action.py b/src/sentry/sentry_apps/utils/alert_rule_action.py index adb71e54cc3002..9aeecce7c224fd 100644 --- a/src/sentry/sentry_apps/utils/alert_rule_action.py +++ b/src/sentry/sentry_apps/utils/alert_rule_action.py @@ -49,11 +49,12 @@ def create_sentry_app_alert_rule_component_for_incidents( f"Something went wrong while trying to create alert rule action. Sentry error ID: {error_id}", status=500, ) + return None def create_sentry_app_alert_rule_issues_component( actions: Sequence[Mapping[str, Any]] -) -> str | Response: +) -> str | Response | None: try: created = trigger_sentry_app_action_creators_for_issues(actions)