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

Author: GabeVillalobos

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
@@ -1272,30 +1272,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.integrations.types import IntegrationProviderSlug
@@ -21,6 +21,7 @@
     ApiUnauthorized,
     IntegrationError,
     IntegrationFormError,
+    IntegrationInstallationConfigurationError,
 )
 from sentry.silo.base import all_silo_function
 from sentry.users.models.identity import Identity
@@ -295,11 +296,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={
@@ -308,6 +309,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/jira/test_integration.py

@@ -9,17 +9,20 @@
 
 from fixtures.integrations.jira.stub_client import StubJiraApiClient
 from fixtures.integrations.stub_service import StubService
-from sentry.exceptions import InvalidConfiguration
 from sentry.integrations.jira.integration import JiraIntegrationProvider
 from sentry.integrations.jira.views import SALT
+from sentry.integrations.mixins.issues import IntegrationSyncTargetNotFound
 from sentry.integrations.models.external_issue import ExternalIssue
 from sentry.integrations.models.integration import Integration
 from sentry.integrations.models.integration_external_project import IntegrationExternalProject
 from sentry.integrations.models.organization_integration import OrganizationIntegration
 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 IntegrationError
+from sentry.shared_integrations.exceptions import (
+    IntegrationError,
+    IntegrationInstallationConfigurationError,
+)
 from sentry.silo.base import SiloMode
 from sentry.testutils.cases import APITestCase, IntegrationTestCase
 from sentry.testutils.factories import EventType
@@ -825,7 +828,7 @@ def test_sync_assignee_outbound_no_email(self):
             "https://example.atlassian.net/rest/api/2/user/assignable/search",
             json=[{"accountId": "deadbeef123", "displayName": "Dead Beef"}],
         )
-        with pytest.raises(InvalidConfiguration):
+        with pytest.raises(IntegrationSyncTargetNotFound):
             installation.sync_assignee_outbound(external_issue, user)
 
         # No sync made as jira users don't have email addresses
@@ -866,6 +869,56 @@ def test_sync_assignee_outbound_use_email_api(self):
         assert assign_issue_response.status_code == 200
         assert assign_issue_response.request.body == b'{"accountId": "deadbeef123"}'
 
+    @responses.activate
+    def test_sync_assignee_outbound_api_unauthorized(self):
+        user = serialize_rpc_user(self.create_user(email="[email protected]"))
+        issue_id = "APP-123"
+        installation = self.integration.get_installation(self.organization.id)
+        assign_issue_url = "https://example.atlassian.net/rest/api/2/issue/%s/assignee" % issue_id
+
+        external_issue = ExternalIssue.objects.create(
+            organization_id=self.organization.id, integration_id=installation.model.id, key=issue_id
+        )
+
+        responses.add(
+            responses.GET,
+            "https://example.atlassian.net/rest/api/2/user/assignable/search",
+            json=[{"accountId": "deadbeef123", "emailAddress": "[email protected]"}],
+        )
+
+        responses.add(responses.PUT, assign_issue_url, status=401, json={})
+
+        with pytest.raises(IntegrationInstallationConfigurationError) as excinfo:
+            installation.sync_assignee_outbound(external_issue, user)
+
+        assert str(excinfo.value) == "Insufficient permissions to assign user to the Jira issue."
+        assert len(responses.calls) == 2
+
+    @responses.activate
+    def test_sync_assignee_outbound_api_error(self):
+        user = serialize_rpc_user(self.create_user(email="[email protected]"))
+        issue_id = "APP-123"
+        installation = self.integration.get_installation(self.organization.id)
+        assign_issue_url = "https://example.atlassian.net/rest/api/2/issue/%s/assignee" % issue_id
+
+        external_issue = ExternalIssue.objects.create(
+            organization_id=self.organization.id, integration_id=installation.model.id, key=issue_id
+        )
+
+        responses.add(
+            responses.GET,
+            "https://example.atlassian.net/rest/api/2/user/assignable/search",
+            json=[{"accountId": "deadbeef123", "emailAddress": "[email protected]"}],
+        )
+
+        responses.add(responses.PUT, assign_issue_url, status=400, json={})
+
+        with pytest.raises(IntegrationError) as excinfo:
+            installation.sync_assignee_outbound(external_issue, user)
+
+        assert str(excinfo.value) == "There was an error assigning the issue."
+        assert len(responses.calls) == 2
+
 
 @control_silo_test
 class JiraIntegrationTest(APITestCase):

tests/sentry/integrations/jira_server/test_integration.py

@@ -11,6 +11,7 @@
 from fixtures.integrations.jira.stub_client import StubJiraApiClient
 from fixtures.integrations.stub_service import StubService
 from sentry.integrations.jira_server.integration import JiraServerIntegration
+from sentry.integrations.mixins.issues import IntegrationSyncTargetNotFound
 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
@@ -19,7 +20,12 @@
 from sentry.integrations.types import ExternalProviders
 from sentry.models.grouplink import GroupLink
 from sentry.models.groupmeta import GroupMeta
-from sentry.shared_integrations.exceptions import ApiError, ApiUnauthorized, IntegrationError
+from sentry.shared_integrations.exceptions import (
+    ApiError,
+    ApiUnauthorized,
+    IntegrationError,
+    IntegrationInstallationConfigurationError,
+)
 from sentry.silo.base import SiloMode
 from sentry.silo.safety import unguarded_write
 from sentry.testutils.cases import APITestCase
@@ -819,9 +825,9 @@ def test_sync_assignee_outbound_no_emails_for_multiple_users(self):
             ],
         )
 
-        with pytest.raises(IntegrationError) as e:
+        with pytest.raises(IntegrationSyncTargetNotFound) as e:
             self.installation.sync_assignee_outbound(external_issue, user)
-        assert str(e.value) == "Failed to assign user to Jira Server issue"
+        assert str(e.value) == "No matching Jira Server user found"
 
         # No sync made as jira users don't have email addresses
         assert len(responses.calls) == 1
@@ -877,7 +883,7 @@ def test_sync_assignee_outbound_unauthorized(self, mock_assign_issue):
                 }
             ],
         )
-        with pytest.raises(IntegrationError) as exc_info:
+        with pytest.raises(IntegrationInstallationConfigurationError) as exc_info:
             self.installation.sync_assignee_outbound(
                 external_issue=external_issue, user=user, assign=True
             )
@@ -902,7 +908,7 @@ def test_sync_assignee_outbound_api_error(self, mock_assign_issue):
                 {
                     "accountId": "deadbeef123",
                     "displayName": "Dead Beef",
-                    "username": "[email protected]",
+                    "emailAddress": "[email protected]",
                 }
             ],
         )