Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe response Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to No concrete merge-blocking regression remains; the response-length changes are covered by targeted tests. 🚥 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 GuideCorrects response length detection by leaving Web ReadableStream and Response bodies unknown, using Blob.size when applicable, and preserving explicit Content-Length values; regression tests cover the length getter and HEAD response behavior. Flow diagram for response body length detectionflowchart TD
A[Response body] --> B{Explicit Content-Length?}
B -->|Yes| C[Use explicit length]
B -->|No| D{Body type}
D -->|ReadableStream or Response| E[Leave length undefined]
D -->|Blob| F[Use Blob.size]
D -->|String| G[Use Buffer.byteLength]
D -->|Buffer| H[Use buffer length]
D -->|Other value| I[Use JSON byte length]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2010 +/- ##
=======================================
Coverage 99.90% 99.90%
=======================================
Files 9 9
Lines 2127 2128 +1
=======================================
+ Hits 2125 2126 +1
Misses 2 2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@jakezwang this is a good PR. Thank you! I want to hold off on approving for a bit because even though this fix is correct, it reads like it could lead to a breaking change for products that have already configured workarounds for the content length being 2 vs. unknown—which you are right to address. If I'm incorrect in this concern, I'm all ears! @sourcery-ai @coderabbitai can you take a first-pass on my concern? If other teammates approve because I'm overthinking, I'm all for that as well! I will merge this PR for sure, moving towards v4. Thank you! 🥇 |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '--- PR diff ---'
git diff --no-ext-diff HEAD^ HEAD -- lib/response.js __tests__/response/length.test.js __tests__/application/respond.test.js
echo
echo '--- response.js outline ---'
ast-grep outline lib/response.js --items all --view expanded
echo
echo '--- relevant response implementation ---'
rg -n -C 12 'get length|set length|Content-Length|respond|ReadableStream|Response|Blob' lib/response.js lib/application.js
echo
echo '--- HEAD and Response test contexts ---'
rg -n -C 16 'when HEAD is used|\.body is a Response|ReadableStream|Response\(' __tests__/application/respond.test.js __tests__/response/length.test.js
echo
echo '--- package version ---'
rg -n -C 2 '"version"' package.jsonLength of output: 42134
This PR changes an observable HEAD response behavior for Before this PR, Koa calculated Content-Length: 2That value did not describe the body. It was an implementation artifact. After this PR, Koa leaves the length unknown and does not send The change does not affect these cases:
A product that specifically detects or compensates for I recommend merging with a release note that states: HEAD responses for Web You are interacting with an AI system. |
response.lengthcurrently treats WebReadableStreamandResponsebodies as JSON and returns2. This also makes HEAD responses sendContent-Length: 2when the actual body length is unknown.Leave those lengths undefined, and use
Blob.sizewhen a Blob has no Content-Length header. Explicit Content-Length values still take precedence. Tests cover the getter, HEAD responses, and preserving headers from a Response.Validation: lint, build, commitlint, and all 464 tests with coverage on Node 22, 24, and 26.
Checklist
Summary by Sourcery
Handle Web response body lengths correctly without inventing Content-Length values for bodies whose size is unknown.
Bug Fixes:
Tests:
Summary by CodeRabbit
ReadableStreamandResponsebodies when noContent-Lengthis available.