Skip to content

🔧 chore: classify vsts identity deleted errors as halt #92078

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 22, 2025
Merged
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
9 changes: 6 additions & 3 deletions src/sentry/integrations/tasks/sync_status_outbound.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from sentry import analytics, features
from sentry.constants import ObjectStatus
from sentry.integrations.base import IntegrationInstallation
from sentry.integrations.models.external_issue import ExternalIssue
from sentry.integrations.models.integration import Integration
from sentry.integrations.project_management.metrics import (
Expand All @@ -8,7 +9,7 @@
)
from sentry.integrations.services.integration import integration_service
from sentry.models.group import Group, GroupStatus
from sentry.shared_integrations.exceptions import IntegrationFormError
from sentry.shared_integrations.exceptions import ApiUnauthorized, IntegrationFormError
from sentry.silo.base import SiloMode
from sentry.tasks.base import instrumented_task, retry, track_group_async_operation
from sentry.taskworker.config import TaskworkerConfig
Expand Down Expand Up @@ -55,7 +56,9 @@ def sync_status_outbound(group_id: int, external_issue_id: int) -> bool | None:
)
if not integration:
return None
installation = integration.get_installation(organization_id=external_issue.organization_id)
installation: IntegrationInstallation = integration.get_installation(
organization_id=external_issue.organization_id
)
if not (hasattr(installation, "should_sync") and hasattr(installation, "sync_status_outbound")):
return None

