Skip to content

Commit c8d30a3

Browse files
authored
feat(api): Make InvalidParams a ParseError (#112542)
InvalidParam is raised in API parsing contexts; we have 20+ cases already catching it and turning it into a 400, another 15 or so cases where we _should_ be (prior to this change). Rather than continuing to add that boilerplate, we can make InvalidParam a ParseError, so that the default behavior when parameters don't seem correct is a 400 response. The existing uses that don't convert to a 400 still work as they did previously.
1 parent 3d7fa69 commit c8d30a3

File tree

10 files changed

+25
-52
lines changed

10 files changed

+25
-52
lines changed

src/sentry/api/endpoints/organization_events.py

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
from sentry.apidocs.parameters import GlobalParams, OrganizationParams, VisibilityParams
2626
from sentry.apidocs.utils import inline_sentry_response_serializer
2727
from sentry.discover.models import DiscoverSavedQuery, DiscoverSavedQueryTypes
28-
from sentry.exceptions import InvalidParams
2928
from sentry.models.dashboard_widget import DashboardWidget, DashboardWidgetTypes
3029
from sentry.models.organization import Organization
3130
from sentry.ratelimits.config import RateLimitConfig
@@ -194,8 +193,6 @@ def get(self, request: Request, organization: Organization) -> Response:
194193
},
195194
}
196195
)
197-
except InvalidParams as err:
198-
raise ParseError(detail=str(err))
199196

200197
batch_features = self.get_features(organization, request)
201198

src/sentry/exceptions.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
from django.core.exceptions import SuspiciousOperation
2+
from rest_framework.exceptions import ParseError
23

34

