From ba33c40b51d80076c9b21353e584cabdf6937cc7 Mon Sep 17 00:00:00 2001 From: Shaun Kaasten <900809+skaasten@users.noreply.github.com> Date: Thu, 30 Jul 2026 11:01:39 -0600 Subject: [PATCH 1/2] fix(dashboards): Don't 500 when validating with an empty project list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Widget query validation built the query params from the serializer's `projects` context. That list is empty whenever a request reaches the endpoint without a `project` query param and `get_projects` resolves to nothing, since it then filters by team membership. `resolve_params` raised `UnqualifiedQueryError`, which subclasses `SnubaError` rather than `InvalidSearchQuery`, so none of the existing `except` clauses caught it and it surfaced as a 500. Widget queries are validated for syntactic correctness, which doesn't depend on which projects are in scope — the project list only exists to satisfy the query builder. Fall back to any active project in the organization when the context list is empty, mirroring the workaround already used in `dashboards/on_completion_hook.py`, and catch `UnqualifiedQueryError` so a request can't 500 even when no fallback project exists. The new handler catches `UnqualifiedQueryError` specifically rather than `SnubaError`, and reports a fixed message instead of the exception text. Sibling `SnubaError` types (`SchemaValidationError`, the ClickHouse code map, `QueryConnectionFailed`) carry Snuba and ClickHouse internals that should not reach API consumers, and infrastructure failures such as `RateLimitExceeded` should not be reported as field validation errors. Observed on both the create and details endpoints, across several organizations. The exact reason the resolved list was empty varies per request and isn't recorded in the events, so this fixes the failure mode rather than any single precondition. Fixes SENTRY-44CF Co-Authored-By: Claude Opus 5 (1M context) --- .../serializers/rest_framework/dashboard.py | 36 +++++++- .../endpoints/test_organization_dashboards.py | 90 +++++++++++++++++++ 2 files changed, 124 insertions(+), 2 deletions(-) diff --git a/src/sentry/api/serializers/rest_framework/dashboard.py b/src/sentry/api/serializers/rest_framework/dashboard.py index f9aae095b48a..0ebbb67271d2 100644 --- a/src/sentry/api/serializers/rest_framework/dashboard.py +++ b/src/sentry/api/serializers/rest_framework/dashboard.py @@ -13,7 +13,7 @@ from sentry import features, options from sentry.api.serializers.rest_framework import CamelSnakeSerializer from sentry.api.serializers.rest_framework.base import convert_dict_key_case, snake_to_camel_case -from sentry.constants import ALL_ACCESS_PROJECTS +from sentry.constants import ALL_ACCESS_PROJECTS, ObjectStatus from sentry.discover.arithmetic import ArithmeticError, categorize_columns from sentry.exceptions import InvalidSearchQuery from sentry.issues.issue_search import parse_search_query @@ -30,6 +30,7 @@ DatasetSourcesTypes, get_max_widget_limit, ) +from sentry.models.project import Project from sentry.models.team import Team from sentry.relay.config.metric_extraction import get_current_widget_specs, widget_exceeds_max_specs from sentry.search.eap.trace_metrics.validator import extract_trace_metric_from_aggregate @@ -44,6 +45,7 @@ set_or_create_on_demand_state, ) from sentry.utils.dates import parse_stats_period +from sentry.utils.snuba import UnqualifiedQueryError from sentry.utils.strings import oxfordize_list from sentry.utils.tracing import set_span_data, start_span @@ -285,7 +287,7 @@ def validate(self, data): params: ParamsType = { "start": datetime.now() - timedelta(days=1), "end": datetime.now(), - "project_id": [p.id for p in self.context["projects"]], + "project_id": self._get_validation_project_ids(), "organization_id": self.context["organization"].id, "environment": self.context.get("environment", []), } @@ -333,6 +335,13 @@ def validate(self, data): except InvalidSearchQuery as err: data["discover_query_error"] = {"conditions": [f"Invalid conditions: {err}"]} return data + except UnqualifiedQueryError: + # Fixed message: sibling SnubaError types carry Snuba/ClickHouse + # internals that must not reach API consumers. + data["discover_query_error"] = { + "conditions": ["Could not validate query: no project available."] + } + return data # TODO(dam): Add validation for metrics fields/queries try: @@ -351,6 +360,29 @@ def validate(self, data): return data + def _get_validation_project_ids(self) -> list[int]: + """Project ids used only to satisfy the query builder during validation. + + Which projects are in scope doesn't affect the outcome, so any active + one in the organization will do when the context list is empty. That + happens routinely: `get_projects` filters on team membership, while + `allow_joinleave` orgs grant project access without it, so any teamless + member of an open-membership org lands here despite seeing everything. + Not an access check — no query runs against the borrowed project. + """ + project_ids = [p.id for p in self.context["projects"]] + if project_ids: + return project_ids + + fallback_id = ( + Project.objects.filter( + organization_id=self.context["organization"].id, status=ObjectStatus.ACTIVE + ) + .values_list("id", flat=True) + .first() + ) + return [fallback_id] if fallback_id is not None else [] + def _get_attr(self, data, attr, empty_value=None): value = data.get(attr) if value is not None: diff --git a/tests/sentry/dashboards/endpoints/test_organization_dashboards.py b/tests/sentry/dashboards/endpoints/test_organization_dashboards.py index 5cb2f709936a..d589a9000878 100644 --- a/tests/sentry/dashboards/endpoints/test_organization_dashboards.py +++ b/tests/sentry/dashboards/endpoints/test_organization_dashboards.py @@ -2666,3 +2666,93 @@ def test_post_validate_only_error_for_invalid_dashboard(self) -> None: assert not Dashboard.objects.filter( organization=self.organization, title="Invalid Dashboard" ).exists() + + # resolve_params only raises on an empty project list outside the test + # environment, so these patch in_test_environment to reach that path. + + def _single_widget_dashboard(self, title: str) -> dict[str, Any]: + return { + "title": title, + "widgets": [ + { + "displayType": "line", + "interval": "5m", + "title": "Transaction count()", + "queries": [ + { + "name": "Transactions", + "fields": ["count()"], + "columns": [], + "aggregates": ["count()"], + "conditions": "event.type:transaction", + } + ], + }, + ], + } + + @patch("sentry.search.events.builder.base.in_test_environment", return_value=False) + def test_post_validate_only_succeeds_for_user_without_team_membership( + self, mock_in_test_environment + ) -> None: + """get_projects filters by team membership, so this resolves to [].""" + assert self.project # the fixture is lazy; the org needs a project + teamless_user = self.create_user() + self.create_member( + organization=self.organization, user=teamless_user, role="member", teams=[] + ) + self.login_as(teamless_user) + + response = self.do_request( + "post", self.url + "?validateOnly=1", data=self._single_widget_dashboard("Teamless") + ) + assert response.status_code == 200, response.data + assert not Dashboard.objects.filter( + organization=self.organization, title="Teamless" + ).exists() + + @patch("sentry.search.events.builder.base.in_test_environment", return_value=False) + def test_post_creates_dashboard_for_teamless_member_of_open_membership_org( + self, mock_in_test_environment + ) -> None: + """allow_joinleave grants project access without team membership. + + These users resolve to an empty list but can see every project, so + creation must still work — not just validation. + """ + org = self.create_organization(owner=self.user, flags=1) # allow_joinleave + team = self.create_team(organization=org) + self.create_project(organization=org, teams=[team]) + member = self.create_user() + self.create_member(organization=org, user=member, role="member", teams=[]) + self.login_as(member) + + url = reverse( + "sentry-api-0-organization-dashboards", + kwargs={"organization_id_or_slug": org.slug}, + ) + response = self.do_request("post", url, data=self._single_widget_dashboard("Open Org")) + assert response.status_code == 201, response.data + assert Dashboard.objects.filter(organization=org, title="Open Org").exists() + + @patch("sentry.search.events.builder.base.in_test_environment", return_value=False) + def test_post_validate_only_does_not_error_for_organization_without_projects( + self, mock_in_test_environment + ) -> None: + """With no projects there is no fallback, so this hits the backstop.""" + empty_org = self.create_organization(owner=self.user) + url = reverse( + "sentry-api-0-organization-dashboards", + kwargs={"organization_id_or_slug": empty_org.slug}, + ) + self.login_as(self.user) + + response = self.do_request( + "post", url + "?validateOnly=1", data=self._single_widget_dashboard("No Projects") + ) + assert response.status_code == 400, response.data + # Exact match: the exception text must not be echoed back. + assert response.data["widgets"][0]["queries"][0]["conditions"] == [ + "Could not validate query: no project available." + ] + assert not Dashboard.objects.filter(organization=empty_org, title="No Projects").exists() From 88b8a14f42361f921925078f7c7c9e24eca6eb54 Mon Sep 17 00:00:00 2001 From: Shaun Kaasten <900809+skaasten@users.noreply.github.com> Date: Fri, 31 Jul 2026 13:44:10 -0600 Subject: [PATCH 2/2] ref(dashboards): Cache the validation fallback project lookup _get_validation_project_ids runs once per widget query, so an empty project context triggered the same fallback lookup up to MAX_WIDGETS times per request. Cache it in the serializer context, which DRF shares across the serializer tree for the life of the request. Co-Authored-By: Claude Opus 5 (1M context) --- .../serializers/rest_framework/dashboard.py | 23 ++++++--- .../endpoints/test_organization_dashboards.py | 47 +++++++++++++++++++ 2 files changed, 63 insertions(+), 7 deletions(-) diff --git a/src/sentry/api/serializers/rest_framework/dashboard.py b/src/sentry/api/serializers/rest_framework/dashboard.py index 0ebbb67271d2..041a3a4b2b4c 100644 --- a/src/sentry/api/serializers/rest_framework/dashboard.py +++ b/src/sentry/api/serializers/rest_framework/dashboard.py @@ -53,6 +53,8 @@ AGGREGATE_BASE = r".*(\w+)\((.*)?\)" EQUATION_PREFIX = "equation|" +_FALLBACK_PROJECT_IDS_KEY = "_validation_fallback_project_ids" + OnDemandExtractionState = DashboardWidgetQueryOnDemand.OnDemandExtractionState DATASET_SOURCE_MAP = {source[1]: source[0] for source in DatasetSourcesTypes.as_choices()} @@ -374,14 +376,21 @@ def _get_validation_project_ids(self) -> list[int]: if project_ids: return project_ids - fallback_id = ( - Project.objects.filter( - organization_id=self.context["organization"].id, status=ObjectStatus.ACTIVE + # Cached in the (request-scoped, tree-wide) serializer context because + # this runs once per widget query, up to MAX_WIDGETS times per request. + if _FALLBACK_PROJECT_IDS_KEY not in self.context: + fallback_id = ( + Project.objects.filter( + organization_id=self.context["organization"].id, status=ObjectStatus.ACTIVE + ) + .values_list("id", flat=True) + .first() ) - .values_list("id", flat=True) - .first() - ) - return [fallback_id] if fallback_id is not None else [] + self.context[_FALLBACK_PROJECT_IDS_KEY] = ( + [fallback_id] if fallback_id is not None else [] + ) + + return self.context[_FALLBACK_PROJECT_IDS_KEY] def _get_attr(self, data, attr, empty_value=None): value = data.get(attr) diff --git a/tests/sentry/dashboards/endpoints/test_organization_dashboards.py b/tests/sentry/dashboards/endpoints/test_organization_dashboards.py index d589a9000878..6fdae266eb76 100644 --- a/tests/sentry/dashboards/endpoints/test_organization_dashboards.py +++ b/tests/sentry/dashboards/endpoints/test_organization_dashboards.py @@ -4,6 +4,8 @@ from typing import Any from unittest.mock import patch +from django.db import connection +from django.test.utils import CaptureQueriesContext from django.urls import reverse from sentry.dashboards.endpoints.organization_dashboards import ( @@ -2735,6 +2737,51 @@ def test_post_creates_dashboard_for_teamless_member_of_open_membership_org( assert response.status_code == 201, response.data assert Dashboard.objects.filter(organization=org, title="Open Org").exists() + @patch("sentry.search.events.builder.base.in_test_environment", return_value=False) + def test_post_validate_only_looks_up_fallback_project_once( + self, mock_in_test_environment + ) -> None: + """The fallback runs per widget query, so its lookup must be cached.""" + assert self.project # the fixture is lazy; the org needs a project + teamless_user = self.create_user() + self.create_member( + organization=self.organization, user=teamless_user, role="member", teams=[] + ) + self.login_as(teamless_user) + + data = { + "title": "Many Widgets", + "widgets": [ + { + "displayType": "line", + "interval": "5m", + "title": f"Widget {i}", + "queries": [ + { + "name": f"q{j}", + "fields": ["count()"], + "columns": [], + "aggregates": ["count()"], + "conditions": "", + } + for j in range(2) + ], + } + for i in range(5) + ], + } + + with CaptureQueriesContext(connection) as queries: + response = self.do_request("post", self.url + "?validateOnly=1", data=data) + assert response.status_code == 200, response.data + + fallback_lookups = [ + q["sql"] + for q in queries.captured_queries + if 'FROM "sentry_project"' in q["sql"] and "LIMIT 1" in q["sql"] + ] + assert len(fallback_lookups) == 1, fallback_lookups + @patch("sentry.search.events.builder.base.in_test_environment", return_value=False) def test_post_validate_only_does_not_error_for_organization_without_projects( self, mock_in_test_environment