From 0754d48da86f1079b5eb659c332a680d8902324f Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Mon, 23 Feb 2026 10:33:21 +0100 Subject: [PATCH 01/11] fix(artifacts): Report error for unknown artifact types in distribution (EME-422) Replace the TODO in _do_distribution() with a call to _update_artifact_error() so that unsupported artifact types are properly reported back to Sentry instead of silently failing. --- src/launchpad/artifact_processor.py | 9 +++++-- .../unit/artifacts/test_artifact_processor.py | 24 +++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/launchpad/artifact_processor.py b/src/launchpad/artifact_processor.py index bf90179d..50d1fe7f 100644 --- a/src/launchpad/artifact_processor.py +++ b/src/launchpad/artifact_processor.py @@ -354,9 +354,14 @@ 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})") + self._update_artifact_error( + organization_id, + project_id, + artifact_id, + ProcessingErrorCode.ARTIFACT_PROCESSING_ERROR, + ProcessingErrorMessage.UNSUPPORTED_ARTIFACT_TYPE, + ) def _do_size( self, diff --git a/tests/unit/artifacts/test_artifact_processor.py b/tests/unit/artifacts/test_artifact_processor.py index 01611b1f..fd8a3998 100644 --- a/tests/unit/artifacts/test_artifact_processor.py +++ b/tests/unit/artifacts/test_artifact_processor.py @@ -3,6 +3,7 @@ from objectstore_client import Client as ObjectstoreClient from launchpad.artifact_processor import ArtifactProcessor, ServiceConfig +from launchpad.artifacts.artifact import Artifact from launchpad.constants import ( ProcessingErrorCode, ProcessingErrorMessage, @@ -134,6 +135,29 @@ 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): + """Test that _do_distribution reports an error for unknown artifact types.""" + mock_sentry_client = Mock(spec=SentryClient) + mock_sentry_client.update_artifact.return_value = None + self.processor._sentry_client = mock_sentry_client + + 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_artifact.assert_called_once_with( + org="test-org-id", + project="test-project-id", + artifact_id="test-artifact-id", + data={ + "error_code": ProcessingErrorCode.ARTIFACT_PROCESSING_ERROR.value, + "error_message": ProcessingErrorMessage.UNSUPPORTED_ARTIFACT_TYPE.value, + }, + ) + class TestArtifactProcessorMessageHandling: """Test message processing functionality in ArtifactProcessor.""" From b6ae5426cd21f9690fd3c9beeb8840618df079a0 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Mon, 23 Feb 2026 11:25:06 +0100 Subject: [PATCH 02/11] fix(artifacts): Report error for iOS distribution failures (EME-422) When a ZippedXCArchive has an invalid code signature or is a simulator build, _do_distribution now reports the specific error via _update_artifact_error instead of silently doing nothing. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/launchpad/artifact_processor.py | 33 ++++++++++--- src/launchpad/constants.py | 2 + .../unit/artifacts/test_artifact_processor.py | 47 +++++++++++++++++++ 3 files changed, 75 insertions(+), 7 deletions(-) diff --git a/src/launchpad/artifact_processor.py b/src/launchpad/artifact_processor.py index 50d1fe7f..71635965 100644 --- a/src/launchpad/artifact_processor.py +++ b/src/launchpad/artifact_processor.py @@ -330,13 +330,32 @@ 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_artifact_error( + organization_id, + project_id, + artifact_id, + ProcessingErrorCode.ARTIFACT_PROCESSING_ERROR, + ProcessingErrorMessage.INVALID_CODE_SIGNATURE, + ) + return + if apple_info.is_simulator: + logger.warning(f"BUILD_DISTRIBUTION skipped for {artifact_id}: simulator build") + self._update_artifact_error( + organization_id, + project_id, + artifact_id, + ProcessingErrorCode.ARTIFACT_PROCESSING_ERROR, + ProcessingErrorMessage.SIMULATOR_BUILD, + ) + 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) diff --git a/src/launchpad/constants.py b/src/launchpad/constants.py index ea079ef8..cfece454 100644 --- a/src/launchpad/constants.py +++ b/src/launchpad/constants.py @@ -46,6 +46,8 @@ class ProcessingErrorMessage(Enum): SIZE_ANALYSIS_FAILED = "Failed to perform size analysis" ARTIFACT_PARSING_FAILED = "Failed to parse artifact file" UNSUPPORTED_ARTIFACT_TYPE = "Unsupported artifact type" + INVALID_CODE_SIGNATURE = "Cannot distribute app with invalid code signature" + SIMULATOR_BUILD = "Cannot distribute simulator builds" # System-related errors TEMP_FILE_CREATION_FAILED = "Failed to create temporary file" diff --git a/tests/unit/artifacts/test_artifact_processor.py b/tests/unit/artifacts/test_artifact_processor.py index fd8a3998..38c98f23 100644 --- a/tests/unit/artifacts/test_artifact_processor.py +++ b/tests/unit/artifacts/test_artifact_processor.py @@ -3,6 +3,7 @@ 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 ( ProcessingErrorCode, @@ -158,6 +159,52 @@ def test_do_distribution_unknown_artifact_type_reports_error(self): }, ) + def test_do_distribution_invalid_code_signature_reports_error(self): + mock_sentry_client = Mock(spec=SentryClient) + mock_sentry_client.update_artifact.return_value = None + self.processor._sentry_client = mock_sentry_client + + 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_artifact.assert_called_once_with( + org="test-org-id", + project="test-project-id", + artifact_id="test-artifact-id", + data={ + "error_code": ProcessingErrorCode.ARTIFACT_PROCESSING_ERROR.value, + "error_message": ProcessingErrorMessage.INVALID_CODE_SIGNATURE.value, + }, + ) + mock_sentry_client.upload_installable_app.assert_not_called() + + def test_do_distribution_simulator_build_reports_error(self): + mock_sentry_client = Mock(spec=SentryClient) + mock_sentry_client.update_artifact.return_value = None + self.processor._sentry_client = mock_sentry_client + + 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_artifact.assert_called_once_with( + org="test-org-id", + project="test-project-id", + artifact_id="test-artifact-id", + data={ + "error_code": ProcessingErrorCode.ARTIFACT_PROCESSING_ERROR.value, + "error_message": ProcessingErrorMessage.SIMULATOR_BUILD.value, + }, + ) + mock_sentry_client.upload_installable_app.assert_not_called() + class TestArtifactProcessorMessageHandling: """Test message processing functionality in ArtifactProcessor.""" From 815e7d41511cc22c85392d2f31a6317deade6208 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Mon, 2 Mar 2026 13:25:46 +0100 Subject: [PATCH 03/11] wip --- src/launchpad/artifact_processor.py | 48 ++++++++++--------- src/launchpad/constants.py | 9 +++- .../unit/artifacts/test_artifact_processor.py | 20 ++++---- 3 files changed, 43 insertions(+), 34 deletions(-) diff --git a/src/launchpad/artifact_processor.py b/src/launchpad/artifact_processor.py index 71635965..3e33e56c 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, + DistributionState, PreprodFeature, ProcessingErrorCode, ProcessingErrorMessage, @@ -332,23 +333,11 @@ def _do_distribution( apple_info = cast(AppleAppInfo, info) if not apple_info.is_code_signature_valid: logger.warning(f"BUILD_DISTRIBUTION skipped for {artifact_id}: invalid code signature") - self._update_artifact_error( - organization_id, - project_id, - artifact_id, - ProcessingErrorCode.ARTIFACT_PROCESSING_ERROR, - ProcessingErrorMessage.INVALID_CODE_SIGNATURE, - ) + self._update_distribution_skip(organization_id, project_id, artifact_id, "invalid_signature") return if apple_info.is_simulator: logger.warning(f"BUILD_DISTRIBUTION skipped for {artifact_id}: simulator build") - self._update_artifact_error( - organization_id, - project_id, - artifact_id, - ProcessingErrorCode.ARTIFACT_PROCESSING_ERROR, - ProcessingErrorMessage.SIMULATOR_BUILD, - ) + self._update_distribution_skip(organization_id, project_id, artifact_id, "simulator") return with tempfile.TemporaryDirectory() as temp_dir_str: temp_dir = Path(temp_dir_str) @@ -373,14 +362,8 @@ def _do_distribution( with apk.raw_file() as f: self._sentry_client.upload_installable_app(organization_id, project_id, artifact_id, f) else: - logger.error(f"BUILD_DISTRIBUTION failed for {artifact_id} (project: {project_id}, org: {organization_id})") - self._update_artifact_error( - organization_id, - project_id, - artifact_id, - ProcessingErrorCode.ARTIFACT_PROCESSING_ERROR, - ProcessingErrorMessage.UNSUPPORTED_ARTIFACT_TYPE, - ) + logger.error(f"BUILD_DISTRIBUTION failed for {artifact_id}: unsupported artifact type") + self._update_distribution_skip(organization_id, project_id, artifact_id, "unsupported") def _do_size( self, @@ -462,6 +445,27 @@ def _update_artifact_error( else: logger.info(f"Successfully updated artifact {artifact_id} with error information") + def _update_distribution_skip( + self, + organization_id: str, + project_id: str, + artifact_id: str, + skip_reason: str, + ) -> None: + """Update artifact with distribution skip state.""" + try: + self._sentry_client.update_artifact( + org=organization_id, + project=project_id, + artifact_id=artifact_id, + data={ + "distribution_state": DistributionState.NOT_RAN.value, + "distribution_skip_reason": skip_reason, + }, + ) + except SentryClientError: + logger.exception(f"Failed to update distribution skip for artifact {artifact_id}") + def _update_size_error_from_exception( self, organization_id: str, diff --git a/src/launchpad/constants.py b/src/launchpad/constants.py index cfece454..0c612c71 100644 --- a/src/launchpad/constants.py +++ b/src/launchpad/constants.py @@ -29,6 +29,13 @@ class PreprodFeature(Enum): BUILD_DISTRIBUTION = "build_distribution" +# Matches PreprodArtifact.DistributionState in sentry +class DistributionState(Enum): + PENDING = 0 + COMPLETED = 1 + NOT_RAN = 2 + + # Health check threshold - consider unhealthy if file not touched in 60 seconds HEALTHCHECK_MAX_AGE_SECONDS = 60.0 @@ -46,8 +53,6 @@ class ProcessingErrorMessage(Enum): SIZE_ANALYSIS_FAILED = "Failed to perform size analysis" ARTIFACT_PARSING_FAILED = "Failed to parse artifact file" UNSUPPORTED_ARTIFACT_TYPE = "Unsupported artifact type" - INVALID_CODE_SIGNATURE = "Cannot distribute app with invalid code signature" - SIMULATOR_BUILD = "Cannot distribute simulator builds" # System-related errors TEMP_FILE_CREATION_FAILED = "Failed to create temporary file" diff --git a/tests/unit/artifacts/test_artifact_processor.py b/tests/unit/artifacts/test_artifact_processor.py index 38c98f23..e3346426 100644 --- a/tests/unit/artifacts/test_artifact_processor.py +++ b/tests/unit/artifacts/test_artifact_processor.py @@ -6,6 +6,7 @@ from launchpad.artifacts.apple.zipped_xcarchive import ZippedXCArchive from launchpad.artifacts.artifact import Artifact from launchpad.constants import ( + DistributionState, ProcessingErrorCode, ProcessingErrorMessage, ) @@ -136,8 +137,7 @@ 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): - """Test that _do_distribution reports an error for unknown artifact types.""" + def test_do_distribution_unknown_artifact_type_skips(self): mock_sentry_client = Mock(spec=SentryClient) mock_sentry_client.update_artifact.return_value = None self.processor._sentry_client = mock_sentry_client @@ -154,12 +154,12 @@ def test_do_distribution_unknown_artifact_type_reports_error(self): project="test-project-id", artifact_id="test-artifact-id", data={ - "error_code": ProcessingErrorCode.ARTIFACT_PROCESSING_ERROR.value, - "error_message": ProcessingErrorMessage.UNSUPPORTED_ARTIFACT_TYPE.value, + "distribution_state": DistributionState.NOT_RAN.value, + "distribution_skip_reason": "unsupported", }, ) - def test_do_distribution_invalid_code_signature_reports_error(self): + def test_do_distribution_invalid_code_signature_skips(self): mock_sentry_client = Mock(spec=SentryClient) mock_sentry_client.update_artifact.return_value = None self.processor._sentry_client = mock_sentry_client @@ -176,13 +176,13 @@ def test_do_distribution_invalid_code_signature_reports_error(self): project="test-project-id", artifact_id="test-artifact-id", data={ - "error_code": ProcessingErrorCode.ARTIFACT_PROCESSING_ERROR.value, - "error_message": ProcessingErrorMessage.INVALID_CODE_SIGNATURE.value, + "distribution_state": DistributionState.NOT_RAN.value, + "distribution_skip_reason": "invalid_signature", }, ) mock_sentry_client.upload_installable_app.assert_not_called() - def test_do_distribution_simulator_build_reports_error(self): + def test_do_distribution_simulator_build_skips(self): mock_sentry_client = Mock(spec=SentryClient) mock_sentry_client.update_artifact.return_value = None self.processor._sentry_client = mock_sentry_client @@ -199,8 +199,8 @@ def test_do_distribution_simulator_build_reports_error(self): project="test-project-id", artifact_id="test-artifact-id", data={ - "error_code": ProcessingErrorCode.ARTIFACT_PROCESSING_ERROR.value, - "error_message": ProcessingErrorMessage.SIMULATOR_BUILD.value, + "distribution_state": DistributionState.NOT_RAN.value, + "distribution_skip_reason": "simulator", }, ) mock_sentry_client.upload_installable_app.assert_not_called() From 5e24e6e0dadf80ef23740ac6f050820d7749a977 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Fri, 6 Mar 2026 10:25:02 +0100 Subject: [PATCH 04/11] fix(artifacts): Use distribution endpoint for skip reporting (EME-422) The update_artifact endpoint silently ignored distribution_state and distribution_skip_reason fields. Use the dedicated distribution endpoint that accepts error_code and error_message instead. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/launchpad/artifact_processor.py | 13 +++---- src/launchpad/constants.py | 11 +++--- src/launchpad/sentry_client.py | 18 ++++++++++ .../unit/artifacts/test_artifact_processor.py | 35 +++++++------------ 4 files changed, 42 insertions(+), 35 deletions(-) diff --git a/src/launchpad/artifact_processor.py b/src/launchpad/artifact_processor.py index 3e33e56c..28a5d772 100644 --- a/src/launchpad/artifact_processor.py +++ b/src/launchpad/artifact_processor.py @@ -33,7 +33,7 @@ from launchpad.artifacts.artifact_factory import ArtifactFactory from launchpad.constants import ( ArtifactType, - DistributionState, + InstallableAppErrorCode, PreprodFeature, ProcessingErrorCode, ProcessingErrorMessage, @@ -452,16 +452,13 @@ def _update_distribution_skip( artifact_id: str, skip_reason: str, ) -> None: - """Update artifact with distribution skip state.""" + """Report distribution skip via the dedicated distribution endpoint.""" try: - self._sentry_client.update_artifact( + self._sentry_client.update_distribution_error( org=organization_id, - project=project_id, artifact_id=artifact_id, - data={ - "distribution_state": DistributionState.NOT_RAN.value, - "distribution_skip_reason": skip_reason, - }, + error_code=InstallableAppErrorCode.SKIPPED.value, + error_message=skip_reason, ) except SentryClientError: logger.exception(f"Failed to update distribution skip for artifact {artifact_id}") diff --git a/src/launchpad/constants.py b/src/launchpad/constants.py index 0c612c71..7b53d05a 100644 --- a/src/launchpad/constants.py +++ b/src/launchpad/constants.py @@ -29,11 +29,12 @@ class PreprodFeature(Enum): BUILD_DISTRIBUTION = "build_distribution" -# Matches PreprodArtifact.DistributionState in sentry -class DistributionState(Enum): - PENDING = 0 - COMPLETED = 1 - NOT_RAN = 2 +# 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 diff --git a/src/launchpad/sentry_client.py b/src/launchpad/sentry_client.py index e2581ecc..24d03872 100644 --- a/src/launchpad/sentry_client.py +++ b/src/launchpad/sentry_client.py @@ -252,6 +252,24 @@ 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_error(self, org: str, artifact_id: str, error_code: int, error_message: str) -> None: + """Report distribution error via the dedicated distribution endpoint.""" + endpoint = f"/api/0/organizations/{org}/preprodartifacts/{artifact_id}/distribution/" + url = self._build_url(endpoint) + body = json.dumps({"error_code": error_code, "error_message": error_message}).encode("utf-8") + + logger.debug(f"PUT {url}") + response = self.session.request( + method="PUT", + url=url, + data=body, + auth=self.auth, + timeout=30, + ) + + if response.status_code != 200: + raise SentryClientError(response=response) + 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 e3346426..90b0ab0a 100644 --- a/tests/unit/artifacts/test_artifact_processor.py +++ b/tests/unit/artifacts/test_artifact_processor.py @@ -6,7 +6,7 @@ from launchpad.artifacts.apple.zipped_xcarchive import ZippedXCArchive from launchpad.artifacts.artifact import Artifact from launchpad.constants import ( - DistributionState, + InstallableAppErrorCode, ProcessingErrorCode, ProcessingErrorMessage, ) @@ -139,7 +139,7 @@ def test_processing_error_message_enum_values(self): def test_do_distribution_unknown_artifact_type_skips(self): mock_sentry_client = Mock(spec=SentryClient) - mock_sentry_client.update_artifact.return_value = None + mock_sentry_client.update_distribution_error.return_value = None self.processor._sentry_client = mock_sentry_client unknown_artifact = Mock(spec=Artifact) @@ -149,19 +149,16 @@ def test_do_distribution_unknown_artifact_type_skips(self): "test-org-id", "test-project-id", "test-artifact-id", unknown_artifact, mock_info ) - mock_sentry_client.update_artifact.assert_called_once_with( + mock_sentry_client.update_distribution_error.assert_called_once_with( org="test-org-id", - project="test-project-id", artifact_id="test-artifact-id", - data={ - "distribution_state": DistributionState.NOT_RAN.value, - "distribution_skip_reason": "unsupported", - }, + error_code=InstallableAppErrorCode.SKIPPED.value, + error_message="unsupported", ) def test_do_distribution_invalid_code_signature_skips(self): mock_sentry_client = Mock(spec=SentryClient) - mock_sentry_client.update_artifact.return_value = None + mock_sentry_client.update_distribution_error.return_value = None self.processor._sentry_client = mock_sentry_client artifact = Mock(spec=ZippedXCArchive) @@ -171,20 +168,17 @@ def test_do_distribution_invalid_code_signature_skips(self): self.processor._do_distribution("test-org-id", "test-project-id", "test-artifact-id", artifact, mock_info) - mock_sentry_client.update_artifact.assert_called_once_with( + mock_sentry_client.update_distribution_error.assert_called_once_with( org="test-org-id", - project="test-project-id", artifact_id="test-artifact-id", - data={ - "distribution_state": DistributionState.NOT_RAN.value, - "distribution_skip_reason": "invalid_signature", - }, + error_code=InstallableAppErrorCode.SKIPPED.value, + error_message="invalid_signature", ) mock_sentry_client.upload_installable_app.assert_not_called() def test_do_distribution_simulator_build_skips(self): mock_sentry_client = Mock(spec=SentryClient) - mock_sentry_client.update_artifact.return_value = None + mock_sentry_client.update_distribution_error.return_value = None self.processor._sentry_client = mock_sentry_client artifact = Mock(spec=ZippedXCArchive) @@ -194,14 +188,11 @@ def test_do_distribution_simulator_build_skips(self): self.processor._do_distribution("test-org-id", "test-project-id", "test-artifact-id", artifact, mock_info) - mock_sentry_client.update_artifact.assert_called_once_with( + mock_sentry_client.update_distribution_error.assert_called_once_with( org="test-org-id", - project="test-project-id", artifact_id="test-artifact-id", - data={ - "distribution_state": DistributionState.NOT_RAN.value, - "distribution_skip_reason": "simulator", - }, + error_code=InstallableAppErrorCode.SKIPPED.value, + error_message="simulator", ) mock_sentry_client.upload_installable_app.assert_not_called() From afa1aa7b939fb53fe8a359d67fccf5fc5d37c216 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Fri, 6 Mar 2026 14:21:41 +0100 Subject: [PATCH 05/11] fix(artifacts): Use PROCESSING_ERROR for unsupported artifact types (EME-422) Unsupported artifact types are genuine errors, not intentional skips. Report them with PROCESSING_ERROR instead of SKIPPED so they surface correctly to users. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/launchpad/artifact_processor.py | 10 +++++++++- tests/unit/artifacts/test_artifact_processor.py | 6 +++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/launchpad/artifact_processor.py b/src/launchpad/artifact_processor.py index 28a5d772..1dcfd225 100644 --- a/src/launchpad/artifact_processor.py +++ b/src/launchpad/artifact_processor.py @@ -363,7 +363,15 @@ def _do_distribution( self._sentry_client.upload_installable_app(organization_id, project_id, artifact_id, f) else: logger.error(f"BUILD_DISTRIBUTION failed for {artifact_id}: unsupported artifact type") - self._update_distribution_skip(organization_id, project_id, artifact_id, "unsupported") + try: + self._sentry_client.update_distribution_error( + org=organization_id, + artifact_id=artifact_id, + error_code=InstallableAppErrorCode.PROCESSING_ERROR.value, + error_message="unsupported artifact type", + ) + except SentryClientError: + logger.exception(f"Failed to update distribution error for artifact {artifact_id}") def _do_size( self, diff --git a/tests/unit/artifacts/test_artifact_processor.py b/tests/unit/artifacts/test_artifact_processor.py index 90b0ab0a..cca70ac7 100644 --- a/tests/unit/artifacts/test_artifact_processor.py +++ b/tests/unit/artifacts/test_artifact_processor.py @@ -137,7 +137,7 @@ 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_skips(self): + def test_do_distribution_unknown_artifact_type_reports_error(self): mock_sentry_client = Mock(spec=SentryClient) mock_sentry_client.update_distribution_error.return_value = None self.processor._sentry_client = mock_sentry_client @@ -152,8 +152,8 @@ def test_do_distribution_unknown_artifact_type_skips(self): mock_sentry_client.update_distribution_error.assert_called_once_with( org="test-org-id", artifact_id="test-artifact-id", - error_code=InstallableAppErrorCode.SKIPPED.value, - error_message="unsupported", + error_code=InstallableAppErrorCode.PROCESSING_ERROR.value, + error_message="unsupported artifact type", ) def test_do_distribution_invalid_code_signature_skips(self): From 3aa2762910fcaff021f3914c94facdea072eb770 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Mon, 9 Mar 2026 10:38:20 +0100 Subject: [PATCH 06/11] fix(artifacts): Widen exception handling for distribution notifications (EME-422) Catch all exceptions (not just SentryClientError) when reporting distribution errors/skips. Network errors like ConnectionError and Timeout from requests aren't subclasses of SentryClientError, so they would propagate uncaught and crash the pipeline. These are best-effort notifications that should never block artifact processing. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/launchpad/artifact_processor.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/launchpad/artifact_processor.py b/src/launchpad/artifact_processor.py index 1dcfd225..f7524438 100644 --- a/src/launchpad/artifact_processor.py +++ b/src/launchpad/artifact_processor.py @@ -370,7 +370,7 @@ def _do_distribution( error_code=InstallableAppErrorCode.PROCESSING_ERROR.value, error_message="unsupported artifact type", ) - except SentryClientError: + except Exception: logger.exception(f"Failed to update distribution error for artifact {artifact_id}") def _do_size( @@ -468,7 +468,7 @@ def _update_distribution_skip( error_code=InstallableAppErrorCode.SKIPPED.value, error_message=skip_reason, ) - except SentryClientError: + except Exception: logger.exception(f"Failed to update distribution skip for artifact {artifact_id}") def _update_size_error_from_exception( From 56ddd6f1c2bdde22615a91bd3178147a78534070 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Wed, 11 Mar 2026 17:46:56 +0100 Subject: [PATCH 07/11] fix(artifacts): Remove iOS distribution skip reporting (EME-422) Revert the iOS-specific changes that reported skip reasons for invalid code signatures and simulator builds. Keep the unsupported artifact type error reporting via the distribution endpoint. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/launchpad/artifact_processor.py | 39 ++++-------------- .../unit/artifacts/test_artifact_processor.py | 41 ------------------- 2 files changed, 7 insertions(+), 73 deletions(-) diff --git a/src/launchpad/artifact_processor.py b/src/launchpad/artifact_processor.py index f7524438..329b3859 100644 --- a/src/launchpad/artifact_processor.py +++ b/src/launchpad/artifact_processor.py @@ -331,20 +331,13 @@ 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 not apple_info.is_code_signature_valid: - logger.warning(f"BUILD_DISTRIBUTION skipped for {artifact_id}: invalid code signature") - self._update_distribution_skip(organization_id, project_id, artifact_id, "invalid_signature") - return - if apple_info.is_simulator: - logger.warning(f"BUILD_DISTRIBUTION skipped for {artifact_id}: simulator build") - self._update_distribution_skip(organization_id, project_id, artifact_id, "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) + 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) elif isinstance(artifact, (AAB, ZippedAAB)): with tempfile.TemporaryDirectory() as temp_dir_str: temp_dir = Path(temp_dir_str) @@ -453,24 +446,6 @@ def _update_artifact_error( else: logger.info(f"Successfully updated artifact {artifact_id} with error information") - def _update_distribution_skip( - self, - organization_id: str, - project_id: str, - artifact_id: str, - skip_reason: str, - ) -> None: - """Report distribution skip via the dedicated distribution endpoint.""" - try: - self._sentry_client.update_distribution_error( - org=organization_id, - artifact_id=artifact_id, - error_code=InstallableAppErrorCode.SKIPPED.value, - error_message=skip_reason, - ) - except Exception: - logger.exception(f"Failed to update distribution skip for artifact {artifact_id}") - def _update_size_error_from_exception( self, organization_id: str, diff --git a/tests/unit/artifacts/test_artifact_processor.py b/tests/unit/artifacts/test_artifact_processor.py index cca70ac7..41fbb250 100644 --- a/tests/unit/artifacts/test_artifact_processor.py +++ b/tests/unit/artifacts/test_artifact_processor.py @@ -3,7 +3,6 @@ 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, @@ -156,46 +155,6 @@ def test_do_distribution_unknown_artifact_type_reports_error(self): error_message="unsupported artifact type", ) - def test_do_distribution_invalid_code_signature_skips(self): - mock_sentry_client = Mock(spec=SentryClient) - mock_sentry_client.update_distribution_error.return_value = None - self.processor._sentry_client = mock_sentry_client - - 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_error.assert_called_once_with( - org="test-org-id", - artifact_id="test-artifact-id", - error_code=InstallableAppErrorCode.SKIPPED.value, - error_message="invalid_signature", - ) - mock_sentry_client.upload_installable_app.assert_not_called() - - def test_do_distribution_simulator_build_skips(self): - mock_sentry_client = Mock(spec=SentryClient) - mock_sentry_client.update_distribution_error.return_value = None - self.processor._sentry_client = mock_sentry_client - - 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_error.assert_called_once_with( - org="test-org-id", - artifact_id="test-artifact-id", - error_code=InstallableAppErrorCode.SKIPPED.value, - error_message="simulator", - ) - mock_sentry_client.upload_installable_app.assert_not_called() - class TestArtifactProcessorMessageHandling: """Test message processing functionality in ArtifactProcessor.""" From ec4a615a3a2fe84c4cbdcd442957029aaad39f82 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Wed, 15 Apr 2026 14:45:03 +0200 Subject: [PATCH 08/11] fix(artifacts): Report distribution skip for iOS builds (EME-422) When Sentry requests BUILD_DISTRIBUTION for an iOS artifact that has an invalid code signature or is a simulator build, Launchpad was silently doing nothing. This left the artifact stuck in a "processing" state in Sentry with no timeout to recover. Report SKIPPED via the distribution endpoint so Sentry can resolve the state. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/launchpad/artifact_processor.py | 38 +++++++++++++---- .../unit/artifacts/test_artifact_processor.py | 41 +++++++++++++++++++ 2 files changed, 72 insertions(+), 7 deletions(-) diff --git a/src/launchpad/artifact_processor.py b/src/launchpad/artifact_processor.py index 329b3859..2e0119b1 100644 --- a/src/launchpad/artifact_processor.py +++ b/src/launchpad/artifact_processor.py @@ -331,13 +331,20 @@ 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._report_distribution_skip(organization_id, artifact_id, "invalid_signature") + return + if apple_info.is_simulator: + logger.warning(f"BUILD_DISTRIBUTION skipped for {artifact_id}: simulator build") + self._report_distribution_skip(organization_id, artifact_id, "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) @@ -446,6 +453,23 @@ def _update_artifact_error( else: logger.info(f"Successfully updated artifact {artifact_id} with error information") + def _report_distribution_skip( + self, + organization_id: str, + artifact_id: str, + skip_reason: str, + ) -> None: + """Report distribution skip via the dedicated distribution endpoint.""" + try: + self._sentry_client.update_distribution_error( + org=organization_id, + artifact_id=artifact_id, + error_code=InstallableAppErrorCode.SKIPPED.value, + error_message=skip_reason, + ) + except Exception: + logger.exception(f"Failed to report distribution skip for artifact {artifact_id}") + def _update_size_error_from_exception( self, organization_id: str, diff --git a/tests/unit/artifacts/test_artifact_processor.py b/tests/unit/artifacts/test_artifact_processor.py index 41fbb250..0e88f8ec 100644 --- a/tests/unit/artifacts/test_artifact_processor.py +++ b/tests/unit/artifacts/test_artifact_processor.py @@ -3,6 +3,7 @@ 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, @@ -155,6 +156,46 @@ def test_do_distribution_unknown_artifact_type_reports_error(self): error_message="unsupported artifact type", ) + def test_do_distribution_invalid_code_signature_reports_skip(self): + mock_sentry_client = Mock(spec=SentryClient) + mock_sentry_client.update_distribution_error.return_value = None + self.processor._sentry_client = mock_sentry_client + + 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_error.assert_called_once_with( + org="test-org-id", + artifact_id="test-artifact-id", + error_code=InstallableAppErrorCode.SKIPPED.value, + error_message="invalid_signature", + ) + 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_error.return_value = None + self.processor._sentry_client = mock_sentry_client + + 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_error.assert_called_once_with( + org="test-org-id", + artifact_id="test-artifact-id", + error_code=InstallableAppErrorCode.SKIPPED.value, + error_message="simulator", + ) + mock_sentry_client.upload_installable_app.assert_not_called() + class TestArtifactProcessorMessageHandling: """Test message processing functionality in ArtifactProcessor.""" From e32d4e76886ac9cf255bf5f94ba96ab8fc0d8762 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Wed, 15 Apr 2026 16:41:59 +0200 Subject: [PATCH 09/11] refactor(artifacts): Unify distribution error/skip reporting (EME-422) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace _report_distribution_skip with _update_distribution_error that mirrors _update_artifact_error — covers both skip and error cases with statsd metrics, structured logging, and SentryClientError handling. Also refactor SentryClient: rename update_distribution_error to update_distribution with a generic data dict, and make _make_json_request model param optional so it can handle fire-and-forget PUT requests. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/launchpad/artifact_processor.py | 52 ++++++++++------ src/launchpad/sentry_client.py | 25 +++----- .../unit/artifacts/test_artifact_processor.py | 60 +++++++++++++++---- 3 files changed, 88 insertions(+), 49 deletions(-) diff --git a/src/launchpad/artifact_processor.py b/src/launchpad/artifact_processor.py index 2e0119b1..a6403e56 100644 --- a/src/launchpad/artifact_processor.py +++ b/src/launchpad/artifact_processor.py @@ -333,11 +333,15 @@ def _do_distribution( apple_info = cast(AppleAppInfo, info) if not apple_info.is_code_signature_valid: logger.warning(f"BUILD_DISTRIBUTION skipped for {artifact_id}: invalid code signature") - self._report_distribution_skip(organization_id, artifact_id, "invalid_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._report_distribution_skip(organization_id, artifact_id, "simulator") + 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) @@ -363,15 +367,9 @@ def _do_distribution( self._sentry_client.upload_installable_app(organization_id, project_id, artifact_id, f) else: logger.error(f"BUILD_DISTRIBUTION failed for {artifact_id}: unsupported artifact type") - try: - self._sentry_client.update_distribution_error( - org=organization_id, - artifact_id=artifact_id, - error_code=InstallableAppErrorCode.PROCESSING_ERROR.value, - error_message="unsupported artifact type", - ) - except Exception: - logger.exception(f"Failed to update distribution error for artifact {artifact_id}") + self._update_distribution_error( + organization_id, artifact_id, InstallableAppErrorCode.PROCESSING_ERROR, "unsupported artifact type" + ) def _do_size( self, @@ -453,22 +451,38 @@ def _update_artifact_error( else: logger.info(f"Successfully updated artifact {artifact_id} with error information") - def _report_distribution_skip( + def _update_distribution_error( self, organization_id: str, artifact_id: str, - skip_reason: str, + error_code: InstallableAppErrorCode, + error_message: str, ) -> None: - """Report distribution skip via the dedicated distribution endpoint.""" + """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_error( + self._sentry_client.update_distribution( org=organization_id, artifact_id=artifact_id, - error_code=InstallableAppErrorCode.SKIPPED.value, - error_message=skip_reason, + data={ + "error_code": error_code.value, + "error_message": error_message, + }, ) - except Exception: - logger.exception(f"Failed to report distribution skip for artifact {artifact_id}") + 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, diff --git a/src/launchpad/sentry_client.py b/src/launchpad/sentry_client.py index 24d03872..dc723a3b 100644 --- a/src/launchpad/sentry_client.py +++ b/src/launchpad/sentry_client.py @@ -252,23 +252,10 @@ 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_error(self, org: str, artifact_id: str, error_code: int, error_message: str) -> None: - """Report distribution error via the dedicated distribution endpoint.""" + def update_distribution(self, org: str, artifact_id: str, data: Dict[str, Any]) -> None: + """Update preprod artifact distribution.""" endpoint = f"/api/0/organizations/{org}/preprodartifacts/{artifact_id}/distribution/" - url = self._build_url(endpoint) - body = json.dumps({"error_code": error_code, "error_message": error_message}).encode("utf-8") - - logger.debug(f"PUT {url}") - response = self.session.request( - method="PUT", - url=url, - data=body, - auth=self.auth, - timeout=30, - ) - - if response.status_code != 200: - raise SentryClientError(response=response) + self._make_json_request("PUT", endpoint, data=data) def upload_size_analysis_file( self, @@ -384,10 +371,10 @@ def _make_json_request( self, method: str, endpoint: str, - model: ResponseModel, + model: ResponseModel | None = None, data: Dict[str, Any] | None = None, timeout: int = 30, - ) -> ResponseModel: + ) -> ResponseModel | None: """Make a JSON request with standard error handling.""" url = self._build_url(endpoint) body = json.dumps(data).encode("utf-8") if data else b"" @@ -402,6 +389,8 @@ def _make_json_request( ) if response.status_code == 200: + if model is None: + return None try: j = response.json() return model.model_validate(j) diff --git a/tests/unit/artifacts/test_artifact_processor.py b/tests/unit/artifacts/test_artifact_processor.py index 0e88f8ec..376717b8 100644 --- a/tests/unit/artifacts/test_artifact_processor.py +++ b/tests/unit/artifacts/test_artifact_processor.py @@ -139,8 +139,10 @@ def test_processing_error_message_enum_values(self): def test_do_distribution_unknown_artifact_type_reports_error(self): mock_sentry_client = Mock(spec=SentryClient) - mock_sentry_client.update_distribution_error.return_value = None + 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() @@ -149,17 +151,29 @@ def test_do_distribution_unknown_artifact_type_reports_error(self): "test-org-id", "test-project-id", "test-artifact-id", unknown_artifact, mock_info ) - mock_sentry_client.update_distribution_error.assert_called_once_with( + mock_sentry_client.update_distribution.assert_called_once_with( org="test-org-id", artifact_id="test-artifact-id", - error_code=InstallableAppErrorCode.PROCESSING_ERROR.value, - error_message="unsupported artifact type", + 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_error.return_value = None + 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() @@ -168,18 +182,30 @@ def test_do_distribution_invalid_code_signature_reports_skip(self): self.processor._do_distribution("test-org-id", "test-project-id", "test-artifact-id", artifact, mock_info) - mock_sentry_client.update_distribution_error.assert_called_once_with( + mock_sentry_client.update_distribution.assert_called_once_with( org="test-org-id", artifact_id="test-artifact-id", - error_code=InstallableAppErrorCode.SKIPPED.value, - error_message="invalid_signature", + 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_error.return_value = None + 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() @@ -188,11 +214,21 @@ def test_do_distribution_simulator_build_reports_skip(self): self.processor._do_distribution("test-org-id", "test-project-id", "test-artifact-id", artifact, mock_info) - mock_sentry_client.update_distribution_error.assert_called_once_with( + mock_sentry_client.update_distribution.assert_called_once_with( org="test-org-id", artifact_id="test-artifact-id", - error_code=InstallableAppErrorCode.SKIPPED.value, - error_message="simulator", + 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() From cd781c4148f0e0596a6903586fa87016bbe4ee88 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Wed, 15 Apr 2026 17:07:00 +0200 Subject: [PATCH 10/11] refactor(artifacts): Use EmptyResponse model instead of optional model (EME-422) Keep _make_json_request signature strict by introducing an EmptyResponse model for endpoints that don't return meaningful data, rather than making the model parameter optional. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/launchpad/sentry_client.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/launchpad/sentry_client.py b/src/launchpad/sentry_client.py index dc723a3b..8ff333c2 100644 --- a/src/launchpad/sentry_client.py +++ b/src/launchpad/sentry_client.py @@ -134,6 +134,10 @@ class RetentionResponse(BaseModel): build_distribution: int +class EmptyResponse(BaseModel): + model_config = ConfigDict(extra="allow") + + def create_retry_session(max_retries: int = 3) -> requests.Session: """Create a requests session with retry configuration.""" session = requests.Session() @@ -252,10 +256,10 @@ 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]) -> None: + def update_distribution(self, org: str, artifact_id: str, data: Dict[str, Any]) -> EmptyResponse: """Update preprod artifact distribution.""" endpoint = f"/api/0/organizations/{org}/preprodartifacts/{artifact_id}/distribution/" - self._make_json_request("PUT", endpoint, data=data) + return self._make_json_request("PUT", endpoint, EmptyResponse, data=data) def upload_size_analysis_file( self, @@ -371,10 +375,10 @@ def _make_json_request( self, method: str, endpoint: str, - model: ResponseModel | None = None, + model: ResponseModel, data: Dict[str, Any] | None = None, timeout: int = 30, - ) -> ResponseModel | None: + ) -> ResponseModel: """Make a JSON request with standard error handling.""" url = self._build_url(endpoint) body = json.dumps(data).encode("utf-8") if data else b"" @@ -389,8 +393,6 @@ def _make_json_request( ) if response.status_code == 200: - if model is None: - return None try: j = response.json() return model.model_validate(j) From d5b3c422674e827ff53f0fe78142302fcbd44276 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Wed, 15 Apr 2026 17:19:54 +0200 Subject: [PATCH 11/11] fix(artifacts): Model actual distribution response instead of EmptyResponse (EME-422) Replace EmptyResponse with DistributionResponse that models the actual backend response (artifactId). Avoids misleading naming that could lead to silent failures if the endpoint response changes. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/launchpad/sentry_client.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/launchpad/sentry_client.py b/src/launchpad/sentry_client.py index 8ff333c2..d7304edd 100644 --- a/src/launchpad/sentry_client.py +++ b/src/launchpad/sentry_client.py @@ -134,8 +134,9 @@ class RetentionResponse(BaseModel): build_distribution: int -class EmptyResponse(BaseModel): - model_config = ConfigDict(extra="allow") +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: @@ -256,10 +257,10 @@ 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]) -> EmptyResponse: + 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, EmptyResponse, data=data) + return self._make_json_request("PUT", endpoint, DistributionResponse, data=data) def upload_size_analysis_file( self,