Skip to content

feat(preprod): Add granular installable app error codes (EME-883) #113440

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 3 commits into from
Apr 24, 2026
+91 −16
Merged

feat(preprod): Add granular installable app error codes (EME-883) #113440

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7da0b69
feat(preprod): Add granular installable app error codes (EME-883)
runningcode Apr 20, 2026
156e0af
ref(preprod): Simplify distribution PUT endpoint validation (EME-883)
runningcode Apr 23, 2026
8315dc9
fix(preprod): Set error message for Sentry-skipped distributions (EME…
runningcode Apr 23, 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
6 changes: 3 additions & 3 deletions src/sentry/preprod/api/endpoints/project_preprod_artifact_update.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -439,11 +439,11 @@ def put(
distro_error_code = PreprodArtifact.InstallableAppErrorCode.NO_QUOTA
distro_error_message = "Distribution quota exceeded"
elif distro_skip_reason == "disabled":
distro_error_code = PreprodArtifact.InstallableAppErrorCode.SKIPPED
distro_error_code = PreprodArtifact.InstallableAppErrorCode.DISTRIBUTION_DISABLED
Copy link
Copy Markdown
Contributor

@chromy chromy Apr 23, 2026

Choose a reason for hiding this comment

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

Are we going to keep the switch in the frontend? If not could just put the right messages here.

Copy link
Copy Markdown
Contributor Author

@runningcode runningcode Apr 23, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, but following from you other comment, I'll emit the messages from launchpad. Started a PR to do that here: getsentry/launchpad#603

Copy link
Copy Markdown
Contributor

@chromy chromy Apr 23, 2026

Choose a reason for hiding this comment

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

These ones I don't think can some from launchpad - because if we skip for quota/disabled I'm not sure the launchpad code ever runs.

distro_error_message = "Distribution disabled for this project"
else:
distro_error_code = PreprodArtifact.InstallableAppErrorCode.SKIPPED
distro_error_message = "Distribution filtered out by project settings"
distro_error_code = PreprodArtifact.InstallableAppErrorCode.DISTRIBUTION_FILTERED
distro_error_message = "Build filtered out by project settings"

head_artifact.installable_app_error_code = distro_error_code
head_artifact.installable_app_error_message = distro_error_message
Expand Down
4 changes: 2 additions & 2 deletions src/sentry/preprod/api/endpoints/project_preprod_distribution.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import logging
from typing import Any, cast

from pydantic import BaseModel, Field
from pydantic import BaseModel
from rest_framework.request import Request
from rest_framework.response import Response

Expand All @@ -24,7 +24,7 @@


class PutDistribution(BaseModel):
error_code: int = Field(ge=0, le=3)
error_code: PreprodArtifact.InstallableAppErrorCode
error_message: str
Comment on lines +27 to 28
Copy link
Copy Markdown
Contributor

@sentry sentry Bot Apr 23, 2026

Choose a reason for hiding this comment

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

Bug: The error_message field in PutDistribution is a required string, but the PR description states it's optional. Omitting it will cause a 400 error.
Severity: HIGH

Suggested Fix

To align the implementation with the stated intent, make the error_message field optional in the PutDistribution model. This can be done by providing a default value, such as error_message: str = "", or by allowing it to be None with error_message: str | None = None.

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/sentry/preprod/api/endpoints/project_preprod_distribution.py#L27-L28

Potential issue: The `PutDistribution` pydantic model defines `error_message` as a
required string without a default value. However, the PR description states this field
"becomes purely optional". If a client, such as the one in a planned follow-up PR, omits
this field from the request payload based on the description, the
`parse_request_with_pydantic` function will catch the resulting
`pydantic.ValidationError` and return an HTTP 400 response. This mismatch between the
implementation and the documented contract will likely cause a production regression
when the client is updated.

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



Expand Down
19 changes: 17 additions & 2 deletions src/sentry/preprod/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,19 @@ class InstallableAppErrorCode(IntEnum):
NO_QUOTA = 1
"""No quota available for distribution."""
SKIPPED = 2
"""Distribution was not requested on this build."""
"""Generic skip; a more specific code should be used when available."""
PROCESSING_ERROR = 3
"""Distribution failed due to a processing error."""
"""Generic distribution failure; a more specific code should be used when available."""
DISTRIBUTION_DISABLED = 4
"""Build distribution is disabled in the project's settings."""
DISTRIBUTION_FILTERED = 5
"""Build does not match the project's distribution filter rules."""
INVALID_CODE_SIGNATURE = 6
"""The build's code signature could not be verified."""
SIMULATOR_BUILD = 7
"""Simulator builds cannot be distributed."""
UNSUPPORTED_ARTIFACT_TYPE = 8
"""The artifact type is not supported for distribution."""

@classmethod
def as_choices(cls) -> tuple[tuple[int, str], ...]:
Expand All @@ -170,6 +180,11 @@ def as_choices(cls) -> tuple[tuple[int, str], ...]:
(cls.NO_QUOTA, "no_quota"),
(cls.SKIPPED, "skipped"),
(cls.PROCESSING_ERROR, "processing_error"),
(cls.DISTRIBUTION_DISABLED, "distribution_disabled"),
(cls.DISTRIBUTION_FILTERED, "distribution_filtered"),
(cls.INVALID_CODE_SIGNATURE, "invalid_code_signature"),
(cls.SIMULATOR_BUILD, "simulator_build"),
(cls.UNSUPPORTED_ARTIFACT_TYPE, "unsupported_artifact_type"),
)

__relocation_scope__ = RelocationScope.Excluded
Expand Down
26 changes: 23 additions & 3 deletions tests/sentry/preprod/api/endpoints/test_project_preprod_artifact_update.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -651,7 +651,7 @@ def test_update_sets_error_code_no_quota(self, mock_has_installable_quota) -> No
assert self.preprod_artifact.installable_app_error_message == "Distribution quota exceeded"

@override_settings(LAUNCHPAD_RPC_SHARED_SECRET=["test-secret-key"])
def test_update_sets_error_code_skipped_when_filtered(self) -> None:
def test_update_sets_error_code_distribution_filtered(self) -> None:
self.preprod_artifact.app_id = "com.my.app"
self.preprod_artifact.save()

Expand All @@ -664,11 +664,31 @@ def test_update_sets_error_code_skipped_when_filtered(self) -> None:
self.preprod_artifact.refresh_from_db()
assert (
self.preprod_artifact.installable_app_error_code
== PreprodArtifact.InstallableAppErrorCode.SKIPPED
== PreprodArtifact.InstallableAppErrorCode.DISTRIBUTION_FILTERED
)
assert (
self.preprod_artifact.installable_app_error_message
== "Distribution filtered out by project settings"
== "Build filtered out by project settings"
)

@override_settings(LAUNCHPAD_RPC_SHARED_SECRET=["test-secret-key"])
def test_update_sets_error_code_distribution_disabled(self) -> None:
from sentry.preprod.quotas import DISTRIBUTION_ENABLED_KEY

self.project.update_option(DISTRIBUTION_ENABLED_KEY, False)

data = {"artifact_type": 1}
response = self._make_request(data)

assert response.status_code == 200
self.preprod_artifact.refresh_from_db()
assert (
self.preprod_artifact.installable_app_error_code
== PreprodArtifact.InstallableAppErrorCode.DISTRIBUTION_DISABLED
)
assert (
self.preprod_artifact.installable_app_error_message
== "Distribution disabled for this project"
)


Expand Down
52 changes: 46 additions & 6 deletions tests/sentry/preprod/api/endpoints/test_project_preprod_distribution.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,24 +61,64 @@ def test_bad_json(self, mock_send_webhook) -> None:
@patch(
"sentry.preprod.api.endpoints.project_preprod_distribution.send_build_distribution_webhook"
)
def test_set_error(self, mock_send_webhook) -> None:
response = self._put(
orjson.dumps({"error_code": 3, "error_message": "Unsupported artifact type"})
)
def test_accepts_generic_processing_error(self, mock_send_webhook) -> None:
response = self._put(orjson.dumps({"error_code": 3, "error_message": "some novel failure"}))

assert response.status_code == 200
self.artifact.refresh_from_db()
assert (
self.artifact.installable_app_error_code
== PreprodArtifact.InstallableAppErrorCode.PROCESSING_ERROR
)
assert self.artifact.installable_app_error_message == "Unsupported artifact type"
assert self.artifact.installable_app_error_message == "some novel failure"

# Verify webhook was sent
mock_send_webhook.assert_called_once()
call_kwargs = mock_send_webhook.call_args
assert call_kwargs.kwargs["organization_id"] == self.project.organization_id

@override_settings(LAUNCHPAD_RPC_SHARED_SECRET=[SHARED_SECRET_FOR_TESTS])
@patch(
"sentry.preprod.api.endpoints.project_preprod_distribution.send_build_distribution_webhook"
)
def test_legacy_payload_stored_verbatim(self, mock_send_webhook) -> None:
# Launchpad deployments that still send the legacy shape
# (error_code=SKIPPED + error_message="invalid_signature", etc.)
# should continue to write unchanged until launchpad emits the
# new granular codes directly.
response = self._put(orjson.dumps({"error_code": 2, "error_message": "invalid_signature"}))

assert response.status_code == 200
self.artifact.refresh_from_db()
assert (
self.artifact.installable_app_error_code
== PreprodArtifact.InstallableAppErrorCode.SKIPPED
)
assert self.artifact.installable_app_error_message == "invalid_signature"

@override_settings(LAUNCHPAD_RPC_SHARED_SECRET=[SHARED_SECRET_FOR_TESTS])
@patch(
"sentry.preprod.api.endpoints.project_preprod_distribution.send_build_distribution_webhook"
)
def test_accepts_new_granular_code(self, mock_send_webhook) -> None:
response = self._put(
orjson.dumps(
{
"error_code": int(
PreprodArtifact.InstallableAppErrorCode.INVALID_CODE_SIGNATURE
),
"error_message": "",
}
)
)

assert response.status_code == 200
self.artifact.refresh_from_db()
assert (
self.artifact.installable_app_error_code
== PreprodArtifact.InstallableAppErrorCode.INVALID_CODE_SIGNATURE
)
assert self.artifact.installable_app_error_message == ""

@override_settings(LAUNCHPAD_RPC_SHARED_SECRET=[SHARED_SECRET_FOR_TESTS])
def test_invalid_error_code(self) -> None:
response = self._put(orjson.dumps({"error_code": 99, "error_message": "bad"}))
Expand Down
Loading