Skip to content

fix(api-client): Handle generic 400 responses in validateEvents - #1253

Open
sentry[bot] wants to merge 5 commits into
mainfrom
seer/fix/validate-events-zoderror
Open

sentry[bot] wants to merge 5 commits into
mainfrom
seer/fix/validate-events-zoderror

Conversation

@sentry

@sentry sentry Bot commented Aug 15, 2026 •

Copy link
Copy Markdown
Contributor

Handle HTTP 400 detail error envelopes from /events/validate/ as typed API errors rather than failing the validation-result schema. The endpoint's get_snuba_params can raise ParseError before it constructs a structured validation response.

Keep allowStatuses: [400] so structured invalid-query results remain available to callers. Responses with a valid discriminator, unrecognized 400 bodies, and malformed successful responses still go through strict schema validation; this does not suppress arbitrary schema failures.

Related: MCP-SERVER-G3K. The captured failure confirms an HTTP 400 followed by missing validation-result fields; its exact response body was not retained in the evidence inspected.

via David Cramer.

--

View Junior Session [Sentry]

Comment thread packages/mcp-core/src/api-client/client.ts Outdated
@sentry sentry Bot changed the title fix(api-client): Prevent ZodError on Sentry API 400 in validateEvents fix(api-client): Handle generic 400 API errors in validateEvents Aug 15, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 2b58c30. Configure here.

Comment thread packages/mcp-core/src/api-client/client.ts Outdated
Comment thread packages/mcp-core/src/api-client/client.ts Outdated
@sentry sentry Bot changed the title fix(api-client): Handle generic 400 API errors in validateEvents fix(api-client): Handle generic 400 responses in validateEvents Aug 15, 2026
@github-actions github-actions Bot added the risk: medium PR risk score: medium label Sep 23, 2026

@sentry-junior sentry-junior Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Addressed the outstanding status-classification feedback in f8d7209: only an actual HTTP 400 schema failure becomes ApiValidationError; malformed 2xx responses still surface as unexpected ZodError. Structured 400 validation results remain supported, matching Sentry's OrganizationEventsValidateEndpoint. Added MSW regressions for generic 400 and malformed 200 bodies. All 98 API-client tests and the full tsc/lint/test gate pass locally. Awaiting current-head CI.

Resolve conflicts with main while retaining both test suites. Classify only HTTP 400 detail envelopes as expected API errors; do not hide malformed validation results.

@sentry-junior sentry-junior Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Rechecked afb9e28 against the latest MCP-SERVER-G3K event and the producer. The narrowed validateEvents guard is preferable to catching every ZodError: Sentry's OrganizationEndpoint raises ParseError for invalid date ranges/project IDs, while OrganizationEventsValidateEndpoint also returns structured HTTP 400 validation results. All 118 API-client tests pass locally, including preservation of structured 400s and reporting malformed 200/400 results.

The remaining evidence gap is the motivating response shape: the latest event confirms HTTP 400 and missing validation fields, but does not capture whether detail was a string or valid was absent. Before claiming this resolves G3K, please provide a sanitized reproduction or shape-only evidence (HTTP status, body container type, whether valid exists, and typeof detail; no payload values or customer data). The current description still claims broad ZodError catching, which is no longer what this head does.

Also blocked on the current-head warden: code-review check: it failed with “Provider unavailable: all 1 chunk failed to analyze,” not a code finding. The PR remains open pending evidence and a successful check rerun.

@sentry-junior sentry-junior Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good at afb9e28. The missing evidence from my last review is now available: recent MCP-SERVER-G3K events are all HTTP 400 responses from /events/validate/ for requests with unparseable absolute time bounds, like end=now or a start that's only a time. Each event's ZodError reports every top-level key (valid, projects, …) as undefined, which means the body is an object with no valid field. That matches the producer: get_snuba_params turns InvalidParams into ParseError(detail="Invalid date range: …"), and DRF serializes that as a string detail envelope.

This head turns that envelope into ApiValidationError. The embedded-agent tool wrapper then hands it back as an actionable "Input Error" instead of logging a ZodError issue. Structured 400 results, malformed 400 bodies, and malformed 2xx bodies still go through strict validation.

All 259 api-client/search-events/agent-tool tests pass locally, and they still pass after merging current main. Removing only the guard makes the HTTP 400 detail regression fail. I reran the Warden workflow, whose earlier failure was a provider outage, and warden: code-review now passes. All current checks are green, including the required test. No code corrections needed.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk: medium PR risk score: medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants