diff --git a/src/sentry/preprod/vcs/status_checks/size/tasks.py b/src/sentry/preprod/vcs/status_checks/size/tasks.py index 8dc40980600a16..8efd3e2f90b5d9 100644 --- a/src/sentry/preprod/vcs/status_checks/size/tasks.py +++ b/src/sentry/preprod/vcs/status_checks/size/tasks.py @@ -24,6 +24,7 @@ from sentry.preprod.models import PreprodArtifact, PreprodArtifactSizeMetrics from sentry.preprod.url_utils import get_preprod_artifact_url from sentry.preprod.vcs.status_checks.size.templates import format_status_check_messages +from sentry.shared_integrations.exceptions import ApiError, IntegrationConfigurationError from sentry.silo.base import SiloMode from sentry.tasks.base import instrumented_task from sentry.taskworker.namespaces import integrations_tasks @@ -36,7 +37,7 @@ name="sentry.preprod.tasks.create_preprod_status_check", namespace=integrations_tasks, processing_deadline_duration=30, - retry=Retry(times=3), + retry=Retry(times=3, ignore=(IntegrationConfigurationError,)), silo_mode=SiloMode.REGION, ) def create_preprod_status_check_task(preprod_artifact_id: int) -> None: @@ -351,9 +352,47 @@ def create_status_check( response = self.client.create_check_run(repo=repo, data=check_data) check_id = response.get("id") return str(check_id) if check_id else None - except Exception as e: + except ApiError as e: lifecycle.record_failure(e) - return None + # Only convert specific permission 403s as IntegrationConfigurationError + # GitHub can return 403 for various reasons (rate limits, temporary issues, permissions) + if e.code == 403: + error_message = str(e).lower() + if ( + "resource not accessible" in error_message + or "insufficient" in error_message + or "permission" in error_message + ): + logger.exception( + "preprod.status_checks.create.insufficient_permissions", + extra={ + "organization_id": self.organization_id, + "integration_id": self.integration_id, + "repo": repo, + "error_message": str(e), + }, + ) + raise IntegrationConfigurationError( + "GitHub App lacks permissions to create check runs. " + "Please ensure the app has the required permissions and that " + "the organization has accepted any updated permissions." + ) from e + elif e.code and 400 <= e.code < 500 and e.code != 429: + logger.exception( + "preprod.status_checks.create.client_error", + extra={ + "organization_id": self.organization_id, + "integration_id": self.integration_id, + "repo": repo, + "status_code": e.code, + }, + ) + raise IntegrationConfigurationError( + f"GitHub API returned {e.code} client error when creating check run" + ) from e + + # For non-permission 403s, 429s, 5xx, and other error + raise GITHUB_STATUS_CHECK_STATUS_MAPPING: dict[StatusCheckStatus, GitHubCheckStatus] = { diff --git a/tests/sentry/preprod/vcs/status_checks/size/test_status_checks_tasks.py b/tests/sentry/preprod/vcs/status_checks/size/test_status_checks_tasks.py index 19c6b69f12e038..31ca4416b01f5b 100644 --- a/tests/sentry/preprod/vcs/status_checks/size/test_status_checks_tasks.py +++ b/tests/sentry/preprod/vcs/status_checks/size/test_status_checks_tasks.py @@ -3,11 +3,14 @@ import uuid from unittest.mock import Mock, patch +import responses + from sentry.integrations.source_code_management.status_check import StatusCheckStatus from sentry.models.commitcomparison import CommitComparison from sentry.models.repository import Repository from sentry.preprod.models import PreprodArtifact, PreprodArtifactSizeMetrics from sentry.preprod.vcs.status_checks.size.tasks import create_preprod_status_check_task +from sentry.shared_integrations.exceptions import IntegrationConfigurationError from sentry.testutils.cases import TestCase from sentry.testutils.silo import region_silo_test @@ -445,3 +448,218 @@ def test_create_preprod_status_check_task_mixed_states_monorepo(self): assert "com.example.uploading" in summary assert "com.example.failed" in summary assert "Upload timeout" in summary + + @responses.activate + def test_create_preprod_status_check_task_github_permission_error(self): + """Test task handles GitHub permission errors without retrying.""" + preprod_artifact = self._create_preprod_artifact( + state=PreprodArtifact.ArtifactState.PROCESSED + ) + + PreprodArtifactSizeMetrics.objects.create( + preprod_artifact=preprod_artifact, + metrics_artifact_type=PreprodArtifactSizeMetrics.MetricsArtifactType.MAIN_ARTIFACT, + state=PreprodArtifactSizeMetrics.SizeAnalysisState.COMPLETED, + min_download_size=1024 * 1024, + max_download_size=1024 * 1024, + min_install_size=2 * 1024 * 1024, + max_install_size=2 * 1024 * 1024, + ) + + integration = self.create_integration( + organization=self.organization, + external_id="123", + provider="github", + metadata={"access_token": "test_token", "expires_at": "2099-01-01T00:00:00Z"}, + ) + + Repository.objects.create( + organization_id=self.organization.id, + name="owner/repo", + provider="integrations:github", + integration_id=integration.id, + ) + + responses.add( + responses.POST, + "https://api.github.com/repos/owner/repo/check-runs", + status=403, + json={ + "message": "Resource not accessible by integration", + "documentation_url": "https://docs.github.com/rest/checks/runs#create-a-check-run", + }, + ) + + with self.tasks(): + try: + create_preprod_status_check_task(preprod_artifact.id) + assert False, "Expected IntegrationConfigurationError to be raised" + except IntegrationConfigurationError as e: + assert "GitHub App lacks permissions" in str(e) + assert "required permissions" in str(e) + + # Verify no retries due to ignore policy + assert len(responses.calls) == 1 + assert ( + responses.calls[0].request.url == "https://api.github.com/repos/owner/repo/check-runs" + ) + + @responses.activate + def test_create_preprod_status_check_task_github_non_permission_403(self): + """Test task re-raises non-permission 403 errors (allows retry for transient issues).""" + preprod_artifact = self._create_preprod_artifact( + state=PreprodArtifact.ArtifactState.PROCESSED + ) + + PreprodArtifactSizeMetrics.objects.create( + preprod_artifact=preprod_artifact, + metrics_artifact_type=PreprodArtifactSizeMetrics.MetricsArtifactType.MAIN_ARTIFACT, + state=PreprodArtifactSizeMetrics.SizeAnalysisState.COMPLETED, + min_download_size=1024 * 1024, + max_download_size=1024 * 1024, + min_install_size=2 * 1024 * 1024, + max_install_size=2 * 1024 * 1024, + ) + + integration = self.create_integration( + organization=self.organization, + external_id="456", + provider="github", + metadata={"access_token": "test_token", "expires_at": "2099-01-01T00:00:00Z"}, + ) + + Repository.objects.create( + organization_id=self.organization.id, + name="owner/repo", + provider="integrations:github", + integration_id=integration.id, + ) + + # 403 error that's NOT permission-related (should re-raise to allow retry) + responses.add( + responses.POST, + "https://api.github.com/repos/owner/repo/check-runs", + status=403, + json={ + "message": "Repository is temporarily unavailable", + }, + ) + + with self.tasks(): + # Should re-raise ApiForbiddenError (not convert to IntegrationConfigurationError) + # This allows the task system to retry in case it's a transient issue + try: + create_preprod_status_check_task(preprod_artifact.id) + assert False, "Expected ApiForbiddenError to be raised" + except Exception as e: + assert e.__class__.__name__ == "ApiForbiddenError" + assert "temporarily unavailable" in str(e) + + assert len(responses.calls) == 1 + assert ( + responses.calls[0].request.url == "https://api.github.com/repos/owner/repo/check-runs" + ) + + @responses.activate + def test_create_preprod_status_check_task_github_400_error(self): + """Test task converts 400 errors to IntegrationConfigurationError (no retry).""" + preprod_artifact = self._create_preprod_artifact( + state=PreprodArtifact.ArtifactState.PROCESSED + ) + + PreprodArtifactSizeMetrics.objects.create( + preprod_artifact=preprod_artifact, + metrics_artifact_type=PreprodArtifactSizeMetrics.MetricsArtifactType.MAIN_ARTIFACT, + state=PreprodArtifactSizeMetrics.SizeAnalysisState.COMPLETED, + min_download_size=1024 * 1024, + max_download_size=1024 * 1024, + min_install_size=2 * 1024 * 1024, + max_install_size=2 * 1024 * 1024, + ) + + integration = self.create_integration( + organization=self.organization, + external_id="789", + provider="github", + metadata={"access_token": "test_token", "expires_at": "2099-01-01T00:00:00Z"}, + ) + + Repository.objects.create( + organization_id=self.organization.id, + name="owner/repo", + provider="integrations:github", + integration_id=integration.id, + ) + + responses.add( + responses.POST, + "https://api.github.com/repos/owner/repo/check-runs", + status=400, + json={ + "message": "Invalid request", + "errors": [{"field": "head_sha", "code": "invalid"}], + }, + ) + + with self.tasks(): + try: + create_preprod_status_check_task(preprod_artifact.id) + assert False, "Expected IntegrationConfigurationError to be raised" + except IntegrationConfigurationError as e: + assert "400 client error" in str(e) + + # Verify no retries + assert len(responses.calls) == 1 + + @responses.activate + def test_create_preprod_status_check_task_github_429_error(self): + """Test task allows 429 rate limit errors to retry""" + preprod_artifact = self._create_preprod_artifact( + state=PreprodArtifact.ArtifactState.PROCESSED + ) + + PreprodArtifactSizeMetrics.objects.create( + preprod_artifact=preprod_artifact, + metrics_artifact_type=PreprodArtifactSizeMetrics.MetricsArtifactType.MAIN_ARTIFACT, + state=PreprodArtifactSizeMetrics.SizeAnalysisState.COMPLETED, + min_download_size=1024 * 1024, + max_download_size=1024 * 1024, + min_install_size=2 * 1024 * 1024, + max_install_size=2 * 1024 * 1024, + ) + + integration = self.create_integration( + organization=self.organization, + external_id="999", + provider="github", + metadata={"access_token": "test_token", "expires_at": "2099-01-01T00:00:00Z"}, + ) + + Repository.objects.create( + organization_id=self.organization.id, + name="owner/repo", + provider="integrations:github", + integration_id=integration.id, + ) + + responses.add( + responses.POST, + "https://api.github.com/repos/owner/repo/check-runs", + status=429, + json={ + "message": "API rate limit exceeded", + "documentation_url": "https://docs.github.com/rest/overview/resources-in-the-rest-api#rate-limiting", + }, + ) + + with self.tasks(): + # 429 should be re-raised as ApiRateLimitedError (not converted to IntegrationConfigurationError) + # This allows the task system to retry with backoff + try: + create_preprod_status_check_task(preprod_artifact.id) + assert False, "Expected ApiRateLimitedError to be raised" + except Exception as e: + assert e.__class__.__name__ == "ApiRateLimitedError" + assert "rate limit" in str(e).lower() + + assert len(responses.calls) == 1