Expand All @@ -76,7 +79,7 @@ def sync_status_outbound(group_id: int, external_issue_id: int) -> bool | None:
installation.sync_status_outbound(
external_issue, group.status == GroupStatus.RESOLVED, group.project_id
)
except IntegrationFormError as e:
except (IntegrationFormError, ApiUnauthorized) as e:
lifecycle.record_halt(halt_reason=e)
return None
analytics.record(
Expand Down
3 changes: 3 additions & 0 deletions src/sentry/integrations/utils/metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@ def __init__(self, payload: EventLifecycleMetric, assume_success: bool = True) -
self._state: EventLifecycleOutcome | None = None
self._extra = dict(self.payload.get_extras())

def get_state(self) -> EventLifecycleOutcome | None:
return self._state

def add_extra(self, name: str, value: Any) -> None:
"""Add a value to logged "extra" data.

Expand Down
17 changes: 16 additions & 1 deletion src/sentry/integrations/vsts/issues.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

from abc import ABC
from collections.abc import Mapping, MutableMapping, Sequence
from typing import TYPE_CHECKING, Any
from typing import TYPE_CHECKING, Any, NoReturn

from django.urls import reverse
from django.utils.translation import gettext as _
Expand All @@ -17,6 +17,7 @@
from sentry.models.activity import Activity
from sentry.shared_integrations.exceptions import ApiError, ApiUnauthorized, IntegrationError
from sentry.silo.base import all_silo_function
from sentry.users.models.identity import Identity
from sentry.users.models.user import User
from sentry.users.services.user import RpcUser
from sentry.users.services.user.service import user_service
Expand All @@ -25,6 +26,12 @@
from sentry.integrations.models.external_issue import ExternalIssue
from sentry.models.group import Group

# Specific substring to identify Azure Entra ID "identity deleted" errors
# Example: According to Microsoft Entra, your Identity xxx is currently Deleted within the following Microsoft Entra tenant: xxx Please contact your Microsoft Entra administrator to resolve this.
VSTS_IDENTITY_DELETED_ERROR_SUBSTRING = [
"is currently Deleted within the following Microsoft Entra tenant"
]


class VstsIssuesSpec(IssueSyncIntegration, SourceCodeIssueIntegration, ABC):
description = "Integrate Azure DevOps work items by linking a project."
Expand All @@ -46,6 +53,14 @@ def create_default_repo_choice(self, default_repo: str) -> tuple[str, str]:
project = self.get_client().get_project(default_repo)
return (project["id"], project["name"])

def raise_error(self, exc: Exception, identity: Identity | None = None) -> NoReturn:
# Reraise Azure Specific Errors correctly
if isinstance(exc, ApiError) and any(
substring in str(exc) for substring in VSTS_IDENTITY_DELETED_ERROR_SUBSTRING
):
raise ApiUnauthorized(text=str(exc))
raise super().raise_error(exc, identity)

def get_project_choices(
self, group: Group | None = None, **kwargs: Any
) -> tuple[str | None, Sequence[tuple[str, str]]]:
Expand Down
27 changes: 25 additions & 2 deletions tests/sentry/integrations/tasks/test_sync_status_outbound.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from sentry.integrations.models import ExternalIssue, Integration
from sentry.integrations.tasks import sync_status_outbound
from sentry.integrations.types import EventLifecycleOutcome
from sentry.shared_integrations.exceptions import IntegrationFormError
from sentry.shared_integrations.exceptions import ApiUnauthorized, IntegrationFormError
from sentry.testutils.asserts import assert_count_of_metric, assert_halt_metric
from sentry.testutils.cases import TestCase
from sentry.testutils.silo import assume_test_silo_mode_of, region_silo_test
Expand All @@ -20,6 +20,10 @@ def raise_integration_form_error(*args, **kwargs):
raise IntegrationFormError(field_errors={"foo": "Invalid foo provided"})


def raise_api_unauthorized_error(*args, **kwargs):
raise ApiUnauthorized(text="auth failed")


@region_silo_test
class TestSyncStatusOutbound(TestCase):
def setUp(self):
Expand Down Expand Up @@ -129,5 +133,24 @@ def test_integration_form_error(self, mock_sync_status, mock_record):
)

assert_halt_metric(
mock_record=mock_record, error_msg=IntegrationFormError({"error": "bruh"})
mock_record=mock_record, error_msg=IntegrationFormError({"foo": "Invalid foo provided"})
)

@mock.patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
@mock.patch.object(ExampleIntegration, "sync_status_outbound")
def test_api_unauthorized_error_halts(self, mock_sync_status, mock_record):
mock_sync_status.side_effect = raise_api_unauthorized_error
external_issue: ExternalIssue = self.create_integration_external_issue(
group=self.group, key="foo_integration", integration=self.example_integration
)

sync_status_outbound(self.group.id, external_issue_id=external_issue.id)

assert_count_of_metric(
mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=1
)
assert_count_of_metric(
mock_record=mock_record, outcome=EventLifecycleOutcome.HALTED, outcome_count=1
)

assert_halt_metric(mock_record=mock_record, error_msg=ApiUnauthorized("auth failed"))
12 changes: 11 additions & 1 deletion tests/sentry/integrations/vsts/test_issues.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
from sentry.integrations.models.integration_external_project import IntegrationExternalProject
from sentry.integrations.services.integration import integration_service
from sentry.integrations.vsts.integration import VstsIntegration
from sentry.shared_integrations.exceptions import IntegrationError
from sentry.shared_integrations.exceptions import ApiError, ApiUnauthorized, IntegrationError
from sentry.silo.base import SiloMode
from sentry.silo.util import PROXY_PATH
from sentry.testutils.cases import TestCase
Expand Down Expand Up @@ -591,3 +591,13 @@ def test_default_project_no_projects(self):
fields = self.integration.get_create_issue_config(self.group, self.user)

self.assert_project_field(fields, None, [])


@region_silo_test
class VstsIssueRaiseErrorTest(VstsIssueBase):
@responses.activate
def test_raise_error_api_unauthorized(self):
error_message = "According to Microsoft Entra, your Identity xxx is currently Deleted within the following Microsoft Entra tenant: xxx Please contact your Microsoft Entra administrator to resolve this."
api_error = ApiError(error_message)
with pytest.raises(ApiUnauthorized):
self.integration.raise_error(api_error)
Loading