Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/sentry/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -595,14 +595,17 @@ def as_int_choices(cls) -> Sequence[int]:
class SentryAppInstallationStatus:
PENDING = 0
INSTALLED = 1
PENDING_DELETION = 2
PENDING_STR = "pending"
INSTALLED_STR = "installed"
PENDING_DELETION_STR = "pending_deletion"

@classmethod
def as_choices(cls) -> Sequence[tuple[int, str]]:
return (
(cls.PENDING, cls.PENDING_STR),
(cls.INSTALLED, cls.INSTALLED_STR),
(cls.PENDING_DELETION, cls.PENDING_DELETION_STR),
)

@classmethod
Expand All @@ -611,6 +614,8 @@ def as_str(cls, status: int) -> str:
return cls.PENDING_STR
elif status == cls.INSTALLED:
return cls.INSTALLED_STR
elif status == cls.PENDING_DELETION:
return cls.PENDING_DELETION_STR
else:
raise ValueError(f"Not a SentryAppInstallationStatus int: {status!r}")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@
from rest_framework.request import Request
from rest_framework.response import Response

from sentry import analytics, audit_log, deletions
from sentry import analytics, audit_log
from sentry.analytics.events.sentry_app_uninstalled import SentryAppUninstalledEvent
from sentry.api.api_owners import ApiOwner
from sentry.api.api_publish_status import ApiPublishStatus
from sentry.api.base import control_silo_endpoint
from sentry.api.serializers import serialize
from sentry.constants import SentryAppInstallationStatus
from sentry.deletions.models.scheduleddeletion import ScheduledDeletion
from sentry.sentry_apps.api.bases.sentryapps import SentryAppInstallationBaseEndpoint
from sentry.sentry_apps.api.parsers.sentry_app_installation import SentryAppInstallationParser
from sentry.sentry_apps.api.serializers.sentry_app_installation import (
Expand Down Expand Up @@ -56,7 +58,8 @@ def delete(self, request: Request, installation) -> Response:
# if the error is from a request exception, log the error and continue
except RequestException as exc:
sentry_sdk.capture_exception(exc)
deletions.exec_sync(sentry_app_installation)
sentry_app_installation.update(status=SentryAppInstallationStatus.PENDING_DELETION)
ScheduledDeletion.schedule(sentry_app_installation, days=0, actor=request.user)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Async Deletion Causes Audit Log Inconsistency

Switching to asynchronous deletion means the API returns success (204) and audit/analytics events are recorded before the installation is actually removed, creating a race condition and inconsistent state. The audit log entry also lacks a link to the scheduled deletion, and the scheduled deletion record itself may be routed to the wrong database.

Fix in Cursor Fix in Web

create_audit_entry(
request=request,
organization_id=sentry_app_installation.organization_id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from rest_framework.request import Request
from rest_framework.response import Response

from sentry import analytics, audit_log, deletions, features
from sentry import analytics, audit_log, features
from sentry.analytics.events.sentry_app_deleted import SentryAppDeletedEvent
from sentry.analytics.events.sentry_app_schema_validation_error import (
SentryAppSchemaValidationError,
Expand Down Expand Up @@ -230,7 +230,6 @@ def delete(self, request: Request, sentry_app) -> Response:
SentryAppInstallationNotifier(
sentry_app_installation=install, user=request.user, action="deleted"
).run()
deletions.exec_sync(install)
except RequestException as exc:
sentry_sdk.capture_exception(exc)

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from fixtures.page_objects.organization_integration_settings import (
OrganizationSentryAppDetailViewPage,
)
from sentry.deletions.tasks.scheduled import run_scheduled_deletions_control
from sentry.sentry_apps.models.sentry_app_installation import SentryAppInstallation
from sentry.testutils.cases import AcceptanceTestCase
from sentry.testutils.silo import no_silo_test
Expand Down Expand Up @@ -49,6 +50,10 @@ def test_uninstallation(self) -> None:

detail_view_page.uninstall()
self.browser.wait_until('[data-test-id="toast-success"]')

with self.tasks():
run_scheduled_deletions_control()

assert not SentryAppInstallation.objects.filter(
organization_id=self.organization.id, sentry_app=self.sentry_app
)
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@
)
from sentry.analytics.events.sentry_app_uninstalled import SentryAppUninstalledEvent
from sentry.constants import SentryAppInstallationStatus
from sentry.deletions.tasks.scheduled import run_scheduled_deletions_control
from sentry.models.auditlogentry import AuditLogEntry
from sentry.sentry_apps.models.sentry_app_installation import SentryAppInstallation
from sentry.sentry_apps.token_exchange.grant_exchanger import GrantExchanger
from sentry.testutils.cases import APITestCase
from sentry.testutils.helpers.analytics import assert_last_analytics_event
Expand Down Expand Up @@ -120,6 +122,11 @@ def test_delete_install(self, record: MagicMock) -> None:
),
)

with self.tasks():
run_scheduled_deletions_control()

assert not SentryAppInstallation.objects.filter(id=self.installation2.id).exists()

response_body = json.loads(responses.calls[0].request.body)

assert response_body.get("installation").get("uuid") == self.installation2.uuid
Expand Down
Loading