Skip to content
Open
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
53 changes: 43 additions & 10 deletions src/launchpad/artifact_processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
from launchpad.artifacts.artifact_factory import ArtifactFactory
from launchpad.constants import (
ArtifactType,
InstallableAppErrorCode,
PreprodFeature,
ProcessingErrorCode,
ProcessingErrorMessage,
Expand Down Expand Up @@ -302,13 +303,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:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cursorbot said that if a ZippedXCArchive has an invalid code signature or simulator build, it silently does nothing so this addresses that.

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_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)
elif isinstance(artifact, (AAB, ZippedAAB)):
with tempfile.TemporaryDirectory() as temp_dir_str:
temp_dir = Path(temp_dir_str)
Expand All @@ -326,9 +334,16 @@ 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")
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}")

def _do_size(
self,
Expand Down Expand Up @@ -410,6 +425,24 @@ 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}")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused project_id parameter creates monitoring blind spot

Low Severity

_update_distribution_skip accepts project_id but never uses it. Every sibling error-reporting method (_update_artifact_error, _update_size_error) uses project_id for self._statsd.increment calls that track errors with project_id and organization_id tags. The new method emits no StatsD metrics at all, meaning distribution skip events are invisible to monitoring — unlike all other failure paths in this class.

Fix in Cursor Fix in Web


def _update_size_error_from_exception(
self,
organization_id: str,
Expand Down
8 changes: 8 additions & 0 deletions src/launchpad/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,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

Expand Down
18 changes: 18 additions & 0 deletions src/launchpad/sentry_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
62 changes: 62 additions & 0 deletions tests/unit/artifacts/test_artifact_processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@
)

from launchpad.artifact_processor import ArtifactProcessor
from launchpad.artifacts.apple.zipped_xcarchive import ZippedXCArchive
from launchpad.artifacts.artifact import Artifact
from launchpad.constants import (
InstallableAppErrorCode,
ProcessingErrorCode,
ProcessingErrorMessage,
)
Expand Down Expand Up @@ -137,6 +140,65 @@ 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_error.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_distribution_error.assert_called_once_with(
org="test-org-id",
artifact_id="test-artifact-id",
error_code=InstallableAppErrorCode.PROCESSING_ERROR.value,
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."""
Expand Down
Loading