fix: type rawBody as optional to match runtime behavior - #178
Conversation
rawBody is only assigned once a body actually gets parsed. For unsupported content types, disabled parsers, or when a request has no body, it stays undefined - the existing tests already check for this case with toEqual(undefined). The type declarations on koa.Request and http.IncomingMessage still said plain string though, so anything using strict type checking has no way to know it might not be there.
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates both rawBody declarations to string | undefined, accurately reflecting that the property is only populated when a supported body is parsed while preserving the existing runtime behavior. File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe request type declarations now mark Merge Risk: ⚪ Minimal · up to This updates request typings so rawBody may be absent when no body is parsed, matching runtime behavior. No merge-blocking risk remains. 🚥 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 |
![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 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/body-parser.ts" line_range="13" />
<code_context>
// eslint-disable-next-line @typescript-eslint/consistent-type-definitions
interface Request {
body?: unknown;
- rawBody: string;
+ rawBody?: string;
}
</code_context>
<issue_to_address>
**issue (bug_risk):** With `exactOptionalPropertyTypes` enabled, `rawBody?: string` allows the property to be omitted but does not allow callers to explicitly assign `undefined`; this contradicts the intended `string | undefined` contract and the runtime behavior.
**Triggers:** When a TypeScript consumer uses `exactOptionalPropertyTypes` and clears or forwards `rawBody` by assigning `undefined`.
**Suggested fix:** Declare the properties as `rawBody: string | undefined` if explicit `undefined` assignments are part of the supported contract.
</issue_to_address>Sourcery assessment
Approval pending. 1 finding to address first.
Blocking findings: src/body-parser.ts:13
| // eslint-disable-next-line @typescript-eslint/consistent-type-definitions | ||
| interface Request { | ||
| body?: unknown; | ||
| rawBody: string; |
There was a problem hiding this comment.
issue (bug_risk): With exactOptionalPropertyTypes enabled, rawBody?: string allows the property to be omitted but does not allow callers to explicitly assign undefined; this contradicts the intended string | undefined contract and the runtime behavior.
Triggers: When a TypeScript consumer uses exactOptionalPropertyTypes and clears or forwards rawBody by assigning undefined.
Suggested fix: Declare the properties as rawBody: string | undefined if explicit undefined assignments are part of the supported contract.
1 participant
Closes #174
rawBodygets typed as a plainstringon bothkoa.Requestandhttp.IncomingMessage, but at runtime it's only set once a body actually gets parsed. If the content type isn't one of the supported ones, or the parser is disabled, or there's no body at all, it staysundefined. The test suite already checks for exactly that, there's a couple ofexpect(ctx.request.rawBody).toEqual(undefined)assertions in middleware.test.ts, so the runtime behavior clearly isn't in question here, just the type not reflecting it.Widened both declarations to
string | undefined. Small change, nothing else touched.Ran the full test suite and
xolint locally, both clean, plustsc --noEmitto confirm the type change compiles fine on its own.Summary by Sourcery
Bug Fixes:
rawBodyby typing it as optional on Koa requests and incoming HTTP messages.Summary by CodeRabbit