diff --git a/src/sentry/preprod/vcs/status_checks/size/tasks.py b/src/sentry/preprod/vcs/status_checks/size/tasks.py index 3d62fab9112733..955ec97845d0b6 100644 --- a/src/sentry/preprod/vcs/status_checks/size/tasks.py +++ b/src/sentry/preprod/vcs/status_checks/size/tasks.py @@ -3,8 +3,11 @@ import logging from abc import ABC, abstractmethod from datetime import datetime +from enum import StrEnum from typing import Any +from django.db import router, transaction + from sentry.constants import ObjectStatus from sentry.integrations.base import IntegrationInstallation from sentry.integrations.github.status_check import GitHubCheckConclusion, GitHubCheckStatus @@ -129,19 +132,24 @@ def create_preprod_status_check_task(preprod_artifact_id: int) -> None: if GITHUB_STATUS_CHECK_STATUS_MAPPING[status] == GitHubCheckStatus.COMPLETED: completed_at = preprod_artifact.date_updated - check_id = provider.create_status_check( - repo=commit_comparison.head_repo_name, - sha=commit_comparison.head_sha, - status=status, - title=title, - subtitle=subtitle, - text=None, # TODO(telkins): add text field support - summary=summary, - external_id=str(preprod_artifact.id), - target_url=target_url, - started_at=preprod_artifact.date_added, - completed_at=completed_at, - ) + try: + check_id = provider.create_status_check( + repo=commit_comparison.head_repo_name, + sha=commit_comparison.head_sha, + status=status, + title=title, + subtitle=subtitle, + text=None, # TODO(telkins): add text field support + summary=summary, + external_id=str(preprod_artifact.id), + target_url=target_url, + started_at=preprod_artifact.date_added, + completed_at=completed_at, + ) + except Exception as e: + _update_posted_status_check(preprod_artifact, check_type="size", success=False, error=e) + raise + if check_id is None: logger.error( "preprod.status_checks.create.failed", @@ -151,8 +159,13 @@ def create_preprod_status_check_task(preprod_artifact_id: int) -> None: "organization_slug": preprod_artifact.project.organization.slug, }, ) + _update_posted_status_check(preprod_artifact, check_type="size", success=False) return + _update_posted_status_check( + preprod_artifact, check_type="size", success=True, check_id=check_id + ) + logger.info( "preprod.status_checks.create.success", extra={ @@ -165,6 +178,54 @@ def create_preprod_status_check_task(preprod_artifact_id: int) -> None: ) +def _update_posted_status_check( + preprod_artifact: PreprodArtifact, + check_type: str, + success: bool, + check_id: str | None = None, + error: Exception | None = None, +) -> None: + """Update the posted_status_checks field in the artifact's extras.""" + with transaction.atomic(router.db_for_write(PreprodArtifact)): + artifact = PreprodArtifact.objects.select_for_update().get(id=preprod_artifact.id) + extras = artifact.extras or {} + + posted_status_checks = extras.get("posted_status_checks", {}) + + check_result: dict[str, Any] = {"success": success} + if success and check_id: + check_result["check_id"] = check_id + if not success: + check_result["error_type"] = _get_error_type(error).value + + posted_status_checks[check_type] = check_result + extras["posted_status_checks"] = posted_status_checks + artifact.extras = extras + artifact.save(update_fields=["extras"]) + + +def _get_error_type(error: Exception | None) -> StatusCheckErrorType: + """Determine the error type from an exception.""" + if error is None: + return StatusCheckErrorType.UNKNOWN + if isinstance(error, IntegrationConfigurationError): + return StatusCheckErrorType.INTEGRATION_ERROR + if isinstance(error, ApiError): + return StatusCheckErrorType.API_ERROR + return StatusCheckErrorType.UNKNOWN + + +class StatusCheckErrorType(StrEnum): + """Error types for status check creation failures.""" + + UNKNOWN = "unknown" + """An unknown error occurred (e.g., API returned null check_id).""" + API_ERROR = "api_error" + """A retryable API error (5xx, rate limit, transient issues).""" + INTEGRATION_ERROR = "integration_error" + """An integration configuration error (permissions, invalid request, etc.).""" + + def _compute_overall_status( artifacts: list[PreprodArtifact], size_metrics_map: dict[int, list[PreprodArtifactSizeMetrics]] ) -> StatusCheckStatus: 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 d21673a18f1af4..53e22548a79105 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 @@ -11,7 +11,10 @@ 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.preprod.vcs.status_checks.size.tasks import ( + StatusCheckErrorType, + 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 @@ -988,3 +991,165 @@ def test_sibling_deduplication_with_same_app_id_different_platforms(self): assert ( ios_artifact.id not in sibling_ids_from_ios_new ), "Old iOS artifact should be deduplicated (not the triggering artifact)" + + def test_posted_status_check_success(self): + """Test that successful status check posts are recorded in artifact extras.""" + 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, + ) + + _, mock_provider, client_patch, provider_patch = self._create_working_status_check_setup( + preprod_artifact + ) + mock_provider.create_status_check.return_value = "check_12345" + + with client_patch, provider_patch: + with self.tasks(): + create_preprod_status_check_task(preprod_artifact.id) + + preprod_artifact.refresh_from_db() + assert preprod_artifact.extras is not None + assert "posted_status_checks" in preprod_artifact.extras + assert "size" in preprod_artifact.extras["posted_status_checks"] + assert preprod_artifact.extras["posted_status_checks"]["size"]["success"] is True + assert preprod_artifact.extras["posted_status_checks"]["size"]["check_id"] == "check_12345" + + def test_posted_status_check_failure_null_check_id(self): + """Test that failed status check posts (null check_id) are recorded in artifact extras.""" + 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, + ) + + _, mock_provider, client_patch, provider_patch = self._create_working_status_check_setup( + preprod_artifact + ) + mock_provider.create_status_check.return_value = None # Simulate API returning no check_id + + with client_patch, provider_patch: + with self.tasks(): + create_preprod_status_check_task(preprod_artifact.id) + + preprod_artifact.refresh_from_db() + assert preprod_artifact.extras is not None + assert "posted_status_checks" in preprod_artifact.extras + assert "size" in preprod_artifact.extras["posted_status_checks"] + assert preprod_artifact.extras["posted_status_checks"]["size"]["success"] is False + assert ( + preprod_artifact.extras["posted_status_checks"]["size"]["error_type"] + == StatusCheckErrorType.UNKNOWN.value + ) + + @responses.activate + def test_posted_status_check_failure_integration_error(self): + """Test that integration errors during status check creation are recorded in artifact extras.""" + 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="test-integration-error", + 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) + except IntegrationConfigurationError: + pass # Expected + + preprod_artifact.refresh_from_db() + assert preprod_artifact.extras is not None + assert "posted_status_checks" in preprod_artifact.extras + assert "size" in preprod_artifact.extras["posted_status_checks"] + assert preprod_artifact.extras["posted_status_checks"]["size"]["success"] is False + assert ( + preprod_artifact.extras["posted_status_checks"]["size"]["error_type"] + == StatusCheckErrorType.INTEGRATION_ERROR.value + ) + + def test_posted_status_check_preserves_existing_extras(self): + """Test that recording status check result preserves other fields in extras.""" + preprod_artifact = self._create_preprod_artifact( + state=PreprodArtifact.ArtifactState.PROCESSED + ) + preprod_artifact.extras = {"existing_field": "existing_value", "another_field": 123} + preprod_artifact.save() + + 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, + ) + + _, mock_provider, client_patch, provider_patch = self._create_working_status_check_setup( + preprod_artifact + ) + mock_provider.create_status_check.return_value = "check_67890" + + with client_patch, provider_patch: + with self.tasks(): + create_preprod_status_check_task(preprod_artifact.id) + + preprod_artifact.refresh_from_db() + assert preprod_artifact.extras is not None + # Verify existing fields are preserved + assert preprod_artifact.extras["existing_field"] == "existing_value" + assert preprod_artifact.extras["another_field"] == 123 + # Verify new field is added + assert "posted_status_checks" in preprod_artifact.extras + assert "size" in preprod_artifact.extras["posted_status_checks"] + assert preprod_artifact.extras["posted_status_checks"]["size"]["success"] is True + assert preprod_artifact.extras["posted_status_checks"]["size"]["check_id"] == "check_67890"