Add streaming benchmark scenarios#66
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
uWestJS Benchmark Results
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@benchmarks/scenarios/streaming-upload.js`:
- Around line 26-47: Fastify is rejecting 4MB uploads due to the default 1MB
bodyLimit, so when initializing the Fastify instance (the fastify(...) call that
creates `app`) set the bodyLimit to at least 4 * 1024 * 1024 (or larger) so the
POST /hash-body route and the createHashFromRequest streaming handler can
receive the full payload; update the Fastify server options accordingly to match
the benchmark payload size.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6ab4e759-e547-4985-926c-0305f7b2758a
📒 Files selected for processing (4)
benchmarks/scenarios/streaming-upload.jsbenchmarks/scenarios/streaming-with-content-length.jsbenchmarks/scenarios/streaming-without-content-length.jsbenchmarks/wrk-scripts/post-hash-body.lua
| if (framework === 'fastify') { | ||
| app.post('/hash-body', (req, reply) => { | ||
| createHashFromRequest(req.raw, (digest, error) => { | ||
| if (error) { | ||
| reply.code(500).send({ error: error.message }); | ||
| return; | ||
| } | ||
| reply.type('text/plain').send(digest); | ||
| }); | ||
| }); | ||
| } else { | ||
| // Express and uWestJS use same API | ||
| app.post('/hash-body', (req, res) => { | ||
| createHashFromRequest(req, (digest, error) => { | ||
| if (error) { | ||
| res.status(500).type('text/plain').send('Error processing upload'); | ||
| return; | ||
| } | ||
| res.type('text/plain').send(digest); | ||
| }); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check benchmark and server setup for Fastify request-body parsing / limits.
rg -n "addContentTypeParser|bodyLimit|application/octet-stream|hash-body" benchmarks src -C 3Repository: FOSSFORGE/uWestJS
Length of output: 14956
🏁 Script executed:
# Find the benchmark harness entry point and setup files
find benchmarks -name "*.js" -type f | head -20Repository: FOSSFORGE/uWestJS
Length of output: 745
🏁 Script executed:
# Look for the main benchmark file that initializes the app
rg -n "framework|fastify|express|uwestjs" benchmarks --type js -l | head -10Repository: FOSSFORGE/uWestJS
Length of output: 411
🏁 Script executed:
# Check if there's a main/index file or setup file in benchmarks root
ls -la benchmarks/Repository: FOSSFORGE/uWestJS
Length of output: 1068
🏁 Script executed:
cat -n benchmarks/server.jsRepository: FOSSFORGE/uWestJS
Length of output: 4628
🏁 Script executed:
cat -n benchmarks/test-setup.jsRepository: FOSSFORGE/uWestJS
Length of output: 3579
🏁 Script executed:
rg -n "bodyLimit|bodysize" --type json benchmarks/package.jsonRepository: FOSSFORGE/uWestJS
Length of output: 43
🌐 Web query:
Fastify default bodyLimit value
💡 Result:
The default value of bodyLimit in Fastify is 1048576 bytes (1 MiB).
Citations:
- 1: https://fastify.io/docs/latest/Reference/Server/
- 2: https://fastify.io/docs/latest/Reference/Routes/
- 3: fastify/fastify@b2f30d8
- 4: Add option to limit the size of request bodies for the default body parser fastify/fastify#627
Fastify benchmark fails: 4 MB request exceeds default 1 MB bodyLimit.
The Fastify server is initialized without a bodyLimit option (server.js:43), defaulting to 1 MB. The wrk script sends a 4 MB application/octet-stream body, so Fastify will reject the request before the streaming handler runs. Configure Fastify with bodyLimit: 4 * 1024 * 1024 or larger to match the benchmark payload size and test actual streaming performance instead of request rejection.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@benchmarks/scenarios/streaming-upload.js` around lines 26 - 47, Fastify is
rejecting 4MB uploads due to the default 1MB bodyLimit, so when initializing the
Fastify instance (the fastify(...) call that creates `app`) set the bodyLimit to
at least 4 * 1024 * 1024 (or larger) so the POST /hash-body route and the
createHashFromRequest streaming handler can receive the full payload; update the
Fastify server options accordingly to match the benchmark payload size.
There was a problem hiding this comment.
the scenario uses streaming (req.raw for Fastify), so it shouldn't be buffering the body. if there's is a problem suggest the changes
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if there's an existing content type parser registration in server.js or test-setup.js
rg -n "addContentTypeParser|octet-stream" benchmarks/
Length of output: 852
@VikramAditya33, you're right that bodyLimit is not the problem here — Fastify only enforces the body limit when using the parseAs: 'string'|'buffer' option; if the limit is exceeded, the custom parser will not be invoked. Without parseAs, Fastify does not buffer the stream and the limit is irrelevant. So streaming via req.raw is safe from the bodyLimit concern.
However, there is still a real issue: no content type parser is registered for application/octet-stream, and without one, Fastify will reject the POST request with a 415 Unsupported Media Type error before the route handler ever runs.
The official Fastify pattern for raw body streaming is to register a no-op content type parser that simply calls done(), then access request.raw directly in the handler for piping/streaming.
Here is the fix — add a no-op addContentTypeParser call in the Fastify branch of setup:
if (framework === 'fastify') {
// Required: register a no-op parser so Fastify doesn't reject the
// application/octet-stream body with 415 before the handler runs.
// Using the streaming (non-parseAs) form means bodyLimit does NOT apply.
app.addContentTypeParser('application/octet-stream', function (request, payload, done) {
done();
});
app.post('/hash-body', (req, reply) => {
createHashFromRequest(req.raw, (digest, error) => {
if (error) {
reply.code(500).send({ error: error.message });
return;
}
reply.type('text/plain').send(digest);
});
});
}This tells Fastify to accept application/octet-stream without buffering anything, leaving the raw Node.js stream untouched so createHashFromRequest(req.raw, ...) can consume it correctly.
🧠 Learnings used
Learnt from: VikramAditya33
Repo: FOSSFORGE/uWestJS PR: 60
File: src/http/handlers/compression/compression-handler.ts:334-338
Timestamp: 2026-04-28T19:19:30.852Z
Learning: In the uWestJS project (src/http/core/response.ts), `uwsRes.end(buffer)` (the native uWebSockets.js layer) automatically sets the `Content-Length` header based on the buffer size. Therefore, explicitly calling `res.setHeader('content-length', ...)` before `res.send(buffer)` results in a duplicate Content-Length header and triggers HPE_UNEXPECTED_CONTENT_LENGTH. The correct pattern is to let uWS handle Content-Length automatically via `end()`.
There was a problem hiding this comment.
okay makes sense
e2b8aef to
5641462
Compare
Add streaming benchmark scenarios
Adds comprehensive streaming benchmarks to measure performance of large data transfers.
Changes
New Scenarios:
streaming-with-content-length.js- 5MB response stream with Content-Length header (optimaltryEnd()path)streaming-without-content-length.js- 5MB response stream with chunked transfer encodingstreaming-upload.js- 4MB POST request body streaming with SHA-256 hash computationNew Assets:
wrk-scripts/post-hash-body.lua- Lua script for POST streaming benchmarkCloses #61
Summary by CodeRabbit