chore(integrations): Changes exception type for missing org integrations, updates assignment sync to halt on this exception (#91542)

Author: GabeVillalobos

src/sentry/integrations/base.py

@@ -16,6 +16,7 @@
 from sentry.exceptions import InvalidIdentity
 from sentry.identity.services.identity import identity_service
 from sentry.identity.services.identity.model import RpcIdentity
+from sentry.integrations.errors import OrganizationIntegrationNotFound
 from sentry.integrations.models.external_actor import ExternalActor
 from sentry.integrations.models.integration import Integration
 from sentry.integrations.notify_disable import notify_disable
@@ -372,7 +373,7 @@ def org_integration(self) -> RpcOrganizationIntegration:
             organization_id=self.organization_id,
         )
         if integration is None:
-            raise NotFound("missing org_integration")
+            raise OrganizationIntegrationNotFound("missing org_integration")
         return integration
 
     @cached_property
@@ -442,7 +443,7 @@ def default_identity(self) -> RpcIdentity:
         """For Integrations that rely solely on user auth for authentication."""
         try:
             org_integration = self.org_integration
-        except NotFound:
+        except OrganizationIntegrationNotFound:
             raise Identity.DoesNotExist
         else:
             if org_integration.default_auth_id is None:

src/sentry/integrations/errors.py

@@ -4,3 +4,7 @@ class InvalidProviderException(Exception):
     """
 
     pass
+
+
+class OrganizationIntegrationNotFound(Exception):
+    pass

src/sentry/integrations/tasks/sync_assignee_outbound.py

@@ -2,6 +2,7 @@
 
 from sentry import analytics, features
 from sentry.constants import ObjectStatus
+from sentry.integrations.errors import OrganizationIntegrationNotFound
 from sentry.integrations.models.external_issue import ExternalIssue
 from sentry.integrations.models.integration import Integration
 from sentry.integrations.project_management.metrics import (
@@ -69,23 +70,31 @@ def sync_assignee_outbound(
         action_type=ProjectManagementActionType.OUTBOUND_ASSIGNMENT_SYNC, integration=integration
     ).capture() as lifecycle:
         lifecycle.add_extra("sync_task", "sync_assignee_outbound")
+        lifecycle.add_extra("organization_id", external_issue.organization_id)
+        lifecycle.add_extra("integration_id", integration.id)
+
         if not (
             hasattr(installation, "should_sync") and hasattr(installation, "sync_assignee_outbound")
         ):
             return
 
-        parsed_assignment_source = (
-            AssignmentSource.from_dict(assignment_source_dict) if assignment_source_dict else None
-        )
-        if installation.should_sync("outbound_assignee", parsed_assignment_source):
-            # Assume unassign if None.
-            user = user_service.get_user(user_id) if user_id else None
-            installation.sync_assignee_outbound(
-                external_issue, user, assign=assign, assignment_source=parsed_assignment_source
-            )
-            analytics.record(
-                "integration.issue.assignee.synced",
-                provider=integration.provider,
-                id=integration.id,
-                organization_id=external_issue.organization_id,
+        try:
+            parsed_assignment_source = (
+                AssignmentSource.from_dict(assignment_source_dict)
+                if assignment_source_dict
+                else None
             )
+            if installation.should_sync("outbound_assignee", parsed_assignment_source):
+                # Assume unassign if None.
+                user = user_service.get_user(user_id) if user_id else None
+                installation.sync_assignee_outbound(
+                    external_issue, user, assign=assign, assignment_source=parsed_assignment_source
+                )
+                analytics.record(
+                    "integration.issue.assignee.synced",
+                    provider=integration.provider,
+                    id=integration.id,
+                    organization_id=external_issue.organization_id,
+                )
+        except OrganizationIntegrationNotFound:
+            lifecycle.record_halt("organization_integration_not_found")

tests/sentry/integrations/tasks/test_sync_assignee_outbound.py

@@ -4,8 +4,10 @@
 
 from sentry.integrations.example import ExampleIntegration
 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.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
 from sentry.users.services.user import RpcUser
@@ -93,9 +95,7 @@ def test_skips_syncing_if_should_sync_false(
         mock_sync_assignee.assert_not_called()
 
         assert mock_record_event.call_count == 2
-        start, success = mock_record_event.mock_calls
-        assert start.args == (EventLifecycleOutcome.STARTED,)
-        assert success.args == (EventLifecycleOutcome.SUCCESS, None)
+        assert_success_metric(mock_record_event)
 
     @mock.patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
     @mock.patch.object(ExampleIntegration, "sync_assignee_outbound")
@@ -115,9 +115,7 @@ def test_missing_issue_sync(self, mock_sync_assignee, mock_record_event):
         # We don't want to log halt/failure metrics for these as it will taint
         # all non-sync integrations' metrics.
         assert mock_record_event.call_count == 2
-        start, success = mock_record_event.mock_calls
-        assert start.args == (EventLifecycleOutcome.STARTED,)
-        assert success.args == (EventLifecycleOutcome.SUCCESS, None)
+        assert_success_metric(mock_record_event)
 
     @mock.patch.object(ExampleIntegration, "sync_assignee_outbound")
     def test_missing_integration_installation(self, mock_sync_assignee):
@@ -134,3 +132,23 @@ def test_missing_integration_installation(self, mock_sync_assignee):
         assert ExternalIssue.objects.filter(id=external_issue.id).exists()
         sync_assignee_outbound(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)
+        mock_sync_assignee.assert_not_called()
+
+        assert mock_record_event.call_count == 2
+        assert_halt_metric(mock_record_event, "organization_integration_not_found")

tests/sentry/integrations/test_base.py

@@ -2,6 +2,7 @@
 
 from sentry.identity.services.identity.serial import serialize_identity
 from sentry.integrations.base import IntegrationInstallation
+from sentry.integrations.errors import OrganizationIntegrationNotFound
 from sentry.silo.base import SiloMode
 from sentry.testutils.cases import TestCase
 from sentry.testutils.silo import all_silo_test, assume_test_silo_mode
@@ -43,6 +44,9 @@ def test_with_context(self):
         assert integration.default_identity == serialize_identity(self.identity)
 
     def test_missing_org_integration(self):
+        with pytest.raises(OrganizationIntegrationNotFound):
+            ExampleIntegration(self.model, -1).org_integration
+
         with pytest.raises(Identity.DoesNotExist):
             ExampleIntegration(self.model, -1).default_identity