feat: parse bodies for the HTTP QUERY method - #177
Open
WolfieLeader wants to merge 4 commits into
Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe default Mergeability Score: ⚪ Minimal · up to The PR adds QUERY to the methods whose request bodies are parsed and updates the related documentation; no actionable merge-blocking risk remains based on the supplied evidence. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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. Comment |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR extends the body parser to treat the new HTTP QUERY method as a parsed method by default, updating implementation defaults, type documentation, and README, and adds a test to assert JSON body parsing for QUERY requests. Sequence diagram for JSON body parsing on HTTP QUERY requestssequenceDiagram
actor Client
participant NodeServer
participant bodyParserWrapper
participant AppMiddleware
Client->>NodeServer: HTTP QUERY with JSON body
NodeServer->>bodyParserWrapper: bodyParserWrapper(opts)
bodyParserWrapper->>bodyParserWrapper: check parsedMethods includes QUERY
bodyParserWrapper->>NodeServer: set request.body
NodeServer->>AppMiddleware: app.use(ctx)
AppMiddleware->>AppMiddleware: read ctx.request.body
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The default parsed methods list is now defined in multiple places (implementation, types JSDoc, README); consider centralizing this into a single constant to avoid future inconsistencies.
- Given QUERY is now a default parsed method, it may be useful to add a test that explicitly overrides
parsedMethodsto exclude QUERY to ensure the opt-out behavior continues to work as expected.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The default parsed methods list is now defined in multiple places (implementation, types JSDoc, README); consider centralizing this into a single constant to avoid future inconsistencies.
- Given QUERY is now a default parsed method, it may be useful to add a test that explicitly overrides `parsedMethods` to exclude QUERY to ensure the opt-out behavior continues to work as expected.
## Individual Comments
### Comment 1
<location path="README.md" line_range="93" />
<code_context>
- **enableRawChecking**: support the already parsed body on the raw request by override and prioritize the parsed value over the sended payload. (default is `false`)
-- **parsedMethods**: declares the HTTP methods where bodies will be parsed, default `['POST', 'PUT', 'PATCH']`.
</code_context>
<issue_to_address>
**issue (typo):** Use correct past participle "sent" instead of "sended".
Please change "sended payload" to "sent payload" for grammatical correctness and readability.
```suggestion
- **enableRawChecking**: support the already parsed body on the raw request by override and prioritize the parsed value over the sent payload. (default is `false`)
```
</issue_to_address>
### Comment 2
<location path="README.md" line_range="97" />
<code_context>
-- **parsedMethods**: declares the HTTP methods where bodies will be parsed, default `['POST', 'PUT', 'PATCH']`.
+- **parsedMethods**: declares the HTTP methods where bodies will be parsed, default `['POST', 'PUT', 'PATCH', 'QUERY']`.
- **disableBodyParser**: you can dynamic disable body parser by set `ctx.disableBodyParser = true`.
</code_context>
<issue_to_address>
**suggestion (typo):** Improve grammar in the description of disabling the body parser.
Consider rephrasing to: "you can dynamically disable the body parser by setting `ctx.disableBodyParser = true`."
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Author
|
Fixed those |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
New QUERY method
Added the new (June 2026) QUERY method
Checklist
Summary by Sourcery
Support parsing request bodies for the HTTP QUERY method alongside existing methods.
New Features:
Documentation:
Tests:
Summary by CodeRabbit
New Features
QUERYrequests, alongsidePOST,PUT, andPATCH.QUERYparsing can be excluded by customizing the configured parsed methods.Documentation
QUERYin the default parsed methods and clarified raw-checking behavior.Tests
QUERYrequest parsing.