fix(deletions): Prevent DB call from deleting issue

src/sentry/api/helpers/group_index/delete.py

@@ -73,6 +73,9 @@ def delete_group_list(
         },
     )
 
+    # Tell seer to delete grouping records for these groups
+    may_schedule_task_to_delete_hashes_from_seer(error_ids)
+
     Group.objects.filter(id__in=group_ids).exclude(
         status__in=[GroupStatus.PENDING_DELETION, GroupStatus.DELETION_IN_PROGRESS]
     ).update(status=GroupStatus.PENDING_DELETION, substatus=None)
@@ -84,9 +87,6 @@ def delete_group_list(
     # fails, we will still have a record of who requested the deletion.
     create_audit_entries(request, project, group_list, delete_type, transaction_id)
 
-    # Tell seer to delete grouping records for these groups
-    may_schedule_task_to_delete_hashes_from_seer(error_ids)
-
     # Removing GroupHash rows prevents new events from associating to the groups
     # we just deleted.
     GroupHash.objects.filter(project_id=project.id, group__id__in=group_ids).delete()

src/sentry/issues/endpoints/organization_group_index.py

@@ -1,4 +1,5 @@
 import functools
+import logging
 from collections.abc import Mapping, Sequence
 from datetime import datetime, timedelta
 from typing import Any
@@ -67,6 +68,8 @@
 ERR_INVALID_STATS_PERIOD = "Invalid stats_period. Valid choices are '', '24h', '14d' and 'auto'"
 allowed_inbox_search_terms = frozenset(["date", "status", "for_review", "assigned_or_suggested"])
 
+logger = logging.getLogger(__name__)
+
 
 def inbox_search(
     projects: Sequence[Project],
@@ -501,4 +504,8 @@ def delete(self, request: Request, organization: Organization) -> Response:
             self.get_environments(request, organization),
         )
 
-        return delete_groups(request, projects, organization.id, search_fn)
+        try:
+            return delete_groups(request, projects, organization.id, search_fn)
+        except Exception:
+            logger.exception("Error deleting groups")
+            return Response({"detail": "Error deleting groups"}, status=500)

src/sentry/issues/endpoints/project_group_index.py

@@ -1,4 +1,5 @@
 import functools
+import logging
 
 from rest_framework.request import Request
 from rest_framework.response import Response
@@ -30,6 +31,7 @@
 
 ERR_INVALID_STATS_PERIOD = "Invalid stats_period. Valid choices are '', '24h', and '14d'"
 ERR_HASHES_AND_OTHER_QUERY = "Cannot use 'hashes' with 'query'"
+logger = logging.getLogger(__name__)
 
 
 @region_silo_endpoint
@@ -299,4 +301,8 @@ def delete(self, request: Request, project: Project) -> Response:
         :auth: required
         """
         search_fn = functools.partial(prep_search, request, project)
-        return delete_groups(request, [project], project.organization_id, search_fn)
+        try:
+            return delete_groups(request, [project], project.organization_id, search_fn)
+        except Exception:
+            logger.exception("Error deleting groups")
+            return Response({"detail": "Error deleting groups"}, status=500)

tests/sentry/issues/endpoints/test_organization_group_index.py

@@ -7,7 +7,7 @@
 from unittest.mock import MagicMock, Mock, call, patch
 from uuid import uuid4
 
-from django.db import OperationalError
+from django.db.utils import OperationalError
 from django.urls import reverse
 from django.utils import timezone
 
@@ -4455,3 +4455,26 @@ def test_bulk_delete_performance_issues(self) -> None:
             assert response.status_code == 204
 
         self.assert_deleted_groups(groups)
+
+    @patch("sentry.api.helpers.group_index.delete.may_schedule_task_to_delete_hashes_from_seer")
+    def test_do_not_mark_as_pending_deletion_if_seer_fails(self, mock_seer_delete: Mock):
+        """
+        Test that the issue is not marked as pending deletion if the seer call fails.
+        """
+        # When trying to gather the hashes, the query could be cancelled by the user
+        mock_seer_delete.side_effect = OperationalError(
+            "QueryCanceled('canceling statement due to user request\n')"
+        )
+        event = self.store_event(data={}, project_id=self.project.id)
+        group1 = Group.objects.get(id=event.group_id)
+        assert GroupHash.objects.filter(group=group1).exists()
+
+        self.login_as(user=self.user)
+        with self.tasks():
+            response = self.get_response(qs_params={"id": [group1.id]})
+            assert response.status_code == 500
+            assert response.data["detail"] == "Error deleting groups"
+
+        # The group has not been marked as pending deletion
+        assert Group.objects.get(id=group1.id).status == group1.status
+        assert GroupHash.objects.filter(group=group1).exists()

tests/snuba/api/endpoints/test_project_group_index.py

@@ -9,6 +9,7 @@
 from uuid import uuid4
 
 from django.conf import settings
+from django.db.utils import OperationalError
 from django.utils import timezone
 
 from sentry.integrations.models.external_issue import ExternalIssue
@@ -1637,26 +1638,28 @@ def test_bulk_delete_performance_issues(self):
             self.assert_groups_are_gone(groups)
 
     @patch("sentry.api.helpers.group_index.delete.may_schedule_task_to_delete_hashes_from_seer")
-    @patch("sentry.utils.audit.log_service.record_audit_log")
-    def test_audit_log_even_if_exception_raised(
-        self, mock_record_audit_log: Mock, mock_seer_delete: Mock
-    ):
+    def test_do_not_mark_as_pending_deletion_if_seer_fails(self, mock_seer_delete: Mock):
         """
-        Test that audit log is created even if an exception is raised after the audit log is created.
+        Test that the issue is not marked as pending deletion if the seer call fails.
         """
-        # Calling seer happens after creating the audit log entry
-        mock_seer_delete.side_effect = Exception("Seer error!")
-        group1 = self.create_group()
+        # When trying to gather the hashes, the query could be cancelled by the user
+        mock_seer_delete.side_effect = OperationalError(
+            "QueryCanceled('canceling statement due to user request\n')"
+        )
+        event = self.store_event(data={}, project_id=self.project.id)
+        group1 = Group.objects.get(id=event.group_id)
+        assert GroupHash.objects.filter(group=group1).exists()
+
         self.login_as(user=self.user)
         url = f"{self.path}?id={group1.id}"
         with self.tasks():
             response = self.client.delete(url, format="json")
             assert response.status_code == 500
+            assert response.data["detail"] == "Error deleting groups"
 
-        self.assert_audit_log_entry([group1], mock_record_audit_log)
-
-        # They have been marked as pending deletion but the exception prevented their complete deletion
-        assert Group.objects.get(id=group1.id).status == GroupStatus.PENDING_DELETION
+        # The group has not been marked as pending deletion
+        assert Group.objects.get(id=group1.id).status == group1.status
+        assert GroupHash.objects.filter(group=group1).exists()
 
     def test_new_event_for_pending_deletion_group_creates_new_group(self):
         """Test that after deleting a group, new events with the same fingerprint create a new group."""