From 697311a669e73f84e98ca309f87c77d362f05008 Mon Sep 17 00:00:00 2001 From: Christinarlong Date: Wed, 9 Sep 2026 10:04:34 -0700 Subject: [PATCH 1/2] add func to self heal hooks in background w/o org id --- src/sentry/sentry_apps/tasks/sentry_apps.py | 33 ++++++++++++ .../sentry_apps/tasks/test_sentry_apps.py | 54 +++++++++++++++++++ 2 files changed, 87 insertions(+) diff --git a/src/sentry/sentry_apps/tasks/sentry_apps.py b/src/sentry/sentry_apps/tasks/sentry_apps.py index 2d671ebd69b5..c2ce7a24be48 100644 --- a/src/sentry/sentry_apps/tasks/sentry_apps.py +++ b/src/sentry/sentry_apps/tasks/sentry_apps.py @@ -415,8 +415,41 @@ def _load_service_hook(organization_id: int | None, installation_id: int) -> Ser ) return service_hook except ServiceHook.DoesNotExist: + # Attempt to repair the hook if the organization_id is missing + return _repair_hook_missing_organization_id(organization_id, installation_id) + + +def _repair_hook_missing_organization_id( + organization_id: int | None, installation_id: int +) -> ServiceHook | None: + """ + Attempt to repair the hook if the organization_id is missing (there was a gap from + between 2025-08-26 and 2026-02-18) + TODO: Remove this once the gap is closed + """ + if organization_id is None: return None + try: + service_hook = ServiceHook.objects.get( + installation_id=installation_id, + organization_id__isnull=True, + ) + except ServiceHook.DoesNotExist: + return None + + service_hook.organization_id = organization_id + service_hook.save(update_fields=["organization_id"]) + logger.info( + "service_hook.repaired_missing_organization_id", + extra={ + "service_hook_id": service_hook.id, + "installation_id": installation_id, + "organization_id": organization_id, + }, + ) + return service_hook + @cache_func_for_models( [(ServiceHookProject, lambda hook_project: (hook_project.service_hook_id,))], diff --git a/tests/sentry/sentry_apps/tasks/test_sentry_apps.py b/tests/sentry/sentry_apps/tasks/test_sentry_apps.py index 1444977dd777..591725d65985 100644 --- a/tests/sentry/sentry_apps/tasks/test_sentry_apps.py +++ b/tests/sentry/sentry_apps/tasks/test_sentry_apps.py @@ -1708,6 +1708,60 @@ def test_does_not_send_if_event_not_in_app_events( mock_record=mock_record, outcome=EventLifecycleOutcome.FAILURE, outcome_count=1 ) + def test_repairs_service_hook_missing_organization_id(self, safe_urlopen: MagicMock) -> None: + with assume_test_silo_mode_of(ServiceHook): + ServiceHook.objects.filter(installation_id=self.install.id).update(organization_id=None) + + workflow_notification(self.install.id, self.issue.id, "resolved", self.user.id) + + ((_, kwargs),) = safe_urlopen.call_args_list + assert kwargs["url"] == self.sentry_app.webhook_url + + with assume_test_silo_mode_of(ServiceHook): + hook = ServiceHook.objects.get(installation_id=self.install.id) + assert hook.organization_id == self.project.organization.id + + def test_does_not_repair_when_multiple_hooks_missing_organization_id( + self, safe_urlopen: MagicMock + ) -> None: + self.create_service_hook( + actor=self.user, + org=self.project.organization, + project_ids=[], + events=["issue.resolved"], + installation_id=self.install.id, + application_id=self.sentry_app.application_id, + url=self.sentry_app.webhook_url, + ) + with assume_test_silo_mode_of(ServiceHook): + ServiceHook.objects.filter(installation_id=self.install.id).update(organization_id=None) + + with pytest.raises(SentryAppSentryError): + workflow_notification(self.install.id, self.issue.id, "resolved", self.user.id) + assert not safe_urlopen.called + + with assume_test_silo_mode_of(ServiceHook): + assert not ServiceHook.objects.filter( + installation_id=self.install.id, organization_id__isnull=False + ).exists() + + def test_does_not_repair_hook_belonging_to_another_organization( + self, safe_urlopen: MagicMock + ) -> None: + other_org = self.create_organization() + with assume_test_silo_mode_of(ServiceHook): + ServiceHook.objects.filter(installation_id=self.install.id).update( + organization_id=other_org.id + ) + + with pytest.raises(SentryAppSentryError): + workflow_notification(self.install.id, self.issue.id, "resolved", self.user.id) + assert not safe_urlopen.called + + with assume_test_silo_mode_of(ServiceHook): + hook = ServiceHook.objects.get(installation_id=self.install.id) + assert hook.organization_id == other_org.id + class TestWebhookRequests(TestCase): def setUp(self) -> None: From 9a7541a7f61b6191b5928064bd94325e8f1c30fd Mon Sep 17 00:00:00 2001 From: Christinarlong Date: Wed, 9 Sep 2026 10:28:01 -0700 Subject: [PATCH 2/2] nevermind I we will handle mult objects returned --- src/sentry/sentry_apps/tasks/sentry_apps.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/sentry/sentry_apps/tasks/sentry_apps.py b/src/sentry/sentry_apps/tasks/sentry_apps.py index c2ce7a24be48..7e814442adbc 100644 --- a/src/sentry/sentry_apps/tasks/sentry_apps.py +++ b/src/sentry/sentry_apps/tasks/sentry_apps.py @@ -437,6 +437,14 @@ def _repair_hook_missing_organization_id( ) except ServiceHook.DoesNotExist: return None + except ServiceHook.MultipleObjectsReturned: + # We can't tell which hook is live, and guessing would send an org's payloads + # to the wrong url. Fall through to the missing_servicehook halt instead. + logger.warning( + "service_hook.duplicate_hooks_missing_organization_id", + extra={"installation_id": installation_id}, + ) + return None service_hook.organization_id = organization_id service_hook.save(update_fields=["organization_id"])