Skip to content

fix(api): classify transient database reads and client-caused failures instead of answering 500 - #9639

Merged
waleedlatif1 merged 3 commits into
stagingfrom
fix/route-5xx-classification
Nov 3, 2026
Merged

waleedlatif1 merged 3 commits into
stagingfrom
fix/route-5xx-classification

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • withRouteHandler answers a GET/HEAD whose query failed transiently (replica recovery conflict, statement/lock/transaction timeout, lost connection, server restart) with 503 + Retry-After in the route family's own envelope, instead of an unhandled 500. Classification reuses getTransientDatabaseFailure, so socket errors count only when a database query carried them. Supersedes fix(logs): return 503 instead of 500 when a v2 logs read is cancelled #9558, which fixed only the v2 logs routes
  • Writes stay 500 on purpose: an earlier statement in the same request may already have committed, so "retry later" would be unsafe advice
  • Mothership chat lookups treat a non-UUID chat id as not found (404) instead of letting Postgres reject it with 22P02
  • File serve answers 404 for a key with no storage-context prefix and for a missing storage object, and logs both at info
  • Fork activity feed projects the Enterprise-plan refusal (403) instead of 500
  • Workspace member removal falls back to the orchestration projection, so not_found/validation stop surfacing as 500
  • Moving several same-named files into one folder answers the existing 409 conflict instead of a unique-index 23505 → 500
  • Logs export answers 400 for invalid filter params; a body that does not parse as multipart is a MultipartFieldValidationError (400) for every multipart route
  • Integration catalog maps a targeted MCP server's connection/DNS/cooldown/egress failure to its 502/503/403 via the typed half of categorizeError (new categorizeMcpServerError), never the substring fallback
  • Workflow operations count declined operations per operation, not per skip: an add that drops several disallowed tools no longer drives applied negative and fails response validation with a 500

Type of Change

  • Bug fix

Testing

  • New cases in with-route-handler.test.ts (503 for four transient classes + HEAD; 500 kept for writes, user cancel, permanent query error, non-DB socket reset; no SQL in the body), stream-limits.test.ts (malformed multipart, both read branches), lifecycle.test.ts (malformed chat id → null with no query), operations.test.ts (multi-skip operation counted once). Each failed with its fix reverted
  • Affected suites pass locally: 214 files, 2639 tests; biome clean
  • Type-check, check:audits, and the full suite run in CI

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing (new tests pass the test-audit authoring gate)
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel

vercel Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
docs Ignored Ignored Preview Nov 3, 2026 9:20pm UTC

Request Review

@greptile-apps

greptile-apps Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; no new actionable issues remain.

Summary

This PR returns more useful HTTP errors instead of generic 500 responses.

  • Safe reads return a retryable response when a database query fails transiently.
  • Malformed multipart bodies receive the routes' validation response.
  • File serving returns 404 for unknown keys and missing storage objects.
  • Logs export returns a validation response for invalid filters.
  • The integration catalog returns typed statuses for MCP server failures.
  • Workspace activity and member removal return orchestration refusals.
  • Malformed chat IDs are treated as not found before the database query.
  • Moving same-named files into one folder returns the existing conflict response.
  • Workflow edits count each declined operation once, even when it creates several skips.

Both earlier threads are addressed: the catalog covers SDK and transport failures, and deferred connections retain their originating operation.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Route throws an error] --> B{Client disconnected?}
  B -->|Yes| C[499 response]
  B -->|No| D{Known HTTP error?}
  D -->|Yes| E[Return its status]
  D -->|No| F{GET or HEAD with temporary database failure?}
  F -->|Yes| G[503 with Retry-After]
  F -->|No| H[Existing fallback]
Loading

Reviews (3) · Last reviewed commit: "fix(uploads): leave an already-consumed ..." · Reviewed by Greptile

@cubic-dev-ai cubic-dev-ai 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.

All reported issues were addressed across 18 files

Reply with feedback, questions, or to request a fix.

Turn on auto-fix | Re-trigger cubic

Comment thread apps/sim/lib/mcp/utils.ts Resolved
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai 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.

No issues found across 18 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Turn on auto-fix | Re-trigger cubic

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai 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.

No issues found across 18 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Turn on auto-fix | Re-trigger cubic

@waleedlatif1
waleedlatif1 merged commit 6c4c49c into staging Nov 3, 2026
82 checks passed
@waleedlatif1
waleedlatif1 deleted the fix/route-5xx-classification branch November 3, 2026 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant