Skip to content

Commit 3d7a9cd

Browse files
authored
fix(preprod): handle integration permission errors (#102981)
This is a common issue for orgs when they first integrate and we want extra visibility around when it happens. I see other parts of our integrations codebase treats this as a `IntegrationConfigurationError`, so doing the same here. I also noticed that these were being retried up to our limit which doesn't make sense since it is not a recoverable error. I added `IntegrationConfigurationError` to our error ignore list. Resolves EME-611
1 parent 0d9a78b commit 3d7a9cd

File tree

2 files changed

+260
-3
lines changed

2 files changed

+260
-3
lines changed

src/sentry/preprod/vcs/status_checks/size/tasks.py

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
from sentry.preprod.models import PreprodArtifact, PreprodArtifactSizeMetrics
2525
from sentry.preprod.url_utils import get_preprod_artifact_url
2626
from sentry.preprod.vcs.status_checks.size.templates import format_status_check_messages
27+
from sentry.shared_integrations.exceptions import ApiError, IntegrationConfigurationError
2728
from sentry.silo.base import SiloMode
2829
from sentry.tasks.base import instrumented_task
2930
from sentry.taskworker.namespaces import integrations_tasks
@@ -36,7 +37,7 @@
3637
name="sentry.preprod.tasks.create_preprod_status_check",
3738
namespace=integrations_tasks,
3839
processing_deadline_duration=30,
39-
retry=Retry(times=3),
40+
retry=Retry(times=3, ignore=(IntegrationConfigurationError,)),
4041
silo_mode=SiloMode.REGION,
4142
)
4243
def create_preprod_status_check_task(preprod_artifact_id: int) -> None:
@@ -351,9 +352,47 @@ def create_status_check(
351352
response = self.client.create_check_run(repo=repo, data=check_data)
352353
check_id = response.get("id")
353354
return str(check_id) if check_id else None
354-
except Exception as e:
355+
except ApiError as e:
355356
lifecycle.record_failure(e)
356-
return None
357+
# Only convert specific permission 403s as IntegrationConfigurationError
358+
# GitHub can return 403 for various reasons (rate limits, temporary issues, permissions)
359+
if e.code == 403:
360+
error_message = str(e).lower()
361+
if (
362+
"resource not accessible" in error_message
363+
or "insufficient" in error_message
364+
or "permission" in error_message
365+
):
366+
logger.exception(
367+
"preprod.status_checks.create.insufficient_permissions",
368+
extra={
369+
"organization_id": self.organization_id,
370+
"integration_id": self.integration_id,
371+
"repo": repo,
372+
"error_message": str(e),
373+
},
374+
)
375+
raise IntegrationConfigurationError(
376+
"GitHub App lacks permissions to create check runs. "
377+
"Please ensure the app has the required permissions and that "
378+
"the organization has accepted any updated permissions."
379+
) from e
380+
elif e.code and 400 <= e.code < 500 and e.code != 429:
381+
logger.exception(
382+
"preprod.status_checks.create.client_error",
383+
extra={
384+
"organization_id": self.organization_id,
385+
"integration_id": self.integration_id,
386+
"repo": repo,
387+
"status_code": e.code,
388+
},
389+
)
390+
raise IntegrationConfigurationError(
391+
f"GitHub API returned {e.code} client error when creating check run"
392+
) from e
393+
394+
# For non-permission 403s, 429s, 5xx, and other error
395+
raise
357396

358397

359398
GITHUB_STATUS_CHECK_STATUS_MAPPING: dict[StatusCheckStatus, GitHubCheckStatus] = {

tests/sentry/preprod/vcs/status_checks/size/test_status_checks_tasks.py

Lines changed: 218 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,14 @@
33
import uuid
44
from unittest.mock import Mock, patch
55

6+
import responses
7+
68
from sentry.integrations.source_code_management.status_check import StatusCheckStatus
79
from sentry.models.commitcomparison import CommitComparison
810
from sentry.models.repository import Repository
911
from sentry.preprod.models import PreprodArtifact, PreprodArtifactSizeMetrics
1012
from sentry.preprod.vcs.status_checks.size.tasks import create_preprod_status_check_task
13+
from sentry.shared_integrations.exceptions import IntegrationConfigurationError
1114
from sentry.testutils.cases import TestCase
1215
from sentry.testutils.silo import region_silo_test
1316

@@ -445,3 +448,218 @@ def test_create_preprod_status_check_task_mixed_states_monorepo(self):
445448
assert "com.example.uploading" in summary
446449
assert "com.example.failed" in summary
447450
assert "Upload timeout" in summary
451+
452+
@responses.activate
453+
def test_create_preprod_status_check_task_github_permission_error(self):
454+
"""Test task handles GitHub permission errors without retrying."""
455+
preprod_artifact = self._create_preprod_artifact(
456+
state=PreprodArtifact.ArtifactState.PROCESSED
457+
)
458+
459+
PreprodArtifactSizeMetrics.objects.create(
460+
preprod_artifact=preprod_artifact,
461+
metrics_artifact_type=PreprodArtifactSizeMetrics.MetricsArtifactType.MAIN_ARTIFACT,
462+
state=PreprodArtifactSizeMetrics.SizeAnalysisState.COMPLETED,
463+
min_download_size=1024 * 1024,
464+
max_download_size=1024 * 1024,
465+
min_install_size=2 * 1024 * 1024,
466+
max_install_size=2 * 1024 * 1024,
467+
)
468+
469+
integration = self.create_integration(
470+
organization=self.organization,
471+
external_id="123",
472+
provider="github",
473+
metadata={"access_token": "test_token", "expires_at": "2099-01-01T00:00:00Z"},
474+
)
475+
476+
Repository.objects.create(
477+
organization_id=self.organization.id,
478+
name="owner/repo",
479+
provider="integrations:github",
480+
integration_id=integration.id,
481+
)
482+
483+
responses.add(
484+
responses.POST,
485+
"https://api.github.com/repos/owner/repo/check-runs",
486+
status=403,
487+
json={
488+
"message": "Resource not accessible by integration",
489+
"documentation_url": "https://docs.github.com/rest/checks/runs#create-a-check-run",
490+
},
491+
)
492+
493+
with self.tasks():
494+
try:
495+
create_preprod_status_check_task(preprod_artifact.id)
496+
assert False, "Expected IntegrationConfigurationError to be raised"
497+
except IntegrationConfigurationError as e:
498+
assert "GitHub App lacks permissions" in str(e)
499+
assert "required permissions" in str(e)
500+
501+
# Verify no retries due to ignore policy
502+
assert len(responses.calls) == 1
503+
assert (
504+
responses.calls[0].request.url == "https://api.github.com/repos/owner/repo/check-runs"
505+
)
506+
507+
@responses.activate
508+
def test_create_preprod_status_check_task_github_non_permission_403(self):
509+
"""Test task re-raises non-permission 403 errors (allows retry for transient issues)."""
510+
preprod_artifact = self._create_preprod_artifact(
511+
state=PreprodArtifact.ArtifactState.PROCESSED
512+
)
513+
514+
PreprodArtifactSizeMetrics.objects.create(
515+
preprod_artifact=preprod_artifact,
516+
metrics_artifact_type=PreprodArtifactSizeMetrics.MetricsArtifactType.MAIN_ARTIFACT,
517+
state=PreprodArtifactSizeMetrics.SizeAnalysisState.COMPLETED,
518+
min_download_size=1024 * 1024,
519+
max_download_size=1024 * 1024,
520+
min_install_size=2 * 1024 * 1024,
521+
max_install_size=2 * 1024 * 1024,
522+
)
523+
524+
integration = self.create_integration(
525+
organization=self.organization,
526+
external_id="456",
527+
provider="github",
528+
metadata={"access_token": "test_token", "expires_at": "2099-01-01T00:00:00Z"},
529+
)
530+
531+
Repository.objects.create(
532+
organization_id=self.organization.id,
533+
name="owner/repo",
534+
provider="integrations:github",
535+
integration_id=integration.id,
536+
)
537+
538+
# 403 error that's NOT permission-related (should re-raise to allow retry)
539+
responses.add(
540+
responses.POST,
541+
"https://api.github.com/repos/owner/repo/check-runs",
542+
status=403,
543+
json={
544+
"message": "Repository is temporarily unavailable",
545+
},
546+
)
547+
548+
with self.tasks():
549+
# Should re-raise ApiForbiddenError (not convert to IntegrationConfigurationError)
550+
# This allows the task system to retry in case it's a transient issue
551+
try:
552+
create_preprod_status_check_task(preprod_artifact.id)
553+
assert False, "Expected ApiForbiddenError to be raised"
554+
except Exception as e:
555+
assert e.__class__.__name__ == "ApiForbiddenError"
556+
assert "temporarily unavailable" in str(e)
557+
558+
assert len(responses.calls) == 1
559+
assert (
560+
responses.calls[0].request.url == "https://api.github.com/repos/owner/repo/check-runs"
561+
)
562+
563+
@responses.activate
564+
def test_create_preprod_status_check_task_github_400_error(self):
565+
"""Test task converts 400 errors to IntegrationConfigurationError (no retry)."""
566+
preprod_artifact = self._create_preprod_artifact(
567+
state=PreprodArtifact.ArtifactState.PROCESSED
568+
)
569+
570+
PreprodArtifactSizeMetrics.objects.create(
571+
preprod_artifact=preprod_artifact,
572+
metrics_artifact_type=PreprodArtifactSizeMetrics.MetricsArtifactType.MAIN_ARTIFACT,
573+
state=PreprodArtifactSizeMetrics.SizeAnalysisState.COMPLETED,
574+
min_download_size=1024 * 1024,
575+
max_download_size=1024 * 1024,
576+
min_install_size=2 * 1024 * 1024,
577+
max_install_size=2 * 1024 * 1024,
578+
)
579+
580+
integration = self.create_integration(
581+
organization=self.organization,
582+
external_id="789",
583+
provider="github",
584+
metadata={"access_token": "test_token", "expires_at": "2099-01-01T00:00:00Z"},
585+
)
586+
587+
Repository.objects.create(
588+
organization_id=self.organization.id,
589+
name="owner/repo",
590+
provider="integrations:github",
591+
integration_id=integration.id,
592+
)
593+
594+
responses.add(
595+
responses.POST,
596+
"https://api.github.com/repos/owner/repo/check-runs",
597+
status=400,
598+
json={
599+
"message": "Invalid request",
600+
"errors": [{"field": "head_sha", "code": "invalid"}],
601+
},
602+
)
603+
604+
with self.tasks():
605+
try:
606+
create_preprod_status_check_task(preprod_artifact.id)
607+
assert False, "Expected IntegrationConfigurationError to be raised"
608+
except IntegrationConfigurationError as e:
609+
assert "400 client error" in str(e)
610+
611+
# Verify no retries
612+
assert len(responses.calls) == 1
613+
614+
@responses.activate
615+
def test_create_preprod_status_check_task_github_429_error(self):
616+
"""Test task allows 429 rate limit errors to retry"""
617+
preprod_artifact = self._create_preprod_artifact(
618+
state=PreprodArtifact.ArtifactState.PROCESSED
619+
)
620+
621+
PreprodArtifactSizeMetrics.objects.create(
622+
preprod_artifact=preprod_artifact,
623+
metrics_artifact_type=PreprodArtifactSizeMetrics.MetricsArtifactType.MAIN_ARTIFACT,
624+
state=PreprodArtifactSizeMetrics.SizeAnalysisState.COMPLETED,
625+
min_download_size=1024 * 1024,
626+
max_download_size=1024 * 1024,
627+
min_install_size=2 * 1024 * 1024,
628+
max_install_size=2 * 1024 * 1024,
629+
)
630+
631+
integration = self.create_integration(
632+
organization=self.organization,
633+
external_id="999",
634+
provider="github",
635+
metadata={"access_token": "test_token", "expires_at": "2099-01-01T00:00:00Z"},
636+
)
637+
638+
Repository.objects.create(
639+
organization_id=self.organization.id,
640+
name="owner/repo",
641+
provider="integrations:github",
642+
integration_id=integration.id,
643+
)
644+
645+
responses.add(
646+
responses.POST,
647+
"https://api.github.com/repos/owner/repo/check-runs",
648+
status=429,
649+
json={
650+
"message": "API rate limit exceeded",
651+
"documentation_url": "https://docs.github.com/rest/overview/resources-in-the-rest-api#rate-limiting",
652+
},
653+
)
654+
655+
with self.tasks():
656+
# 429 should be re-raised as ApiRateLimitedError (not converted to IntegrationConfigurationError)
657+
# This allows the task system to retry with backoff
658+
try:
659+
create_preprod_status_check_task(preprod_artifact.id)
660+
assert False, "Expected ApiRateLimitedError to be raised"
661+
except Exception as e:
662+
assert e.__class__.__name__ == "ApiRateLimitedError"
663+
assert "rate limit" in str(e).lower()
664+
665+
assert len(responses.calls) == 1

0 commit comments

Comments
 (0)