fix(Jira-Server): Adds halts, better exceptions for failed syncs

src/sentry/integrations/jira/integration.py

@@ -15,7 +15,6 @@
 
 from sentry import features
 from sentry.eventstore.models import GroupEvent
-from sentry.exceptions import InvalidConfiguration
 from sentry.integrations.base import (
     FeatureDescription,
     IntegrationData,
@@ -25,7 +24,12 @@
 )
 from sentry.integrations.jira.models.create_issue_metadata import JiraIssueTypeMetadata
 from sentry.integrations.jira.tasks import migrate_issues
-from sentry.integrations.mixins.issues import MAX_CHAR, IssueSyncIntegration, ResolveSyncAction
+from sentry.integrations.mixins.issues import (
+    MAX_CHAR,
+    IntegrationSyncTargetNotFound,
+    IssueSyncIntegration,
+    ResolveSyncAction,
+)
 from sentry.integrations.models.external_issue import ExternalIssue
 from sentry.integrations.models.integration_external_project import IntegrationExternalProject
 from sentry.integrations.pipeline import IntegrationPipeline
@@ -1014,16 +1018,20 @@ def sync_assignee_outbound(
                     },
                 )
                 if not user.emails:
-                    raise InvalidConfiguration(
+                    raise IntegrationSyncTargetNotFound(
                         {
                             "email": "User must have a verified email on Sentry to sync assignee in Jira",
                             "help": "https://sentry.io/settings/account/emails",
                         }
                     )
-                raise InvalidConfiguration({"email": "Unable to find the requested user"})
+                raise IntegrationSyncTargetNotFound("No matching Jira user found.")
         try:
             id_field = client.user_id_field()
             client.assign_issue(external_issue.key, jira_user and jira_user.get(id_field))
+        except ApiUnauthorized as e:
+            raise IntegrationInstallationConfigurationError(
+                "Insufficient permissions to assign user to the Jira issue."
+            ) from e
         except ApiError as e:
             # TODO(jess): do we want to email people about these types of failures?
             logger.info(
@@ -1036,7 +1044,7 @@ def sync_assignee_outbound(
                     "issue_key": external_issue.key,
                 },
             )
-            raise
+            raise IntegrationError("There was an error assigning the issue.") from e
 
     def sync_status_outbound(
         self, external_issue: ExternalIssue, is_resolved: bool, project_id: int

src/sentry/integrations/jira_server/integration.py

@@ -29,7 +29,7 @@
 from sentry.integrations.jira.tasks import migrate_issues
 from sentry.integrations.jira_server.utils.choice import build_user_choice
 from sentry.integrations.mixins import ResolveSyncAction
-from sentry.integrations.mixins.issues import IssueSyncIntegration
+from sentry.integrations.mixins.issues import IntegrationSyncTargetNotFound, IssueSyncIntegration
 from sentry.integrations.models.external_actor import ExternalActor
 from sentry.integrations.models.external_issue import ExternalIssue
 from sentry.integrations.models.integration_external_project import IntegrationExternalProject
@@ -1268,30 +1268,32 @@ def sync_assignee_outbound(
             if jira_user is None:
                 # TODO(jess): do we want to email people about these types of failures?
                 logger.info(
-                    "jira.assignee-not-found",
+                    "jira_server.assignee-not-found",
                     extra=logging_context,
                 )
-                raise IntegrationError("Failed to assign user to Jira Server issue")
+                raise IntegrationSyncTargetNotFound("No matching Jira Server user found")
         try:
             id_field = client.user_id_field()
             client.assign_issue(external_issue.key, jira_user and jira_user.get(id_field))
-        except ApiUnauthorized:
+        except ApiUnauthorized as e:
             logger.info(
-                "jira.user-assignment-unauthorized",
+                "jira_server.user-assignment-unauthorized",
                 extra={
                     **logging_context,
                 },
             )
-            raise IntegrationError("Insufficient permissions to assign user to Jira Server issue")
+            raise IntegrationInstallationConfigurationError(
+                "Insufficient permissions to assign user to Jira Server issue"
+            ) from e
         except ApiError as e:
             logger.info(
-                "jira.user-assignment-request-error",
+                "jira_server.user-assignment-request-error",
                 extra={
                     **logging_context,
                     "error": str(e),
                 },
             )
-            raise IntegrationError("Failed to assign user to Jira Server issue")
+            raise IntegrationError("Failed to assign user to Jira Server issue") from e
 
     def sync_status_outbound(
         self, external_issue: ExternalIssue, is_resolved: bool, project_id: int

src/sentry/integrations/mixins/issues.py

@@ -24,6 +24,7 @@
 from sentry.models.grouplink import GroupLink
 from sentry.models.project import Project
 from sentry.notifications.utils import get_notification_group_title
+from sentry.shared_integrations.exceptions import IntegrationError
 from sentry.silo.base import all_silo_function
 from sentry.users.models.user import User
 from sentry.users.services.user import RpcUser
@@ -370,6 +371,10 @@ def update_comment(self, issue_id, user_id, group_note):
         pass
 
 
+class IntegrationSyncTargetNotFound(IntegrationError):
+    pass
+
+
 class IssueSyncIntegration(IssueBasicIntegration, ABC):
     comment_key: ClassVar[str | None] = None
     outbound_status_key: ClassVar[str | None] = None

src/sentry/integrations/tasks/sync_assignee_outbound.py

@@ -2,7 +2,6 @@
 
 from sentry import analytics, features
 from sentry.constants import ObjectStatus
-from sentry.exceptions import InvalidConfiguration
 from sentry.integrations.errors import OrganizationIntegrationNotFound
 from sentry.integrations.models.external_issue import ExternalIssue
 from sentry.integrations.models.integration import Integration
@@ -13,7 +12,11 @@
 from sentry.integrations.services.assignment_source import AssignmentSource
 from sentry.integrations.services.integration import integration_service
 from sentry.models.organization import Organization
-from sentry.shared_integrations.exceptions import ApiUnauthorized, IntegrationError
+from sentry.shared_integrations.exceptions import (
+    ApiUnauthorized,
+    IntegrationError,
+    IntegrationInstallationConfigurationError,
+)
 from sentry.silo.base import SiloMode
 from sentry.tasks.base import instrumented_task, retry
 from sentry.taskworker.config import TaskworkerConfig
@@ -53,6 +56,8 @@ def sync_assignee_outbound(
     assign: bool,
     assignment_source_dict: dict[str, Any] | None = None,
 ) -> None:
+    from sentry.integrations.mixins.issues import IntegrationSyncTargetNotFound
+
     # Sync Sentry assignee to an external issue.
     external_issue = ExternalIssue.objects.get(id=external_issue_id)
 
@@ -98,5 +103,10 @@ def sync_assignee_outbound(
                     id=integration.id,
                     organization_id=external_issue.organization_id,
                 )
-        except (OrganizationIntegrationNotFound, ApiUnauthorized, InvalidConfiguration) as e:
+        except (
+            OrganizationIntegrationNotFound,
+            ApiUnauthorized,
+            IntegrationSyncTargetNotFound,
+            IntegrationInstallationConfigurationError,
+        ) as e:
             lifecycle.record_halt(halt_reason=e)

src/sentry/integrations/vsts/issues.py

@@ -11,7 +11,7 @@
 
 from sentry.constants import ObjectStatus
 from sentry.integrations.mixins import ResolveSyncAction
-from sentry.integrations.mixins.issues import IssueSyncIntegration
+from sentry.integrations.mixins.issues import IntegrationSyncTargetNotFound, IssueSyncIntegration
 from sentry.integrations.services.integration import integration_service
 from sentry.integrations.source_code_management.issues import SourceCodeIssueIntegration
 from sentry.models.activity import Activity
@@ -20,6 +20,7 @@
     ApiUnauthorized,
     IntegrationError,
     IntegrationFormError,
+    IntegrationInstallationConfigurationError,
 )
 from sentry.silo.base import all_silo_function
 from sentry.users.models.identity import Identity
@@ -294,11 +295,11 @@ def sync_assignee_outbound(
                         "issue_key": external_issue.key,
                     },
                 )
-                return
+                raise IntegrationSyncTargetNotFound("No matching VSTS user found.")
 
         try:
             client.update_work_item(external_issue.key, assigned_to=assignee)
-        except (ApiUnauthorized, ApiError):
+        except (ApiUnauthorized, ApiError) as e:
             self.logger.info(
                 "vsts.failed-to-assign",
                 extra={
@@ -307,6 +308,11 @@ def sync_assignee_outbound(
                     "issue_key": external_issue.key,
                 },
             )
+            if isinstance(e, ApiUnauthorized):
+                raise IntegrationInstallationConfigurationError(
+                    "Insufficient permissions to assign user to the VSTS issue."
+                ) from e
+            raise IntegrationError("There was an error assigning the issue.") from e
         except Exception as e:
             self.raise_error(e)
 

src/sentry/shared_integrations/exceptions/__init__.py

@@ -169,7 +169,7 @@ class IntegrationError(Exception):
     pass
 
 
-class IntegrationInstallationConfigurationError(Exception):
+class IntegrationInstallationConfigurationError(IntegrationError):
     """
     Error when external API access is blocked due to configuration issues
     like permissions, visibility changes, or invalid project settings.

tests/sentry/integrations/tasks/test_sync_assignee_outbound.py

@@ -4,10 +4,12 @@
 
 from sentry.integrations.errors import OrganizationIntegrationNotFound
 from sentry.integrations.example import ExampleIntegration
+from sentry.integrations.mixins.issues import IntegrationSyncTargetNotFound
 from sentry.integrations.models import ExternalIssue, Integration
 from sentry.integrations.models.organization_integration import OrganizationIntegration
 from sentry.integrations.tasks import sync_assignee_outbound
 from sentry.integrations.types import EventLifecycleOutcome
+from sentry.shared_integrations.exceptions import IntegrationInstallationConfigurationError
 from sentry.testutils.asserts import assert_halt_metric, assert_success_metric
 from sentry.testutils.cases import TestCase
 from sentry.testutils.silo import assume_test_silo_mode_of
@@ -35,19 +37,23 @@ def setUp(self):
             },
         )
 
-    @mock.patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
-    @mock.patch.object(ExampleIntegration, "sync_assignee_outbound")
-    def test_syncs_outbound_assignee(self, mock_sync_assignee, mock_record_event):
-        external_issue = self.create_integration_external_issue(
+        # Create a shared external issue for most tests
+        self.external_issue = self.create_integration_external_issue(
             group=self.group,
             key="foo-1234",
             integration=self.example_integration,
         )
 
-        sync_assignee_outbound(external_issue.id, self.user.id, True, None)
+        # Verify the external issue was created successfully
+        assert ExternalIssue.objects.filter(id=self.external_issue.id).exists()
+
+    @mock.patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
+    @mock.patch.object(ExampleIntegration, "sync_assignee_outbound")
+    def test_syncs_outbound_assignee(self, mock_sync_assignee, mock_record_event):
+        sync_assignee_outbound(self.external_issue.id, self.user.id, True, None)
         mock_sync_assignee.assert_called_once()
         mock_sync_assignee.assert_called_with(
-            external_issue, mock.ANY, assign=True, assignment_source=None
+            self.external_issue, mock.ANY, assign=True, assignment_source=None
         )
 
         user_arg = mock_sync_assignee.call_args_list[0][0][1]
@@ -61,14 +67,8 @@ def test_syncs_outbound_assignee(self, mock_sync_assignee, mock_record_event):
     def test_sync_failure(self, mock_sync_assignee, mock_record_failure):
         mock_sync_assignee.side_effect = raise_sync_assignee_exception
 
-        external_issue = self.create_integration_external_issue(
-            group=self.group,
-            key="foo-1234",
-            integration=self.example_integration,
-        )
-
         with pytest.raises(Exception) as exc:
-            sync_assignee_outbound(external_issue.id, self.user.id, True, None)
+            sync_assignee_outbound(self.external_issue.id, self.user.id, True, None)
 
         assert exc.match("Something went wrong")
         mock_record_failure.assert_called_once()
@@ -86,13 +86,8 @@ def test_skips_syncing_if_should_sync_false(
         self, mock_should_sync, mock_sync_assignee, mock_record_event
     ):
         mock_should_sync.return_value = False
-        external_issue = self.create_integration_external_issue(
-            group=self.group,
-            key="foo-1234",
-            integration=self.example_integration,
-        )
 
-        sync_assignee_outbound(external_issue.id, self.user.id, True, None)
+        sync_assignee_outbound(self.external_issue.id, self.user.id, True, None)
         mock_sync_assignee.assert_not_called()
 
         assert mock_record_event.call_count == 2
@@ -120,38 +115,55 @@ def test_missing_issue_sync(self, mock_sync_assignee, mock_record_event):
 
     @mock.patch.object(ExampleIntegration, "sync_assignee_outbound")
     def test_missing_integration_installation(self, mock_sync_assignee):
-        external_issue = self.create_integration_external_issue(
-            group=self.group,
-            key="foo-1234",
-            integration=self.example_integration,
-        )
-
         # Delete all integrations, but ensure we still have an external issue
         with assume_test_silo_mode_of(Integration):
             Integration.objects.filter().delete()
 
-        assert ExternalIssue.objects.filter(id=external_issue.id).exists()
-        sync_assignee_outbound(external_issue.id, self.user.id, True, None)
+        sync_assignee_outbound(self.external_issue.id, self.user.id, True, None)
         mock_sync_assignee.assert_not_called()
 
     @mock.patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
     @mock.patch.object(ExampleIntegration, "sync_assignee_outbound")
     def test_missing_organization_integration(self, mock_sync_assignee, mock_record_event):
-        external_issue = self.create_integration_external_issue(
-            group=self.group,
-            key="foo-1234",
-            integration=self.example_integration,
-        )
-
         # Delete all organization integrations, but ensure we still have an external issue
         with assume_test_silo_mode_of(OrganizationIntegration):
             OrganizationIntegration.objects.filter().delete()
 
-        assert ExternalIssue.objects.filter(id=external_issue.id).exists()
-        sync_assignee_outbound(external_issue.id, self.user.id, True, None)
+        sync_assignee_outbound(self.external_issue.id, self.user.id, True, None)
         mock_sync_assignee.assert_not_called()
 
         assert mock_record_event.call_count == 2
         assert_halt_metric(
             mock_record_event, OrganizationIntegrationNotFound("missing org_integration")
         )
+
+    @mock.patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
+    @mock.patch.object(ExampleIntegration, "sync_assignee_outbound")
+    def test_integration_sync_target_not_found(self, mock_sync_assignee, mock_record_event):
+        mock_sync_assignee.side_effect = IntegrationSyncTargetNotFound("No matching user found")
+
+        sync_assignee_outbound(self.external_issue.id, self.user.id, True, None)
+        mock_sync_assignee.assert_called_once()
+
+        assert mock_record_event.call_count == 2
+        assert_halt_metric(
+            mock_record_event, IntegrationSyncTargetNotFound("No matching user found")
+        )
+
+    @mock.patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
+    @mock.patch.object(ExampleIntegration, "sync_assignee_outbound")
+    def test_integration_installation_configuration_error(
+        self, mock_sync_assignee, mock_record_event
+    ):
+        mock_sync_assignee.side_effect = IntegrationInstallationConfigurationError(
+            "Insufficient permissions to assign user"
+        )
+
+        sync_assignee_outbound(self.external_issue.id, self.user.id, True, None)
+        mock_sync_assignee.assert_called_once()
+
+        assert mock_record_event.call_count == 2
+        assert_halt_metric(
+            mock_record_event,
+            IntegrationInstallationConfigurationError("Insufficient permissions to assign user"),
+        )