Skip to content

fix(artifacts): Report distribution errors for unsupported and skipped builds (EME-422) #567

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
runningcode merged 11 commits into from
Apr 15, 2026
Merged

fix(artifacts): Report distribution errors for unsupported and skipped builds (EME-422) #567

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
0754d48
fix(artifacts): Report error for unknown artifact types in distributi…
runningcode Feb 23, 2026
b6ae542
fix(artifacts): Report error for iOS distribution failures (EME-422)
runningcode Feb 23, 2026
815e7d4
wip
runningcode Mar 2, 2026
5e24e6e
fix(artifacts): Use distribution endpoint for skip reporting (EME-422)
runningcode Mar 6, 2026
afa1aa7
fix(artifacts): Use PROCESSING_ERROR for unsupported artifact types (…
runningcode Mar 6, 2026
3aa2762
fix(artifacts): Widen exception handling for distribution notificatio…
runningcode Mar 9, 2026
56ddd6f
fix(artifacts): Remove iOS distribution skip reporting (EME-422)
runningcode Mar 11, 2026
ec4a615
fix(artifacts): Report distribution skip for iOS builds (EME-422)
runningcode Apr 15, 2026
e32d4e7
refactor(artifacts): Unify distribution error/skip reporting (EME-422)
runningcode Apr 15, 2026
cd781c4
refactor(artifacts): Use EmptyResponse model instead of optional mode…
runningcode Apr 15, 2026
d5b3c42
fix(artifacts): Model actual distribution response instead of EmptyRe…
runningcode Apr 15, 2026
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
66 changes: 56 additions & 10 deletions src/launchpad/artifact_processor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
from launchpad.artifacts.artifact_factory import ArtifactFactory
from launchpad.constants import (
ArtifactType,
InstallableAppErrorCode,
PreprodFeature,
ProcessingErrorCode,
ProcessingErrorMessage,
Expand Down Expand Up @@ -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)
Expand All @@ -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
Comment thread
cursor[bot] marked this conversation as resolved.
# 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,
Expand Down Expand Up @@ -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}")
Comment on lines +482 to +483
Copy link
Copy Markdown
Contributor

@sentry sentry bot Apr 15, 2026

Choose a reason for hiding this comment

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

Bug: Network errors (ConnectionError, Timeout) from session.request() are not caught, causing them to propagate and be misreported as hard processing failures.
Severity: MEDIUM

Suggested Fix

Widen the exception handler in _update_distribution_error to catch all exceptions: change except SentryClientError: to except Exception:. This matches the PR's stated design intent and the comment in commit 3aa27629 which explicitly addressed this issue.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: src/launchpad/artifact_processor.py#L482-L483

Potential issue: In `_update_distribution_error`, only `SentryClientError` is caught
(line 482), but `session.request()` in `_make_json_request` can directly raise
network-level exceptions like `ConnectionError` or `Timeout` that are NOT subclasses of
`SentryClientError`. These propagate through `_update_distribution_error` →
`_do_distribution` → `process_artifact` → `process_message`, where the outer `except
Exception` logs them as "Processing failed" and increments `artifact.processing.failed`.
The PR's stated intent (as noted in commit `3aa27629`) is that these notifications are
"best-effort" and "should never block artifact processing", but the current
implementation violates this intent. A network transient error during skip reporting
would be misattributed as a hard failure.

Did we get this right? 👍 / 👎 to inform future reviews.

else:
logger.info(f"Successfully updated distribution for {artifact_id} with error information")

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

Expand Down
10 changes: 10 additions & 0 deletions src/launchpad/sentry_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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,
Expand Down
98 changes: 98 additions & 0 deletions tests/unit/artifacts/test_artifact_processor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down Expand Up @@ -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."""
Expand Down
Loading