diff --git a/src/launchpad/artifact_processor.py b/src/launchpad/artifact_processor.py index bf90179d..a6403e56 100644 --- a/src/launchpad/artifact_processor.py +++ b/src/launchpad/artifact_processor.py @@ -33,6 +33,7 @@ from launchpad.artifacts.artifact_factory import ArtifactFactory from launchpad.constants import ( ArtifactType, + InstallableAppErrorCode, PreprodFeature, ProcessingErrorCode, ProcessingErrorMessage, @@ -330,13 +331,24 @@ def _do_distribution( logger.info(f"BUILD_DISTRIBUTION for {artifact_id} (project: {project_id}, org: {organization_id})") if isinstance(artifact, ZippedXCArchive): apple_info = cast(AppleAppInfo, info) - if apple_info.is_code_signature_valid and not apple_info.is_simulator: - with tempfile.TemporaryDirectory() as temp_dir_str: - temp_dir = Path(temp_dir_str) - ipa_path = temp_dir / "App.ipa" - artifact.generate_ipa(ipa_path) - with open(ipa_path, "rb") as f: - self._sentry_client.upload_installable_app(organization_id, project_id, artifact_id, f) + if not apple_info.is_code_signature_valid: + logger.warning(f"BUILD_DISTRIBUTION skipped for {artifact_id}: invalid code signature") + self._update_distribution_error( + organization_id, artifact_id, InstallableAppErrorCode.SKIPPED, "invalid_signature" + ) + return + if apple_info.is_simulator: + logger.warning(f"BUILD_DISTRIBUTION skipped for {artifact_id}: simulator build") + self._update_distribution_error( + organization_id, artifact_id, InstallableAppErrorCode.SKIPPED, "simulator" + ) + return + with tempfile.TemporaryDirectory() as temp_dir_str: + temp_dir = Path(temp_dir_str) + ipa_path = temp_dir / "App.ipa" + artifact.generate_ipa(ipa_path) + with open(ipa_path, "rb") as f: + self._sentry_client.upload_installable_app(organization_id, project_id, artifact_id, f) elif isinstance(artifact, (AAB, ZippedAAB)): with tempfile.TemporaryDirectory() as temp_dir_str: temp_dir = Path(temp_dir_str) @@ -354,9 +366,10 @@ def _do_distribution( with apk.raw_file() as f: self._sentry_client.upload_installable_app(organization_id, project_id, artifact_id, f) else: - # TODO(EME-422): Should call _update_artifact_error here once we - # support setting errors just for build. - logger.error(f"BUILD_DISTRIBUTION failed for {artifact_id} (project: {project_id}, org: {organization_id})") + logger.error(f"BUILD_DISTRIBUTION failed for {artifact_id}: unsupported artifact type") + self._update_distribution_error( + organization_id, artifact_id, InstallableAppErrorCode.PROCESSING_ERROR, "unsupported artifact type" + ) def _do_size( self, @@ -438,6 +451,39 @@ def _update_artifact_error( else: logger.info(f"Successfully updated artifact {artifact_id} with error information") + def _update_distribution_error( + self, + organization_id: str, + artifact_id: str, + error_code: InstallableAppErrorCode, + error_message: str, + ) -> None: + """Update distribution with error/skip information.""" + logger.info(f"Updating distribution for {artifact_id} with error code {error_code.value}") + + self._statsd.increment( + "distribution.processing.error", + tags=[ + f"error_code:{error_code.value}", + f"error_message:{error_message}", + f"organization_id:{organization_id}", + ], + ) + + try: + self._sentry_client.update_distribution( + org=organization_id, + artifact_id=artifact_id, + data={ + "error_code": error_code.value, + "error_message": error_message, + }, + ) + except SentryClientError: + logger.exception(f"Failed to update distribution error for artifact {artifact_id}") + else: + logger.info(f"Successfully updated distribution for {artifact_id} with error information") + def _update_size_error_from_exception( self, organization_id: str, diff --git a/src/launchpad/constants.py b/src/launchpad/constants.py index ea079ef8..7b53d05a 100644 --- a/src/launchpad/constants.py +++ b/src/launchpad/constants.py @@ -29,6 +29,14 @@ class PreprodFeature(Enum): BUILD_DISTRIBUTION = "build_distribution" +# Matches InstallableApp.ErrorCode in sentry +class InstallableAppErrorCode(Enum): + UNKNOWN = 0 + NO_QUOTA = 1 + SKIPPED = 2 + PROCESSING_ERROR = 3 + + # Health check threshold - consider unhealthy if file not touched in 60 seconds HEALTHCHECK_MAX_AGE_SECONDS = 60.0 diff --git a/src/launchpad/sentry_client.py b/src/launchpad/sentry_client.py index e2581ecc..d7304edd 100644 --- a/src/launchpad/sentry_client.py +++ b/src/launchpad/sentry_client.py @@ -134,6 +134,11 @@ class RetentionResponse(BaseModel): build_distribution: int +class DistributionResponse(BaseModel): + model_config = ConfigDict(strict=True, alias_generator=to_camel) + artifact_id: str + + def create_retry_session(max_retries: int = 3) -> requests.Session: """Create a requests session with retry configuration.""" session = requests.Session() @@ -252,6 +257,11 @@ def update_artifact(self, org: str, project: str, artifact_id: str, data: Dict[s endpoint = f"/api/0/internal/{org}/{project}/files/preprodartifacts/{artifact_id}/update/" return self._make_json_request("PUT", endpoint, UpdateResponse, data=data) + def update_distribution(self, org: str, artifact_id: str, data: Dict[str, Any]) -> DistributionResponse: + """Update preprod artifact distribution.""" + endpoint = f"/api/0/organizations/{org}/preprodartifacts/{artifact_id}/distribution/" + return self._make_json_request("PUT", endpoint, DistributionResponse, data=data) + def upload_size_analysis_file( self, org: str, diff --git a/tests/unit/artifacts/test_artifact_processor.py b/tests/unit/artifacts/test_artifact_processor.py index 01611b1f..376717b8 100644 --- a/tests/unit/artifacts/test_artifact_processor.py +++ b/tests/unit/artifacts/test_artifact_processor.py @@ -3,7 +3,10 @@ from objectstore_client import Client as ObjectstoreClient from launchpad.artifact_processor import ArtifactProcessor, ServiceConfig +from launchpad.artifacts.apple.zipped_xcarchive import ZippedXCArchive +from launchpad.artifacts.artifact import Artifact from launchpad.constants import ( + InstallableAppErrorCode, ProcessingErrorCode, ProcessingErrorMessage, ) @@ -134,6 +137,101 @@ def test_processing_error_message_enum_values(self): assert ProcessingErrorMessage.SIZE_ANALYSIS_FAILED.value == "Failed to perform size analysis" assert ProcessingErrorMessage.UNKNOWN_ERROR.value == "An unknown error occurred" + def test_do_distribution_unknown_artifact_type_reports_error(self): + mock_sentry_client = Mock(spec=SentryClient) + mock_sentry_client.update_distribution.return_value = None + mock_statsd = Mock() + self.processor._sentry_client = mock_sentry_client + self.processor._statsd = mock_statsd + + unknown_artifact = Mock(spec=Artifact) + mock_info = Mock() + + self.processor._do_distribution( + "test-org-id", "test-project-id", "test-artifact-id", unknown_artifact, mock_info + ) + + mock_sentry_client.update_distribution.assert_called_once_with( + org="test-org-id", + artifact_id="test-artifact-id", + data={ + "error_code": InstallableAppErrorCode.PROCESSING_ERROR.value, + "error_message": "unsupported artifact type", + }, + ) + mock_statsd.increment.assert_called_once_with( + "distribution.processing.error", + tags=[ + f"error_code:{InstallableAppErrorCode.PROCESSING_ERROR.value}", + "error_message:unsupported artifact type", + "organization_id:test-org-id", + ], + ) + + def test_do_distribution_invalid_code_signature_reports_skip(self): + mock_sentry_client = Mock(spec=SentryClient) + mock_sentry_client.update_distribution.return_value = None + mock_statsd = Mock() + self.processor._sentry_client = mock_sentry_client + self.processor._statsd = mock_statsd + + artifact = Mock(spec=ZippedXCArchive) + mock_info = Mock() + mock_info.is_code_signature_valid = False + mock_info.is_simulator = False + + self.processor._do_distribution("test-org-id", "test-project-id", "test-artifact-id", artifact, mock_info) + + mock_sentry_client.update_distribution.assert_called_once_with( + org="test-org-id", + artifact_id="test-artifact-id", + data={ + "error_code": InstallableAppErrorCode.SKIPPED.value, + "error_message": "invalid_signature", + }, + ) + mock_statsd.increment.assert_called_once_with( + "distribution.processing.error", + tags=[ + f"error_code:{InstallableAppErrorCode.SKIPPED.value}", + "error_message:invalid_signature", + "organization_id:test-org-id", + ], + ) + mock_sentry_client.upload_installable_app.assert_not_called() + + def test_do_distribution_simulator_build_reports_skip(self): + mock_sentry_client = Mock(spec=SentryClient) + mock_sentry_client.update_distribution.return_value = None + mock_statsd = Mock() + self.processor._sentry_client = mock_sentry_client + self.processor._statsd = mock_statsd + + artifact = Mock(spec=ZippedXCArchive) + mock_info = Mock() + mock_info.is_code_signature_valid = True + mock_info.is_simulator = True + + self.processor._do_distribution("test-org-id", "test-project-id", "test-artifact-id", artifact, mock_info) + + mock_sentry_client.update_distribution.assert_called_once_with( + org="test-org-id", + artifact_id="test-artifact-id", + data={ + "error_code": InstallableAppErrorCode.SKIPPED.value, + "error_message": "simulator", + }, + ) + mock_statsd.increment.assert_called_once_with( + "distribution.processing.error", + tags=[ + f"error_code:{InstallableAppErrorCode.SKIPPED.value}", + "error_message:simulator", + "organization_id:test-org-id", + ], + ) + mock_sentry_client.upload_installable_app.assert_not_called() + class TestArtifactProcessorMessageHandling: """Test message processing functionality in ArtifactProcessor."""