Skip to content

fix(table): return 400 instead of 500 for malformed sort/filter input#4425

Merged
waleedlatif1 merged 2 commits intostagingfrom
waleedlatif1/bandung-v4
May 4, 2026
Merged

fix(table): return 400 instead of 500 for malformed sort/filter input#4425
waleedlatif1 merged 2 commits intostagingfrom
waleedlatif1/bandung-v4

Conversation

@waleedlatif1
Copy link
Copy Markdown
Collaborator

Summary

  • Introduce TableQueryValidationError in lib/table/sql.ts and throw it from buildSortClause/buildFilterClause for caller-supplied bad input (invalid sort direction, invalid field name, invalid operator).
  • Catch it in v0 and v1 /tables/[id]/rows GET/PUT/DELETE handlers and return 400 with the original message instead of the generic 500.
  • Fixes case where ?sort={"column":"date","direction":"desc"} (wrong shape) silently 500'd; now returns a clear 400 the caller can react to.

Type of Change

  • Bug fix

Testing

Tested manually. All 38 existing lib/table/__tests__/sql.test.ts tests pass (error messages preserved). bun run check:api-validation passes.

Checklist

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

@vercel
Copy link
Copy Markdown

vercel Bot commented May 4, 2026

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped May 4, 2026 7:05am

Request Review

@cursor
Copy link
Copy Markdown

cursor Bot commented May 4, 2026

PR Summary

Low Risk
Low risk: this is an error-handling change that reclassifies certain malformed filter/sort inputs from 500 to 400 with clearer messages; main impact is client-visible status/message changes.

Overview
Fixes row query endpoints to treat malformed caller-supplied filter/sort as client errors instead of server errors.

Introduces TableQueryValidationError in lib/table/sql.ts and throws it from buildFilterClause/buildSortClause (invalid field names, operators, or sort directions), then updates both v0 and v1 table rows GET/PUT/DELETE routes to catch it and return HTTP 400 with the original message (rather than a generic 500).

Reviewed by Cursor Bugbot for commit 4306ac7. Configure here.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 4, 2026

Greptile Summary

This PR introduces TableQueryValidationError in lib/table/sql.ts and updates all validation throw-sites (validateFieldName, validateOperator, buildSortClause) to use it, then catches it in all six GET/PUT/DELETE handlers across the v0 and v1 table-rows routes, returning HTTP 400 with the original message instead of a generic 500. The programmer-error default branch in buildFieldCondition correctly retains a plain Error (→ 500), and all JSDoc @throws tags were updated to reference the new subclass.

Confidence Score: 5/5

Safe to merge — targeted, well-scoped fix with no logic regressions or security concerns.

All changed paths are confined to error-path handling. The new error class is minimal and correctly subclasses Error. The programmer-error default case is intentionally kept as a plain Error (→ 500). All six route-handler catch blocks are consistently updated. No P1/P0 findings.

No files require special attention.

Important Files Changed

Filename Overview
apps/sim/lib/table/sql.ts Adds TableQueryValidationError class and replaces all caller-input throw sites; programmer-error default retained as plain Error; JSDoc updated correctly.
apps/sim/app/api/table/[tableId]/rows/route.ts Imports TableQueryValidationError and adds instanceof checks in GET, PUT, DELETE catch blocks to return 400 instead of 500.
apps/sim/app/api/v1/tables/[tableId]/rows/route.ts Same pattern as v0 route — imports TableQueryValidationError and adds instanceof checks across GET, PUT, DELETE handlers.

Sequence Diagram

sequenceDiagram
    participant Client
    participant RouteHandler as Route Handler (v0/v1)
    participant Service as lib/table/service.ts
    participant SQL as lib/table/sql.ts

    Client->>RouteHandler: GET/PUT/DELETE ?sort=...&filter=...
    RouteHandler->>Service: queryRows / bulkUpdate / bulkDelete
    Service->>SQL: buildSortClause / buildFilterClause
    alt Invalid field / direction / operator (caller error)
        SQL-->>Service: throw TableQueryValidationError
        Service-->>RouteHandler: propagates
        RouteHandler-->>Client: 400 { error: message }
    else Programmer error (switch/ALLOWED_OPERATORS drift)
        SQL-->>Service: throw Error
        Service-->>RouteHandler: propagates
        RouteHandler-->>Client: 500 { error: "Failed to query rows" }
    else Valid input
        SQL-->>Service: SQL clause
        Service-->>RouteHandler: rows / result
        RouteHandler-->>Client: 200 success
    end
Loading

Reviews (2): Last reviewed commit: "fix(table): revert default-case throw to..." | Re-trigger Greptile

Comment thread apps/sim/lib/table/sql.ts Outdated
@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

Thanks for the review. Both findings addressed in 4306ac7:

  1. Reverted the default-case throw in buildFieldCondition back to plain Error — that's programmer-error territory (switch / ALLOWED_OPERATORS drift), correctly maps to 500.
  2. Updated @throws JSDoc tags on buildFilterClause, buildSortClause, validateFieldName, validateOperator, and buildFieldCondition to reference TableQueryValidationError.

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 4306ac7. Configure here.

@waleedlatif1 waleedlatif1 merged commit 4c86122 into staging May 4, 2026
14 checks passed
@waleedlatif1 waleedlatif1 deleted the waleedlatif1/bandung-v4 branch May 4, 2026 07:11
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