From f2646c6b17ca8a944b0b86000ec060c37e09ec8c Mon Sep 17 00:00:00 2001 From: Jeremy Stanley Date: Tue, 2 Jun 2026 15:18:11 -0700 Subject: [PATCH 1/2] feat(api): tighten Response on 3 endpoints + add as_validation_errors helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Continues the Response[T] rollout (#116717 was round 1). Three more endpoints tightened via source typing — no `cast()` calls. Two patterns demonstrated: 1. **Typed local variable** for locally-built dicts: - `project_filters.py`: `results: list[ProjectFilterResponse] = []` - `project_replay_jobs_delete.py` POST success path: `response: ReplayDeletionJobDetailResponse = {"data": response_data}` 2. **Validation-error union arm** for DRF `serializer.errors` paths. Added a shared `ValidationErrorResponse` type alias and `as_validation_errors(serializer)` helper to `apidocs/response_types.py`, alongside the existing `DetailResponse`. The helper projects DRF's `ReturnDict[Any, Any]` into a plain `dict[str, list[str]]` so a `Response[ValidationErrorResponse]` union arm is structurally satisfied without `cast()`: def post(...) -> Response[FooResponse] | Response[ValidationErrorResponse]: if not serializer.is_valid(): return Response(as_validation_errors(serializer), status=400) Applied to `organization_trace_item_attributes.py` and `project_replay_jobs_delete.py` POST. The `project_replay_jobs_delete.py` GET endpoints are tightened to `Response[ReplayDeletionJobListResponse]` and `Response[ReplayDeletionJobDetailResponse]` — clean tightenings. Verification: - mypy: 0 errors across all touched files - prek: clean - `sentry django spectacular`: byte-identical OpenAPI vs master Co-Authored-By: Claude Opus 4.7 (1M context) --- .../organization_trace_item_attributes.py | 7 ++- src/sentry/api/endpoints/project_filters.py | 4 +- src/sentry/apidocs/response_types.py | 54 +++++++++++++++---- .../endpoints/project_replay_jobs_delete.py | 15 ++++-- 4 files changed, 61 insertions(+), 19 deletions(-) diff --git a/src/sentry/api/endpoints/organization_trace_item_attributes.py b/src/sentry/api/endpoints/organization_trace_item_attributes.py index 08ce0cafb2129e..044c3198459ecb 100644 --- a/src/sentry/api/endpoints/organization_trace_item_attributes.py +++ b/src/sentry/api/endpoints/organization_trace_item_attributes.py @@ -47,6 +47,7 @@ from sentry.apidocs.constants import RESPONSE_FORBIDDEN, RESPONSE_NOT_FOUND, RESPONSE_UNAUTHORIZED from sentry.apidocs.examples.trace_item_attribute_examples import TraceItemAttributeExamples from sentry.apidocs.parameters import CursorQueryParam, GlobalParams +from sentry.apidocs.response_types import ValidationErrorResponse, as_validation_errors from sentry.apidocs.utils import inline_sentry_response_serializer from sentry.auth.staff import is_active_staff from sentry.auth.superuser import is_active_superuser @@ -362,7 +363,9 @@ class OrganizationTraceItemAttributesEndpoint(OrganizationTraceItemAttributesEnd }, examples=TraceItemAttributeExamples.LIST_TRACE_ITEM_ATTRIBUTES, ) - def get(self, request: Request, organization: Organization) -> Response: + def get( + self, request: Request, organization: Organization + ) -> Response[list[TraceItemAttributeKey]] | Response[ValidationErrorResponse]: """ List the attribute keys available on a given trace item dataset (spans, logs, trace metrics, etc.), with optional substring and structured filtering. @@ -372,7 +375,7 @@ def get(self, request: Request, organization: Organization) -> Response: serializer = OrganizationTraceItemAttributesEndpointSerializer(data=request.GET) if not serializer.is_valid(): - return Response(serializer.errors, status=400) + return Response(as_validation_errors(serializer), status=400) try: snuba_params = self.get_snuba_params(request, organization) diff --git a/src/sentry/api/endpoints/project_filters.py b/src/sentry/api/endpoints/project_filters.py index 4222aa0d1cd821..197613aaa4c361 100644 --- a/src/sentry/api/endpoints/project_filters.py +++ b/src/sentry/api/endpoints/project_filters.py @@ -42,12 +42,12 @@ class ProjectFiltersEndpoint(ProjectEndpoint): }, examples=ProjectExamples.GET_PROJECT_FILTERS, ) - def get(self, request: Request, project) -> Response: + def get(self, request: Request, project) -> Response[list[ProjectFilterResponse]]: """ Retrieve a list of filters for a given project. `active` will be either a boolean or a list for the legacy browser filters. """ - results = [] + results: list[ProjectFilterResponse] = [] for flt in inbound_filters.get_all_filter_specs(): results.append( { diff --git a/src/sentry/apidocs/response_types.py b/src/sentry/apidocs/response_types.py index 877d928e1eed83..83de14f61980d5 100644 --- a/src/sentry/apidocs/response_types.py +++ b/src/sentry/apidocs/response_types.py @@ -1,16 +1,18 @@ -"""Shared TypedDicts for endpoint Response annotations. +"""Shared response shapes and helpers for endpoint Response annotations. -This module holds TypedDict shapes that recur across multiple endpoints. -It is *not* authoritative — endpoints whose error/response shapes don't -match anything here are expected to declare local TypedDicts in their -own files. The structural linter at -`sentry.apidocs._check_response_annotation_matches_schema` is name-agnostic; -it does not special-case any TypedDict name in this module. +This module holds TypedDicts, type aliases, and small helpers that recur +across multiple endpoints in the Response[T] typing rollout. It is *not* +authoritative — endpoints whose error/response shapes don't match anything +here are expected to declare local types in their own files. The structural +linter at `sentry.apidocs._check_response_annotation_matches_schema` is +name-agnostic; it does not special-case any name in this module. `DetailResponse` is included because DRF's exception handler renders every uncaught `APIException` subclass as `{"detail": "..."}` and a non-trivial -number of endpoints return that shape inline. Other shapes can graduate -into this module as they emerge from real usage. +number of endpoints return that shape inline. + +`ValidationErrorResponse` + `as_validation_errors()` cover the parallel +case for DRF `Response(serializer.errors, status=400)` paths. The module is named `response_types` rather than `types` to avoid shadowing Python's stdlib `types` module under subprocess tooling (e.g. some prek hooks). @@ -18,7 +20,9 @@ from __future__ import annotations -from typing import TypedDict +from typing import Any, TypeAlias, TypedDict + +from rest_framework import serializers class DetailResponse(TypedDict): @@ -31,3 +35,33 @@ class DetailResponse(TypedDict): """ detail: str + + +ValidationErrorResponse: TypeAlias = dict[str, list[str]] +"""DRF's flat validation-error body shape: `{field_name: [error_message, ...]}`. + +The runtime value of `serializer.errors` is a `ReturnDict[Any, Any]` that +mypy can't structurally match against any typed `Response[T]` union arm. +Use this alias as the validation-error arm: + + def post(...) -> Response[FooResponse] | Response[ValidationErrorResponse]: + +and produce the body via `as_validation_errors(serializer)` below. + +Limitation: this alias is for *flat* serializers (field names → list of +error strings). Nested serializers whose `.errors` produce +`{field: {nested_field: [...]}}` shapes need a richer local type. +""" + + +def as_validation_errors( + serializer: serializers.Serializer[Any], +) -> ValidationErrorResponse: + """Project a DRF `Serializer.errors` ReturnDict into a structurally typed + `dict[str, list[str]]` so a `Response[ValidationErrorResponse]` union arm + is satisfied without `cast()`. Use immediately after `not serializer.is_valid()`: + + if not serializer.is_valid(): + return Response(as_validation_errors(serializer), status=400) + """ + return {k: list(v) for k, v in serializer.errors.items()} diff --git a/src/sentry/replays/endpoints/project_replay_jobs_delete.py b/src/sentry/replays/endpoints/project_replay_jobs_delete.py index 32dd4ccbbf127d..1750ba27fe6a84 100644 --- a/src/sentry/replays/endpoints/project_replay_jobs_delete.py +++ b/src/sentry/replays/endpoints/project_replay_jobs_delete.py @@ -18,6 +18,7 @@ from sentry.apidocs.constants import RESPONSE_BAD_REQUEST, RESPONSE_FORBIDDEN, RESPONSE_NOT_FOUND from sentry.apidocs.examples.replay_examples import ReplayExamples from sentry.apidocs.parameters import GlobalParams, ReplayParams +from sentry.apidocs.response_types import ValidationErrorResponse, as_validation_errors from sentry.apidocs.utils import inline_sentry_response_serializer from sentry.replays.endpoints.project_replay_endpoint import ProjectReplayEndpoint from sentry.replays.models import ReplayDeletionJobModel @@ -109,7 +110,7 @@ class ProjectReplayDeletionJobsIndexEndpoint(ProjectEndpoint): }, examples=ReplayExamples.GET_REPLAY_DELETION_JOBS, ) - def get(self, request: Request, project) -> Response: + def get(self, request: Request, project) -> Response[ReplayDeletionJobListResponse]: """ Retrieve a collection of replay delete jobs. """ @@ -146,7 +147,9 @@ def get(self, request: Request, project) -> Response: }, examples=ReplayExamples.CREATE_REPLAY_DELETION_JOB, ) - def post(self, request: Request, project) -> Response: + def post( + self, request: Request, project + ) -> Response[ReplayDeletionJobDetailResponse] | Response[ValidationErrorResponse]: """ Create a new replay deletion job. """ @@ -155,7 +158,7 @@ def post(self, request: Request, project) -> Response: serializer = ReplayDeletionJobCreateSerializer(data=request.data) if not serializer.is_valid(): - return Response(serializer.errors, status=400) + return Response(as_validation_errors(serializer), status=400) data = serializer.validated_data["data"] @@ -186,7 +189,7 @@ def post(self, request: Request, project) -> Response: ) response_data = serialize(job, request.user, ReplayDeletionJobSerializer()) - response = {"data": response_data} + response: ReplayDeletionJobDetailResponse = {"data": response_data} return Response(response, status=201) @@ -215,7 +218,9 @@ class ProjectReplayDeletionJobDetailEndpoint(ProjectReplayEndpoint): }, examples=ReplayExamples.GET_REPLAY_DELETION_JOB, ) - def get(self, request: Request, project, job_id: int) -> Response: + def get( + self, request: Request, project, job_id: int + ) -> Response[ReplayDeletionJobDetailResponse]: """ Fetch a replay delete job instance. """ From 81722e5dbb567d2f3b38dfb0bb0a39de5e73f490 Mon Sep 17 00:00:00 2001 From: Jeremy Stanley Date: Tue, 2 Jun 2026 15:44:34 -0700 Subject: [PATCH 2/2] fix(api): preserve DRF's nested validation-error structure in as_validation_errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `{k: list(v) for k, v in serializer.errors.items()}` transform collapsed nested error dicts (from nested serializers or `validate()`-raised non-field errors) to just the key list, losing the actual error messages. `ProjectReplayDeletionJobsIndexTest::test_post_validation_errors` caught this — `ReplayDeletionJobCreateSerializer` wraps an inner `ReplayDeletionJobCreateDataSerializer`, so its errors are `{"data": {"non_field_errors": [...]}}` and `list({"non_field_errors": [...]})` returned `["non_field_errors"]` instead of preserving the message. Fix: pass DRF's structure through verbatim — `dict(serializer.errors)` just narrows the static type without traversing. Widen the alias to `dict[str, Any]` to be honest about the runtime shape (flat lists, nested dicts, non-field-error dicts all occur in real serializers). Co-Authored-By: Claude Opus 4.7 (1M context) --- src/sentry/apidocs/response_types.py | 34 +++++++++++++++++----------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/src/sentry/apidocs/response_types.py b/src/sentry/apidocs/response_types.py index 83de14f61980d5..48736e24f6f6d7 100644 --- a/src/sentry/apidocs/response_types.py +++ b/src/sentry/apidocs/response_types.py @@ -37,20 +37,25 @@ class DetailResponse(TypedDict): detail: str -ValidationErrorResponse: TypeAlias = dict[str, list[str]] -"""DRF's flat validation-error body shape: `{field_name: [error_message, ...]}`. - -The runtime value of `serializer.errors` is a `ReturnDict[Any, Any]` that -mypy can't structurally match against any typed `Response[T]` union arm. -Use this alias as the validation-error arm: +ValidationErrorResponse: TypeAlias = dict[str, Any] +"""DRF's validation-error body shape: `{field_name: , ...}`. + +DRF emits a few different value shapes here depending on the serializer: +- Flat field errors: `{"field": ["error msg", ...]}` +- Nested (e.g. `Serializer` with a nested `Serializer` field): + `{"field": {"nested_field": ["error msg", ...]}}` +- Non-field errors (raised in `validate()`): + `{"non_field_errors": ["error msg", ...]}` + +The alias is intentionally `dict[str, Any]` — narrower types like +`dict[str, list[str]]` collapse the nested-dict case and lose the error +messages at runtime. The runtime value of `serializer.errors` is a +`ReturnDict[Any, Any]` that mypy can't structurally match against any +typed `Response[T]` union arm, so use this alias as the union arm: def post(...) -> Response[FooResponse] | Response[ValidationErrorResponse]: and produce the body via `as_validation_errors(serializer)` below. - -Limitation: this alias is for *flat* serializers (field names → list of -error strings). Nested serializers whose `.errors` produce -`{field: {nested_field: [...]}}` shapes need a richer local type. """ @@ -58,10 +63,13 @@ def as_validation_errors( serializer: serializers.Serializer[Any], ) -> ValidationErrorResponse: """Project a DRF `Serializer.errors` ReturnDict into a structurally typed - `dict[str, list[str]]` so a `Response[ValidationErrorResponse]` union arm - is satisfied without `cast()`. Use immediately after `not serializer.is_valid()`: + `dict[str, Any]` so a `Response[ValidationErrorResponse]` union arm is + satisfied without `cast()`. The DRF error structure (flat or nested) is + preserved verbatim — only the static type is narrowed. + + Use immediately after `not serializer.is_valid()`: if not serializer.is_valid(): return Response(as_validation_errors(serializer), status=400) """ - return {k: list(v) for k, v in serializer.errors.items()} + return dict(serializer.errors)