From ef2c9ed3ffeb82b34a0e902228ef8a41a5f3cb73 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Mon, 26 Oct 2026 23:49:40 +0000 Subject: [PATCH] fix(mcp): Explain rejected Explore field expressions in search tools The search tools pass Sentry's diagnostic through when it rejects a field or sort, but the diagnostic does not say what to use instead. These are the top dataset-specific field failures in `sentry/mcp-server` over the 7 days to 2026-10-26: - `search_logs` with `min(timestamp)` or `max(timestamp)`: about 2.6K failures from ~470 users. Log timestamps are strings. - `search_errors` with `first_seen()`: about 2.3K failures from ~590 users. The errors dataset has `last_seen()` but no `first_seen()`. - `search_errors` sorted by `issue`: about 1K failures from ~270 users. Sentry rejects that sort even when `issue` is selected. Add a fixed hint when the request sent the rejected expression and Sentry's diagnostic matches it, as the CLI's Explore field hints do. Read-only checks confirmed each replacement: `min(timestamp)` and `last_seen()` on errors, `min/max(timestamp_precise)` on logs and metrics, and sorting by `issue.id`. The backend diagnostic stays in the message, and other failures are unchanged. The validate endpoint accepts `first_seen()` and the `issue` sort, so the final Events API error is checked too. The hints never include caller input. The embedded agent's errors field list now includes `min(timestamp)`, because about 700 of the `first_seen()` failures came from agent-generated fields. Co-Authored-By: Burak Yigit Kaya <126780+BYK@users.noreply.github.com> --- .../src/tools/catalog/search-events.test.ts | 122 ++++++++++++++++++ .../src/tools/support/search-events/config.ts | 2 + .../support/search-events/field-hints.ts | 66 ++++++++++ .../src/tools/support/search-events/search.ts | 59 +++++++-- 4 files changed, 237 insertions(+), 12 deletions(-) create mode 100644 packages/mcp-core/src/tools/support/search-events/field-hints.ts diff --git a/packages/mcp-core/src/tools/catalog/search-events.test.ts b/packages/mcp-core/src/tools/catalog/search-events.test.ts index e3e378862..8e18abadc 100644 --- a/packages/mcp-core/src/tools/catalog/search-events.test.ts +++ b/packages/mcp-core/src/tools/catalog/search-events.test.ts @@ -3,6 +3,7 @@ import { mswServer } from "@sentry/mcp-server-mocks"; import { APICallError, generateText } from "ai"; import { HttpResponse, http } from "msw"; import { beforeEach, describe, expect, it, vi } from "vitest"; +import { ApiValidationError } from "../../api-client"; import { UserInputError } from "../../errors"; import { runSearchEvents } from "../support/search-events/search"; import searchEvents from "./search-events"; @@ -3891,6 +3892,127 @@ describe("search_events", () => { expect(mockGenerateText).not.toHaveBeenCalled(); }); + describe("field recovery hints", () => { + const context = { + constraints: { + organizationSlug: null, + regionUrl: null, + projectSlug: null, + }, + accessToken: "test-token", + userId: "1", + }; + const baseParams = { + organizationSlug: "test-org", + regionUrl: null, + projectSlug: null, + period: "24h", + limit: 10, + includeExplanation: false, + }; + + beforeEach(() => { + process.env.OPENAI_API_KEY = ""; + }); + + it("explains a rejected log timestamp aggregate", async () => { + const error = + "timestamp is invalid for parameter 1 in max. Its a string type field, but it must be one of these types: {'number'}"; + mswServer.use( + http.get( + "https://sentry.io/api/0/organizations/test-org/events/validate/", + () => + HttpResponse.json({ + ...validEventsValidationResponse, + valid: false, + field: [ + { name: "max(timestamp)", valid: false, attrType: null, error }, + ], + }), + ), + ); + + const result = searchEvents.handler( + { + ...baseParams, + dataset: "logs", + query: "severity:error", + fields: ["severity", "max(timestamp)"], + sort: "-max(timestamp)", + }, + context, + ); + + await expect(result).rejects.toThrow(UserInputError); + await expect(result).rejects.toThrow(error); + await expect(result).rejects.toThrow( + "Use `min(timestamp_precise)` or `max(timestamp_precise)`", + ); + }); + + it.each([ + { + detail: "first_seen is not a valid function", + fields: ["issue", "count()", "first_seen()"], + sort: "-count()", + hint: "Use `min(timestamp)` for the first occurrence", + }, + { + detail: "Cannot sort by a field that is not selected.", + fields: ["issue", "count()"], + sort: "issue", + hint: "Sort by an aggregate such as `-count()`, or by `issue.id`.", + }, + ])("explains the errors API rejection: $detail", async (testCase) => { + mswServer.use( + http.get("https://sentry.io/api/0/organizations/test-org/events/", () => + HttpResponse.json({ detail: testCase.detail }, { status: 400 }), + ), + ); + + const result = searchEvents.handler( + { + ...baseParams, + dataset: "errors", + query: "level:error", + fields: testCase.fields, + sort: testCase.sort, + }, + context, + ); + + await expect(result).rejects.toThrow(UserInputError); + await expect(result).rejects.toThrow( + `API error (400): ${testCase.detail}`, + ); + await expect(result).rejects.toThrow(testCase.hint); + }); + + it("keeps the API error when the request did not send the rejected expression", async () => { + mswServer.use( + http.get("https://sentry.io/api/0/organizations/test-org/events/", () => + HttpResponse.json( + { detail: "first_seen is not a valid function" }, + { status: 400 }, + ), + ), + ); + + const result = searchEvents.handler( + { + ...baseParams, + dataset: "errors", + query: "level:error", + fields: ["issue", "count()"], + sort: "-count()", + }, + context, + ); + + await expect(result).rejects.toThrow(ApiValidationError); + }); + }); + it("keeps the caller's dataset when lockDataset is set", async () => { mockGenerateText.mockResolvedValueOnce( mockAIResponse("logs", "level:error"), diff --git a/packages/mcp-core/src/tools/support/search-events/config.ts b/packages/mcp-core/src/tools/support/search-events/config.ts index 94e866f48..2e26e3af5 100644 --- a/packages/mcp-core/src/tools/support/search-events/config.ts +++ b/packages/mcp-core/src/tools/support/search-events/config.ts @@ -433,6 +433,8 @@ export const DATASET_FIELDS = { "count_if(field,equals,value)": "Errors-only conditional count, e.g. count_if(error.handled,equals,false). Spans, logs, and metrics use count_if(`filter`) instead", "last_seen()": "Most recent timestamp of the group", + "min(timestamp)": + "Earliest timestamp of the group (there is no first_seen() function)", "eps()": "Events per second rate", "epm()": "Events per minute rate", }, diff --git a/packages/mcp-core/src/tools/support/search-events/field-hints.ts b/packages/mcp-core/src/tools/support/search-events/field-hints.ts new file mode 100644 index 000000000..df6158be7 --- /dev/null +++ b/packages/mcp-core/src/tools/support/search-events/field-hints.ts @@ -0,0 +1,66 @@ +import { parseFunction } from "@sentry/toolkit-core/events-fields"; +import { getSortField } from "@sentry/toolkit-core/events-sort"; +import type { PublicEventsDataset } from "../../../utils/events-datasets"; + +const TIMESTAMP_AGGREGATE_HINT = + "In logs and metrics, `timestamp` is a string. Use `min(timestamp_precise)` or `max(timestamp_precise)` for the earliest or latest time; they return Unix timestamps in nanoseconds."; +const FIRST_SEEN_HINT = + "The errors dataset has no `first_seen()`. Use `min(timestamp)` for the first occurrence, and `max(timestamp)` or `last_seen()` for the latest."; +const ISSUE_SORT_HINT = + "The errors dataset cannot sort by `issue`. Sort by an aggregate such as `-count()`, or by `issue.id`."; + +/** + * Recovery hints for field expressions Sentry rejects in a dataset. + * + * Each hint needs both the expression in the request and Sentry's matching + * diagnostic, so unrelated failures stay unchanged. Hints are fixed text and + * never echo caller input. + */ +export function getFieldRecoveryHints({ + dataset, + fields, + sort, + detail, +}: { + dataset: PublicEventsDataset; + fields: readonly string[]; + sort: string; + detail: string; +}): string[] { + const sortField = getSortField(sort.trim()); + const expressions = [...fields, sortField].map(parseFunction); + const hints: string[] = []; + + if ( + (dataset === "logs" || dataset === "metrics") && + expressions.some( + (parsed) => + (parsed?.name === "min" || parsed?.name === "max") && + parsed.arguments.length === 1 && + parsed.arguments[0] === "timestamp" && + detail.includes( + `timestamp is invalid for parameter 1 in ${parsed.name}.`, + ), + ) + ) { + hints.push(TIMESTAMP_AGGREGATE_HINT); + } + + if ( + dataset === "errors" && + expressions.some((parsed) => parsed?.name === "first_seen") && + detail.includes("first_seen is not a valid function") + ) { + hints.push(FIRST_SEEN_HINT); + } + + if ( + dataset === "errors" && + sortField === "issue" && + detail.includes("Cannot sort by a field that is not selected.") + ) { + hints.push(ISSUE_SORT_HINT); + } + + return hints; +} diff --git a/packages/mcp-core/src/tools/support/search-events/search.ts b/packages/mcp-core/src/tools/support/search-events/search.ts index c8aaa5910..029a02406 100644 --- a/packages/mcp-core/src/tools/support/search-events/search.ts +++ b/packages/mcp-core/src/tools/support/search-events/search.ts @@ -2,6 +2,7 @@ import { getColumnsAndAggregates } from "@sentry/toolkit-core/events-fields"; import { getActiveSpan } from "@sentry/core"; import { ensureSortInFields } from "@sentry/toolkit-core/events-sort"; import { z } from "zod"; +import { ApiValidationError } from "../../../api-client"; import { setTargetTagsAndAttributes } from "../../../telem/scope"; import { UserInputError } from "../../../errors"; import { hasAgentProvider } from "../../../internal/agents/provider-factory"; @@ -34,6 +35,7 @@ import { RECOMMENDED_FIELDS, TRACE_METRICS_SAMPLE_IDENTITY_FIELDS, } from "./config"; +import { getFieldRecoveryHints } from "./field-hints"; import { formatErrorResults, formatLogResults, @@ -141,6 +143,10 @@ function hasFields(fields?: string[] | null): fields is string[] { return Array.isArray(fields) && fields.length > 0; } +function appendHints(message: string, hints: readonly string[]): string { + return hints.length > 0 ? `${message}\n\n${hints.join("\n")}` : message; +} + function formatSearchValue(value: string): string { return /^[^\s"',[\]]+$/.test(value) ? value : JSON.stringify(value); } @@ -1080,7 +1086,15 @@ async function executeSearchEvents( const formatted = formatEventsValidationResults(lastValidation); throw new UserInputError( formatted - ? `Search validation failed:\n${formatted}` + ? appendHints( + `Search validation failed:\n${formatted}`, + getFieldRecoveryHints({ + dataset, + fields: requestFields, + sort: sortParam, + detail: formatted, + }), + ) : "Search validation failed.", ); } @@ -1092,17 +1106,38 @@ async function executeSearchEvents( environment, ); - const eventsResponse = await apiService.searchEvents({ - organizationSlug, - query: sentryQuery, - fields: finalRequestFields, - limit: params.limit, - projectId, - dataset, - sort: sortParam, - crossEventQueries: seerTranslation?.crossEventQueries, - ...timeParams, - }); + let eventsResponse: Awaited>; + try { + eventsResponse = await apiService.searchEvents({ + organizationSlug, + query: sentryQuery, + fields: finalRequestFields, + limit: params.limit, + projectId, + dataset, + sort: sortParam, + crossEventQueries: seerTranslation?.crossEventQueries, + ...timeParams, + }); + } catch (error) { + // The validate endpoint misses some errors-dataset rejections, such as + // `first_seen()` and sorting by `issue`, so check the final API error too. + if (!(error instanceof ApiValidationError)) { + throw error; + } + const hints = getFieldRecoveryHints({ + dataset, + fields: finalRequestFields, + sort: sortParam, + detail: error.message, + }); + if (hints.length === 0) { + throw error; + } + throw new UserInputError(appendHints(error.toUserMessage(), hints), { + cause: error, + }); + } const { columns: groupByFields, aggregates: aggregateFunctions } = getColumnsAndAggregates(fields);