chore(eco): Silences errors from 400 responses when creating Jira issues (#100078)

Author: GabeVillalobos

src/sentry/integrations/jira/integration.py

@@ -2,9 +2,10 @@
 
 import logging
 import re
+import sys
 from collections.abc import Mapping, Sequence
 from operator import attrgetter
-from typing import Any, TypedDict
+from typing import Any, NoReturn, TypedDict
 
 import sentry_sdk
 from django.conf import settings
@@ -51,6 +52,7 @@
     IntegrationFormError,
 )
 from sentry.silo.base import all_silo_function
+from sentry.users.models.identity import Identity
 from sentry.users.models.user import User
 from sentry.users.services.user import RpcUser
 from sentry.users.services.user.service import user_service
@@ -970,6 +972,31 @@ def create_issue(self, data, **kwargs):
         # Immediately fetch and return the created issue.
         return self.get_issue(issue_key)
 
+    def raise_error(self, exc: Exception, identity: Identity | None = None) -> NoReturn:
+        """
+        Overrides the base `raise_error` method to treat ApiInvalidRequestErrors
+        as configuration errors when we don't have error field handling for the
+        response.
+
+        This is because the majority of Jira errors we receive are external
+        configuration problems, like required fields missing.
+        """
+        if isinstance(exc, ApiInvalidRequestError):
+            if exc.json:
+                error_fields = self.error_fields_from_json(exc.json)
+                if error_fields is not None:
+                    raise IntegrationFormError(error_fields).with_traceback(sys.exc_info()[2])
+
+            logger.warning(
+                "sentry.jira.raise_error.api_invalid_request_error",
+                extra={
+                    "exception_type": type(exc).__name__,
+                    "request_body": str(exc.json),
+                },
+            )
+            raise IntegrationConfigurationError(exc.text) from exc
+        super().raise_error(exc, identity=identity)
+
     def sync_assignee_outbound(
         self,
         external_issue: ExternalIssue,

src/sentry/notifications/notification_action/types.py

@@ -9,6 +9,7 @@
 
 from sentry import features
 from sentry.constants import ObjectStatus
+from sentry.exceptions import InvalidIdentity
 from sentry.incidents.grouptype import MetricIssueEvidenceData
 from sentry.incidents.models.incident import TriggerStatus
 from sentry.incidents.typings.metric_detector import (
@@ -27,9 +28,15 @@
 from sentry.notifications.types import TEST_NOTIFICATION_ID
 from sentry.rules.processing.processor import activate_downstream_actions
 from sentry.services.eventstore.models import GroupEvent
+from sentry.shared_integrations.exceptions import (
+    ApiError,
+    IntegrationConfigurationError,
+    IntegrationFormError,
+)
+from sentry.taskworker.retry import RetryError
+from sentry.taskworker.workerchild import ProcessingDeadlineExceeded
 from sentry.types.activity import ActivityType
 from sentry.types.rules import RuleFuture
-from sentry.utils.safe import safe_execute
 from sentry.workflow_engine.models import Action, AlertRuleWorkflow, Detector
 from sentry.workflow_engine.types import DetectorPriorityLevel, WorkflowEventData
 from sentry.workflow_engine.typings.notification_action import (
@@ -41,6 +48,8 @@
 
 logger = logging.getLogger(__name__)
 
+FutureCallback = Callable[[GroupEvent, Sequence[RuleFuture]], Any]
+
 
 class RuleData(TypedDict):
     actions: list[dict[str, Any]]
@@ -63,6 +72,48 @@ def handle_workflow_action(
         raise NotImplementedError
 
 
+EXCEPTION_IGNORE_LIST = (IntegrationFormError, IntegrationConfigurationError, InvalidIdentity)
+RETRYABLE_EXCEPTIONS = (ApiError,)
+
+
+def invoke_future_with_error_handling(
+    event_data: WorkflowEventData,
+    callback: FutureCallback,
+    future: Sequence[RuleFuture],
+) -> None:
+    # WorkflowEventData should only ever be a GroupEvent in this context, so we
+    # narrow the type here to keep mypy happy.
+    assert isinstance(
+        event_data.event, GroupEvent
+    ), f"Expected a GroupEvent, received: {type(event_data.event).__name__}"
+    try:
+        callback(event_data.event, future)
+    except EXCEPTION_IGNORE_LIST:
+        # no-op on any exceptions in the ignore list. We likely have
+        # reporting for them in the integration code already.
+        pass
+    except ProcessingDeadlineExceeded:
+        # We need to reraise ProcessingDeadlineExceeded for workflow engine to
+        # monitor and potentially retry this action.
+        raise
+    except RETRYABLE_EXCEPTIONS as e:
+        raise RetryError from e
+    except Exception as e:
+        # This is just a redefinition of the safe_execute util function, as we
+        # still want to report any unhandled exceptions.
+        if hasattr(callback, "im_class"):
+            cls = callback.im_class
+        else:
+            cls = callback.__class__
+
+        func_name = getattr(callback, "__name__", str(callback))
+        cls_name = cls.__name__
+        local_logger = logging.getLogger(f"sentry.safe_action.{cls_name.lower()}")
+
+        local_logger.exception("%s.process_error", func_name, extra={"exception": e})
+        return None
+
+
 class BaseIssueAlertHandler(ABC):
     """
     Base class for invoking the legacy issue alert registry.
@@ -225,9 +276,7 @@ def get_rule_futures(
     @staticmethod
     def execute_futures(
         event_data: WorkflowEventData,
-        futures: Collection[
-            tuple[Callable[[GroupEvent, Sequence[RuleFuture]], None], list[RuleFuture]]
-        ],
+        futures: Collection[tuple[FutureCallback, list[RuleFuture]]],
     ) -> None:
         """
         This method will execute the futures.
@@ -239,7 +288,7 @@ def execute_futures(
             )
 
         for callback, future in futures:
-            safe_execute(callback, event_data.event, future)
+            invoke_future_with_error_handling(event_data, callback, future)
 
     @staticmethod
     def send_test_notification(

src/sentry/rules/actions/integrations/create_ticket/utils.py

@@ -18,7 +18,6 @@
 from sentry.integrations.services.integration.model import RpcIntegration
 from sentry.integrations.services.integration.service import integration_service
 from sentry.models.grouplink import GroupLink
-from sentry.notifications.types import TEST_NOTIFICATION_ID
 from sentry.notifications.utils.links import create_link_to_workflow
 from sentry.services.eventstore.models import GroupEvent
 from sentry.shared_integrations.exceptions import (
@@ -187,10 +186,8 @@ def create_issue(event: GroupEvent, futures: Sequence[RuleFuture]) -> None:
             ) as e:
                 # Most of the time, these aren't explicit failures, they're
                 # some misconfiguration of an issue field - typically Jira.
-                # We only want to raise if the rule_id is -1 because that means we're testing the action
                 lifecycle.record_halt(e)
-                if rule_id == TEST_NOTIFICATION_ID:
-                    raise
+                raise
             # If we successfully created the issue, we want to create the link
             else:
                 create_link(integration, installation, event, response)

tests/sentry/integrations/jira/test_integration.py

@@ -19,7 +19,11 @@
 from sentry.integrations.services.integration import integration_service
 from sentry.models.grouplink import GroupLink
 from sentry.models.groupmeta import GroupMeta
-from sentry.shared_integrations.exceptions import IntegrationConfigurationError, IntegrationError
+from sentry.shared_integrations.exceptions import (
+    IntegrationConfigurationError,
+    IntegrationError,
+    IntegrationFormError,
+)
 from sentry.silo.base import SiloMode
 from sentry.testutils.cases import APITestCase, IntegrationTestCase
 from sentry.testutils.factories import EventType
@@ -711,6 +715,59 @@ def test_create_issue(self) -> None:
                 "key": "APP-123",
             }
 
+    @responses.activate
+    def test_create_issue_with_form_error(self) -> None:
+        responses.add(
+            responses.GET,
+            "https://example.atlassian.net/rest/api/2/issue/createmeta",
+            body=StubService.get_stub_json("jira", "createmeta_response.json"),
+            content_type="json",
+        )
+        responses.add(
+            responses.POST,
+            "https://example.atlassian.net/rest/api/2/issue",
+            status=400,
+            body=json.dumps({"errors": {"issuetype": ["Issue type is required."]}}),
+            content_type="json",
+        )
+
+        installation = self.integration.get_installation(self.organization.id)
+        with pytest.raises(IntegrationFormError):
+            installation.create_issue(
+                {
+                    "title": "example summary",
+                    "description": "example bug report",
+                    "issuetype": "1",
+                    "project": "10000",
+                }
+            )
+
+    @responses.activate
+    def test_create_issue_with_configuration_error(self) -> None:
+        responses.add(
+            responses.GET,
+            "https://example.atlassian.net/rest/api/2/issue/createmeta",
+            body=StubService.get_stub_json("jira", "createmeta_response.json"),
+            content_type="json",
+        )
+        responses.add(
+            responses.POST,
+            "https://example.atlassian.net/rest/api/2/issue",
+            status=400,
+            body=json.dumps({"error": "Jira had an oopsie"}),
+            content_type="json",
+        )
+        installation = self.integration.get_installation(self.organization.id)
+        with pytest.raises(IntegrationConfigurationError):
+            installation.create_issue(
+                {
+                    "title": "example summary",
+                    "description": "example bug report",
+                    "issuetype": "1",
+                    "project": "10000",
+                }
+            )
+
     @responses.activate
     def test_create_issue_labels_and_option(self) -> None:
         installation = self.integration.get_installation(self.organization.id)

tests/sentry/integrations/jira/test_ticket_action.py

@@ -13,10 +13,9 @@
 from sentry.shared_integrations.exceptions import (
     ApiInvalidRequestError,
     IntegrationConfigurationError,
-    IntegrationError,
     IntegrationFormError,
 )
-from sentry.testutils.asserts import assert_failure_metric, assert_halt_metric
+from sentry.testutils.asserts import assert_halt_metric
 from sentry.testutils.cases import RuleTestCase
 from sentry.testutils.skips import requires_snuba
 from sentry.types.rules import RuleFuture
@@ -143,18 +142,16 @@ def raise_api_error(*args, **kwargs):
             rule_object = Rule.objects.get(id=response.data["id"])
             event = self.get_event()
 
-            with pytest.raises(IntegrationError):
-                # Trigger its `after`, but with a broken client which should raise
-                # an ApiInvalidRequestError, which is reraised as an IntegrationError.
+            with pytest.raises(IntegrationConfigurationError):
                 self.trigger(event, rule_object)
 
             assert mock_record_event.call_count == 2
-            start, failure = mock_record_event.call_args_list
+            start, halt = mock_record_event.call_args_list
             assert start.args == (EventLifecycleOutcome.STARTED,)
 
-            assert_failure_metric(
+            assert_halt_metric(
                 mock_record_event,
-                IntegrationError("Error Communicating with Jira (HTTP 400): unknown error"),
+                IntegrationConfigurationError(),
             )
 
     def test_fails_validation(self) -> None:
@@ -211,7 +208,8 @@ def raise_api_error_with_payload(*args, **kwargs):
             rule_object = Rule.objects.get(id=response.data["id"])
             event = self.get_event()
 
-            self.trigger(event, rule_object)
+            with pytest.raises(IntegrationFormError):
+                self.trigger(event, rule_object)
 
             assert mock_record_event.call_count == 2
             start, halt = mock_record_event.call_args_list
@@ -233,7 +231,8 @@ def test_halts_with_external_api_configuration_error(
             rule_object = Rule.objects.get(id=response.data["id"])
             event = self.get_event()
 
-            self.trigger(event, rule_object)
+            with pytest.raises(IntegrationConfigurationError):
+                self.trigger(event, rule_object)
 
             assert mock_record_event.call_count == 2
             start, halt = mock_record_event.call_args_list