fix: request header is too large

[shreeharsha-factly]

Summary by CodeRabbit

• API Updates
  • Changed table data retrieval endpoint from GET to POST, with query parameters (columns, sort, limit, page, filter) now sent via JSON request body.
  • Updated web interface code snippet generation to reflect POST requests with JSON payloads for curl, PowerShell, and fetch examples.

[claude]

Claude finished @shreeharsha-factly's task —— View job

---

PR Review: fix: request header is too large

I've completed my analysis of this PR. Here's my comprehensive review:

Progress

• Read and analyze all changed files
• Review backend changes (Go files)
• Review frontend changes (TypeScript/React files)
• Evaluate code quality and best practices
• Check for potential bugs and security issues
• Assess performance considerations
• Provide comprehensive feedback

---

📋 Summary

This PR addresses "request header is too large" errors by converting the REST API endpoint from GET with query parameters to POST with JSON body. The changes affect both backend (Go) and frontend (TypeScript/React) code to maintain consistency.

🔍 Detailed Review

Backend Changes (Go)

server/domain/models/utils.go - utils.go:3

• ✅ Good: Added RestRequest struct with proper JSON tags and omitempty for optional fields
• ✅ Good: Clean struct definition with appropriate data types
• 🔄 Minor: Consider adding validation tags (e.g., validate:"min=1" for Limit/Page)

server/interfaces/http/routes/api/rest.go - rest.go:24

• ✅ Good: Proper error handling for JSON body parsing with appropriate HTTP status codes
• ✅ Good: Maintains backward compatibility by handling empty request bodies
• ✅ Good: Updated OpenAPI documentation to reflect POST method and body parameters
• ⚠️ Security: No request body size validation - large payloads could cause memory issues
• 🔄 Improvement: Consider adding request timeout and body size limits

server/interfaces/http/routes/api/routes.go - routes.go:28

• ✅ Good: Consistent route changes in both public and internal routes
• ✅ Good: Simple and clean HTTP method update

Frontend Changes (TypeScript/React)

web/src/lib/queries/dataset/get-table.ts - get-table.ts:28

• ✅ Good: Consistent API client method change from GET to POST
• ✅ Good: Proper JSON payload structure with json property
• ✅ Good: Maintains same function signature for consumers

web/src/app/projects/[projectId]/datasets/[datasetId]/api/page.tsx - page.tsx:91

• ✅ Good: Updated all code generation examples (curl, PowerShell, fetch) to use POST
• ✅ Good: Proper JSON formatting with indentation for better readability
• ✅ Good: Visual improvements showing POST method and request body
• 🔄 Minor: The JSON.stringify(${requestBody}) in fetch examples creates double-stringified JSON

🐛 Potential Issues

1. Double JSON encoding in page.tsx:129,138:

   body: JSON.stringify(${requestBody}) // requestBody is already stringified

   Should be:

   body: ${requestBody}

2. Missing request validation: No limits on request body size or complexity

3. Error handling: Current implementation doesn't validate JSON structure beyond basic parsing

🚀 Performance Considerations

• ✅ Positive: Moving from URL parameters to POST body eliminates URL length limitations
• ✅ Positive: JSON body parsing is more efficient than query parameter parsing for complex data
• ⚠️ Concern: No body size limits could allow very large requests

🔒 Security Assessment

• ✅ Good: Maintains existing authorization middleware
• ✅ Good: Proper error handling without exposing sensitive information
• ⚠️ Missing: Request body size validation to prevent DoS attacks
• ⚠️ Missing: JSON schema validation for stricter input validation

🧪 Test Coverage

• ❓ Unknown: No tests visible in this PR - existing tests may need updates to use POST instead of GET
• 🔄 Recommendation: Verify that API tests are updated to match the new endpoint behavior

📈 Overall Assessment

This is a well-implemented solution to the "request header too large" problem. The changes are:

• Consistent across frontend and backend
• Maintain backward compatibility
• Include proper documentation updates
• Follow established patterns in the codebase

🎯 Recommendations

1. Fix double JSON encoding in frontend fetch examples
2. Add request body size limits (e.g., 1MB) in the backend
3. Verify test updates for the changed HTTP method
4. Consider adding JSON schema validation for better error messages

The core fix is solid and addresses the stated problem effectively! 🎉

---

[coderabbitai]

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

The pull request converts the /tables/:tableName API endpoint from GET with URL query parameters to POST with a JSON body. Changes include adding a new RestRequest struct, updating the backend handler to parse JSON request bodies, changing route registration from GET to POST, and updating frontend client code to construct POST requests with JSON payloads instead of query strings.

Changes

Cohort / File(s)	Summary
API Model Definition server/domain/models/utils.go	Introduced RestRequest struct with JSON-tagged fields (Columns, Sort, Limit, Page, Filter) marked omitempty for conditional serialization.
Backend Handler & Routing server/interfaces/http/routes/api/rest.go, server/interfaces/http/routes/api/routes.go	Updated REST handler to parse RestRequest from POST body with error handling, derive default values (Columns defaults to ["*"], Page defaults to 1), and map fields to RestParams. Changed route method from GET to POST.
Frontend Client & UI web/src/app/projects/[projectId]/datasets/[datasetId]/api/page.tsx, web/src/lib/queries/dataset/get-table.ts	Changed API requests from GET with URL query parameters to POST with JSON body. Updated request payload to send columns as array and filter as nested object. Regenerated code snippets (curl, PowerShell, fetch) to use POST method with JSON bodies and updated UI to display POST label and request payload.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• feat: add source query param to fetch summary #3: Modifies the same server/interfaces/http/routes/api/rest.go handler to swap the service call from driverSvc to olapSvc, directly overlapping with this PR's handler changes.

Poem

🐰 From query strings to JSON we hop,
Parameters now in the body do drop,
GET becomes POST, a protocol switch,
The frontend and backend together enrich! ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title 'fix: request header is too large' is misleading. The PR actually changes the API from GET with query parameters to POST with a JSON body, which is an architectural change, not a fix for oversized headers.	Update the title to accurately reflect the primary change, such as 'refactor: change API endpoint from GET query params to POST with JSON body' or 'feat: convert REST endpoint to accept JSON POST body instead of query parameters'.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/request-headers

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}