Skip to content
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
45 changes: 42 additions & 3 deletions src/sentry/preprod/vcs/status_checks/size/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand Down Expand Up @@ -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] = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Loading