45
class InvalidData(Exception):
@@ -89,5 +90,8 @@ def __init__(
8990
self.tombstone_id = tombstone_id
9091

9192

92-
class InvalidParams(Exception):
93+
class InvalidParams(ParseError):
94+
"""Inherits from ParseError so DRF automatically returns a 400 response
95+
when this exception is unhandled in a view."""
96+
9397
pass

src/sentry/issues/endpoints/group_attachments.py

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
from datetime import datetime, timedelta
22

33
from django.utils import timezone
4-
from rest_framework.exceptions import ParseError
54
from rest_framework.request import Request
65
from rest_framework.response import Response
76

@@ -16,7 +15,6 @@
1615
from sentry.api.serializers import EventAttachmentSerializer, serialize
1716
from sentry.api.utils import get_date_range_from_params, handle_query_errors
1817
from sentry.constants import CELL_API_DEPRECATION_DATE
19-
from sentry.exceptions import InvalidParams
2018
from sentry.issues.endpoints.bases.group import GroupEndpoint
2119
from sentry.models.eventattachment import EventAttachment, event_attachment_screenshot_filter
2220
from sentry.models.group import Group
@@ -103,10 +101,7 @@ def get(self, request: Request, group) -> Response:
103101
event_ids = request.GET.getlist("event_id") or None
104102
screenshot = "screenshot" in request.GET
105103

106-
try:
107-
start, end = get_date_range_from_params(request.GET, optional=True)
108-
except InvalidParams as e:
109-
raise ParseError(detail=str(e))
104+
start, end = get_date_range_from_params(request.GET, optional=True)
110105

111106
if start:
112107
attachments = attachments.filter(date_added__gte=start)

src/sentry/issues/endpoints/group_events.py

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
from sentry.apidocs.parameters import CursorQueryParam, EventParams, GlobalParams, IssueParams
3232
from sentry.apidocs.utils import inline_sentry_response_serializer
3333
from sentry.constants import CELL_API_DEPRECATION_DATE
34-
from sentry.exceptions import InvalidParams, InvalidSearchQuery
34+
from sentry.exceptions import InvalidSearchQuery
3535
from sentry.issues.endpoints.bases.group import GroupEndpoint
3636
from sentry.search.events.types import SnubaParams
3737
from sentry.search.utils import InvalidQuery, parse_query
@@ -99,10 +99,7 @@ def get(self, request: Request, group: Group) -> Response:
9999
except (NoResults, ResourceDoesNotExist):
100100
return Response([])
101101

102-
try:
103-
start, end = get_date_range_from_params(request.GET, optional=True)
104-
except InvalidParams as e:
105-
raise ParseError(detail=str(e))
102+
start, end = get_date_range_from_params(request.GET, optional=True)
106103

107104
try:
108105
with handle_query_errors():

src/sentry/issues/endpoints/organization_group_index.py

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
import sentry_sdk
88
from django.utils import timezone
99
from drf_spectacular.utils import extend_schema
10-
from rest_framework.exceptions import ParseError, PermissionDenied
10+
from rest_framework.exceptions import PermissionDenied
1111
from rest_framework.request import Request
1212
from rest_framework.response import Response
1313
from sentry_sdk import start_span
@@ -54,7 +54,7 @@
5454
)
5555
from sentry.apidocs.utils import inline_sentry_response_serializer
5656
from sentry.constants import ALLOWED_FUTURE_DELTA
57-
from sentry.exceptions import InvalidParams, InvalidSearchQuery
57+
from sentry.exceptions import InvalidSearchQuery
5858
from sentry.models.environment import Environment
5959
from sentry.models.group import QUERY_STATUS_LOOKUP, Group, GroupStatus
6060
from sentry.models.groupenvironment import GroupEnvironment
@@ -242,10 +242,7 @@ def _search(
242242
@track_slo_response("workflow")
243243
def get(self, request: Request, organization: Organization) -> Response:
244244
stats_period = request.GET.get("groupStatsPeriod")
245-
try:
246-
start, end = get_date_range_from_stats_period(request.GET)
247-
except InvalidParams as e:
248-
raise ParseError(detail=str(e))
245+
start, end = get_date_range_from_stats_period(request.GET)
249246

250247
expand = request.GET.getlist("expand", [])
251248
collapse = request.GET.getlist("collapse", [])

src/sentry/issues/endpoints/organization_group_index_stats.py

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
from sentry.api.serializers import serialize
1212
from sentry.api.serializers.models.group_stream import StreamGroupSerializerSnuba
1313
from sentry.api.utils import get_date_range_from_stats_period
14-
from sentry.exceptions import InvalidParams
1514
from sentry.issues.endpoints.organization_group_index import ERR_INVALID_STATS_PERIOD
1615
from sentry.models.group import Group
1716
from sentry.models.organization import Organization
@@ -68,10 +67,7 @@ def get(self, request: Request, organization: Organization) -> Response:
6867
"""
6968

7069
stats_period = request.GET.get("groupStatsPeriod")
71-
try:
72-
start, end = get_date_range_from_stats_period(request.GET)
73-
except InvalidParams as e:
74-
raise ParseError(detail=str(e))
70+
start, end = get_date_range_from_stats_period(request.GET)
7571

7672
expand = request.GET.getlist("expand", [])
7773
collapse = request.GET.getlist("collapse", ["base"])

src/sentry/issues/endpoints/organization_issues_count.py

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
from rest_framework.exceptions import ParseError
21
from rest_framework.request import Request
32
from rest_framework.response import Response
43
from sentry_sdk import start_span
@@ -11,7 +10,6 @@
1110
from sentry.api.helpers.group_index import validate_search_filter_permissions
1211
from sentry.api.helpers.group_index.validators import ValidationError
1312
from sentry.api.utils import get_date_range_from_params
14-
from sentry.exceptions import InvalidParams
1513
from sentry.issues.issue_search import convert_query_values, parse_search_query
1614
from sentry.models.organization import Organization
1715
from sentry.organizations.services.organization.model import RpcOrganization
@@ -76,10 +74,7 @@ def _count(
7674

7775
def get(self, request: Request, organization: Organization | RpcOrganization) -> Response:
7876
stats_period = request.GET.get("groupStatsPeriod")
79-
try:
80-
start, end = get_date_range_from_params(request.GET)
81-
except InvalidParams as e:
82-
raise ParseError(detail=str(e))
77+
start, end = get_date_range_from_params(request.GET)
8378

8479
if stats_period not in (None, "", "24h", "14d", "auto"):
8580
return Response({"detail": ERR_INVALID_STATS_PERIOD}, status=400)

src/sentry/releases/endpoints/organization_release_health_data.py

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -71,19 +71,16 @@ def _validate_fields(self, request: Request):
7171
"""
7272
fields = request.GET.getlist("field", [])
7373
for field in fields:
74-
try:
75-
metric_field = parse_field(field, allow_mri=True)
76-
if (
77-
metric_field.op is None
78-
and not metric_field.metric_mri.startswith("e:")
79-
and metric_field.metric_mri
80-
!= "d:sessions/duration.exited@second" # this is special case, derived metric without 'e' as entity
81-
):
82-
raise ParseError(
83-
detail="You can not use generic metric public field without operation"
84-
)
85-
except InvalidParams as exc:
86-
raise ParseError(detail=str(exc))
74+
metric_field = parse_field(field, allow_mri=True)
75+
if (
76+
metric_field.op is None
77+
and not metric_field.metric_mri.startswith("e:")
78+
and metric_field.metric_mri
79+
!= "d:sessions/duration.exited@second" # this is special case, derived metric without 'e' as entity
80+
):
81+
raise ParseError(
82+
detail="You can not use generic metric public field without operation"
83+
)
8784

8885
def get(self, request: Request, organization: Organization) -> Response:
8986
projects = self.get_projects(request, organization)

src/sentry/rules/history/endpoints/project_rule_stats.py

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
from typing import Any, TypedDict
66

77
from drf_spectacular.utils import extend_schema
8-
from rest_framework.exceptions import ParseError
98
from rest_framework.request import Request
109
from rest_framework.response import Response
1110

@@ -16,7 +15,6 @@
1615
from sentry.api.utils import get_date_range_from_params
1716
from sentry.apidocs.constants import RESPONSE_FORBIDDEN, RESPONSE_NOT_FOUND, RESPONSE_UNAUTHORIZED
1817
from sentry.apidocs.parameters import GlobalParams, IssueAlertParams
19-
from sentry.exceptions import InvalidParams
2018
from sentry.models.project import Project
2119
from sentry.models.rule import Rule
2220
from sentry.rules.history import fetch_rule_hourly_stats
@@ -67,9 +65,6 @@ def get(self, request: Request, project: Project, rule: Rule | Workflow) -> Resp
6765
"""
6866
Note that results are returned in hourly buckets.
6967
"""
70-
try:
71-
start, end = get_date_range_from_params(request.GET)
72-
except InvalidParams as e:
73-
raise ParseError(detail=str(e))
68+
start, end = get_date_range_from_params(request.GET)
7469
results = fetch_rule_hourly_stats(rule, start, end)
7570
return Response(serialize(results, request.user, TimeSeriesValueSerializer()))

src/sentry/snuba/metrics/fields/base.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -583,7 +583,7 @@ def generate_snql_function(
583583
try:
584584
return self.snql_func(**kwargs)
585585
except TypeError as e:
586-
raise InvalidParams(e)
586+
raise InvalidParams(str(e))
587587

588588
def get_default_null_values(self) -> int | list[tuple[float]] | None:
589589
return self.default_null_value

0 commit comments

Comments
 (0)