From d7f42249fcbe6ca1e3dbc9b9eed689129dbc2ae5 Mon Sep 17 00:00:00 2001 From: Christinarlong Date: Tue, 27 May 2025 14:21:07 -0700 Subject: [PATCH 1/5] add more logs/expose them better --- src/sentry/integrations/jira/integration.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/sentry/integrations/jira/integration.py b/src/sentry/integrations/jira/integration.py index a0e20abcf8a222..40efc4fee6504e 100644 --- a/src/sentry/integrations/jira/integration.py +++ b/src/sentry/integrations/jira/integration.py @@ -1007,16 +1007,18 @@ def sync_assignee_outbound( "integration_id": external_issue.integration_id, "user_id": user.id, "issue_key": external_issue.key, + "organization_id": external_issue.organization_id, }, ) - return + raise try: id_field = client.user_id_field() client.assign_issue(external_issue.key, jira_user and jira_user.get(id_field)) - except (ApiUnauthorized, ApiError): + except (ApiUnauthorized, ApiError) as e: # TODO(jess): do we want to email people about these types of failures? logger.info( "jira.failed-to-assign", + exc_info=e, extra={ "organization_id": external_issue.organization_id, "integration_id": external_issue.integration_id, @@ -1024,6 +1026,7 @@ def sync_assignee_outbound( "issue_key": external_issue.key, }, ) + raise def sync_status_outbound( self, external_issue: ExternalIssue, is_resolved: bool, project_id: int From 51dd0f3aea2524482c50af5b5079a5b59701d0f7 Mon Sep 17 00:00:00 2001 From: Christinarlong Date: Tue, 27 May 2025 14:25:19 -0700 Subject: [PATCH 2/5] add more logs/expose them better --- src/sentry/integrations/jira/integration.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/sentry/integrations/jira/integration.py b/src/sentry/integrations/jira/integration.py index 40efc4fee6504e..9f3af33f5f8d3a 100644 --- a/src/sentry/integrations/jira/integration.py +++ b/src/sentry/integrations/jira/integration.py @@ -1006,11 +1006,12 @@ def sync_assignee_outbound( extra={ "integration_id": external_issue.integration_id, "user_id": user.id, + "user_emails": user.emails, "issue_key": external_issue.key, "organization_id": external_issue.organization_id, }, ) - raise + raise IntegrationFormError("Unable to find the requested user from Jira") try: id_field = client.user_id_field() client.assign_issue(external_issue.key, jira_user and jira_user.get(id_field)) From 50ba0703d53bbaf2a087ae3bf4312f1744d49490 Mon Sep 17 00:00:00 2001 From: Christinarlong Date: Tue, 27 May 2025 14:25:30 -0700 Subject: [PATCH 3/5] add more logs/expose them better --- src/sentry/integrations/jira/integration.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry/integrations/jira/integration.py b/src/sentry/integrations/jira/integration.py index 9f3af33f5f8d3a..d598d0218509a2 100644 --- a/src/sentry/integrations/jira/integration.py +++ b/src/sentry/integrations/jira/integration.py @@ -1011,7 +1011,7 @@ def sync_assignee_outbound( "organization_id": external_issue.organization_id, }, ) - raise IntegrationFormError("Unable to find the requested user from Jira") + raise IntegrationFormError("Unable to find the requested user") try: id_field = client.user_id_field() client.assign_issue(external_issue.key, jira_user and jira_user.get(id_field)) From 10632a53809888fec9f72f85d6ab34ede8c7209f Mon Sep 17 00:00:00 2001 From: Christinarlong Date: Tue, 27 May 2025 15:11:05 -0700 Subject: [PATCH 4/5] fix tets and typing --- src/sentry/integrations/jira/integration.py | 2 +- tests/sentry/integrations/jira/test_integration.py | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/sentry/integrations/jira/integration.py b/src/sentry/integrations/jira/integration.py index d598d0218509a2..ce1d51b2701596 100644 --- a/src/sentry/integrations/jira/integration.py +++ b/src/sentry/integrations/jira/integration.py @@ -1011,7 +1011,7 @@ def sync_assignee_outbound( "organization_id": external_issue.organization_id, }, ) - raise IntegrationFormError("Unable to find the requested user") + raise IntegrationFormError({"email": "Unable to find the requested user"}) try: id_field = client.user_id_field() client.assign_issue(external_issue.key, jira_user and jira_user.get(id_field)) diff --git a/tests/sentry/integrations/jira/test_integration.py b/tests/sentry/integrations/jira/test_integration.py index afb19c471922eb..c6ce3c06a9e206 100644 --- a/tests/sentry/integrations/jira/test_integration.py +++ b/tests/sentry/integrations/jira/test_integration.py @@ -18,7 +18,7 @@ from sentry.integrations.services.integration import integration_service from sentry.models.grouplink import GroupLink from sentry.models.groupmeta import GroupMeta -from sentry.shared_integrations.exceptions import IntegrationError +from sentry.shared_integrations.exceptions import IntegrationError, IntegrationFormError from sentry.silo.base import SiloMode from sentry.testutils.cases import APITestCase, IntegrationTestCase from sentry.testutils.factories import EventType @@ -824,7 +824,8 @@ def test_sync_assignee_outbound_no_email(self): "https://example.atlassian.net/rest/api/2/user/assignable/search", json=[{"accountId": "deadbeef123", "displayName": "Dead Beef"}], ) - installation.sync_assignee_outbound(external_issue, user) + with pytest.raises(IntegrationFormError): + installation.sync_assignee_outbound(external_issue, user) # No sync made as jira users don't have email addresses assert len(responses.calls) == 1 From dcec3e8be6b18b0c6548c93e02f3a9577e39a41c Mon Sep 17 00:00:00 2001 From: Christinarlong Date: Wed, 28 May 2025 10:08:42 -0700 Subject: [PATCH 5/5] use different error instead of integrationformerror --- src/sentry/integrations/jira/integration.py | 12 ++++++++++-- .../integrations/tasks/sync_assignee_outbound.py | 7 ++++--- tests/sentry/integrations/jira/test_integration.py | 5 +++-- .../tasks/test_sync_assignee_outbound.py | 5 ++++- 4 files changed, 21 insertions(+), 8 deletions(-) diff --git a/src/sentry/integrations/jira/integration.py b/src/sentry/integrations/jira/integration.py index ce1d51b2701596..441da6bf7c7b9d 100644 --- a/src/sentry/integrations/jira/integration.py +++ b/src/sentry/integrations/jira/integration.py @@ -15,6 +15,7 @@ from sentry import features from sentry.eventstore.models import GroupEvent +from sentry.exceptions import InvalidConfiguration from sentry.integrations.base import ( FeatureDescription, IntegrationData, @@ -1011,11 +1012,18 @@ def sync_assignee_outbound( "organization_id": external_issue.organization_id, }, ) - raise IntegrationFormError({"email": "Unable to find the requested user"}) + if not user.emails: + raise InvalidConfiguration( + { + "email": "User must have a verified email on Sentry to sync assignee in Jira", + "help": "https://sentry.io/settings/account/emails", + } + ) + raise InvalidConfiguration({"email": "Unable to find the requested user"}) try: id_field = client.user_id_field() client.assign_issue(external_issue.key, jira_user and jira_user.get(id_field)) - except (ApiUnauthorized, ApiError) as e: + except ApiError as e: # TODO(jess): do we want to email people about these types of failures? logger.info( "jira.failed-to-assign", diff --git a/src/sentry/integrations/tasks/sync_assignee_outbound.py b/src/sentry/integrations/tasks/sync_assignee_outbound.py index 0d796c47d34d7b..d1b8f0eda09acb 100644 --- a/src/sentry/integrations/tasks/sync_assignee_outbound.py +++ b/src/sentry/integrations/tasks/sync_assignee_outbound.py @@ -2,6 +2,7 @@ from sentry import analytics, features from sentry.constants import ObjectStatus +from sentry.exceptions import InvalidConfiguration from sentry.integrations.errors import OrganizationIntegrationNotFound from sentry.integrations.models.external_issue import ExternalIssue from sentry.integrations.models.integration import Integration @@ -12,7 +13,7 @@ from sentry.integrations.services.assignment_source import AssignmentSource from sentry.integrations.services.integration import integration_service from sentry.models.organization import Organization -from sentry.shared_integrations.exceptions import IntegrationError +from sentry.shared_integrations.exceptions import ApiUnauthorized, IntegrationError from sentry.silo.base import SiloMode from sentry.tasks.base import instrumented_task, retry from sentry.taskworker.config import TaskworkerConfig @@ -96,5 +97,5 @@ def sync_assignee_outbound( id=integration.id, organization_id=external_issue.organization_id, ) - except OrganizationIntegrationNotFound: - lifecycle.record_halt("organization_integration_not_found") + except (OrganizationIntegrationNotFound, ApiUnauthorized, InvalidConfiguration) as e: + lifecycle.record_halt(halt_reason=e) diff --git a/tests/sentry/integrations/jira/test_integration.py b/tests/sentry/integrations/jira/test_integration.py index c6ce3c06a9e206..96bcbf70004245 100644 --- a/tests/sentry/integrations/jira/test_integration.py +++ b/tests/sentry/integrations/jira/test_integration.py @@ -9,6 +9,7 @@ from fixtures.integrations.jira.stub_client import StubJiraApiClient from fixtures.integrations.stub_service import StubService +from sentry.exceptions import InvalidConfiguration from sentry.integrations.jira.integration import JiraIntegrationProvider from sentry.integrations.jira.views import SALT from sentry.integrations.models.external_issue import ExternalIssue @@ -18,7 +19,7 @@ from sentry.integrations.services.integration import integration_service from sentry.models.grouplink import GroupLink from sentry.models.groupmeta import GroupMeta -from sentry.shared_integrations.exceptions import IntegrationError, IntegrationFormError +from sentry.shared_integrations.exceptions import IntegrationError from sentry.silo.base import SiloMode from sentry.testutils.cases import APITestCase, IntegrationTestCase from sentry.testutils.factories import EventType @@ -824,7 +825,7 @@ def test_sync_assignee_outbound_no_email(self): "https://example.atlassian.net/rest/api/2/user/assignable/search", json=[{"accountId": "deadbeef123", "displayName": "Dead Beef"}], ) - with pytest.raises(IntegrationFormError): + with pytest.raises(InvalidConfiguration): installation.sync_assignee_outbound(external_issue, user) # No sync made as jira users don't have email addresses diff --git a/tests/sentry/integrations/tasks/test_sync_assignee_outbound.py b/tests/sentry/integrations/tasks/test_sync_assignee_outbound.py index 09afd63c871f08..ea36b6ba958a44 100644 --- a/tests/sentry/integrations/tasks/test_sync_assignee_outbound.py +++ b/tests/sentry/integrations/tasks/test_sync_assignee_outbound.py @@ -2,6 +2,7 @@ import pytest +from sentry.integrations.errors import OrganizationIntegrationNotFound from sentry.integrations.example import ExampleIntegration from sentry.integrations.models import ExternalIssue, Integration from sentry.integrations.models.organization_integration import OrganizationIntegration @@ -151,4 +152,6 @@ def test_missing_organization_integration(self, mock_sync_assignee, mock_record_ mock_sync_assignee.assert_not_called() assert mock_record_event.call_count == 2 - assert_halt_metric(mock_record_event, "organization_integration_not_found") + assert_halt_metric( + mock_record_event, OrganizationIntegrationNotFound("missing org_integration") + )