Skip to content

fix(artifacts): Report error for unknown artifact types in distribution#567

Open
runningcode wants to merge 6 commits intomainfrom
no/eme-422-update-artifact-error-unknown-type
Open

fix(artifacts): Report error for unknown artifact types in distribution#567
runningcode wants to merge 6 commits intomainfrom
no/eme-422-update-artifact-error-unknown-type

Conversation

@runningcode
Copy link
Contributor

When _do_distribution() encounters an unknown artifact type, it only logged
an error but never reported the failure back to Sentry via
_update_artifact_error(). This left the artifact in an inconsistent state
where distribution was requested but silently failed.

Now calls _update_artifact_error() with ARTIFACT_PROCESSING_ERROR /
UNSUPPORTED_ARTIFACT_TYPE so the failure is surfaced to users.

Fixes EME-422

@linear
Copy link

linear bot commented Feb 23, 2026

@runningcode runningcode requested a review from chromy February 23, 2026 09:34
@runningcode runningcode marked this pull request as ready for review February 23, 2026 09:34
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.

runningcode added a commit to getsentry/sentry that referenced this pull request Mar 5, 2026
Add a dedicated endpoint (`PUT
.../files/preprodartifacts/{id}/distribution/`) for launchpad to report
distribution processing errors back to the monolith. This mirrors the
existing `ProjectPreprodSizeEndpoint` pattern.

When launchpad encounters a distribution failure (unsupported artifact
type, invalid code signature, simulator build), it needs a way to set
`installable_app_error_code` and `installable_app_error_message` on the
artifact so the frontend can display the reason. Previously, the only
option was the general `update` endpoint which marks the entire artifact
as failed — but distribution errors shouldn't affect the artifact's
overall state.

Follow-up to #109062. The launchpad side that calls this endpoint is in
getsentry/launchpad#567.

Refs EME-842, EME-422

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>
JonasBa pushed a commit to getsentry/sentry that referenced this pull request Mar 5, 2026
Add a dedicated endpoint (`PUT
.../files/preprodartifacts/{id}/distribution/`) for launchpad to report
distribution processing errors back to the monolith. This mirrors the
existing `ProjectPreprodSizeEndpoint` pattern.

When launchpad encounters a distribution failure (unsupported artifact
type, invalid code signature, simulator build), it needs a way to set
`installable_app_error_code` and `installable_app_error_message` on the
artifact so the frontend can display the reason. Previously, the only
option was the general `update` endpoint which marks the entire artifact
as failed — but distribution errors shouldn't affect the artifact's
overall state.

Follow-up to #109062. The launchpad side that calls this endpoint is in
getsentry/launchpad#567.

Refs EME-842, EME-422

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>
runningcode and others added 4 commits March 6, 2026 10:00
…on (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.
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
@runningcode runningcode force-pushed the no/eme-422-update-artifact-error-unknown-type branch from 5eaf5cd to fe8817e Compare March 6, 2026 09:27
…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) <noreply@anthropic.com>
@sentry
Copy link
Contributor

sentry bot commented Mar 6, 2026

Sentry Build Distribution

App Version Configuration
Hacker News 1.0.2 (13) Release

…ns (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) <noreply@anthropic.com>
@sentry
Copy link
Contributor

sentry bot commented Mar 9, 2026

Sentry Build Distribution

App Version Configuration
Hacker News 1.0.2 (13) Release

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Unused project_id parameter creates monitoring blind spot
    • Added StatsD metrics emission to _update_distribution_skip following the pattern of sibling methods to track distribution skip events with project_id, organization_id, and skip_reason tags.

Create PR

Or push these changes by commenting:

@cursor push cbb5a57a4d
Preview (cbb5a57a4d)
diff --git a/src/launchpad/artifact_processor.py b/src/launchpad/artifact_processor.py
--- a/src/launchpad/artifact_processor.py
+++ b/src/launchpad/artifact_processor.py
@@ -433,6 +433,15 @@
         skip_reason: str,
     ) -> None:
         """Report distribution skip via the dedicated distribution endpoint."""
+        self._statsd.increment(
+            "artifact.distribution.skipped",
+            tags=[
+                f"skip_reason:{skip_reason}",
+                f"project_id:{project_id}",
+                f"organization_id:{organization_id}",
+            ],
+        )
+
         try:
             self._sentry_client.update_distribution_error(
                 org=organization_id,
This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant