Rewrite the Benchmarker example on collectBody and microcaching - #1321
Dev-next-gen wants to merge 2 commits into
Conversation
The readJson helper responded from its onAborted handler, which throws once the response is aborted, and let JSON.parse throw on an invalid body. Either one took the whole server down. Mirror JsonPost.mjs: catch the parse error, close the response, and do not touch res from the abort path.
|
The AppVeyor log is clear enough to pin down the red check here: it fails in the Windows native build, before anything reaches This PR only touches Found by a defect-hunting pipeline I build and run (Dev-next-gen), using Claude Code with Anthropic's Claude Opus 5. |
|
The benchmarker example is really just a silly relic from some third party benhmark, and the whole thing needs a rewrite to use Microcaching and onDataV2 / collectBody if anything |
The JSON route now uses res.collectBody instead of hand rolling the onData accumulation: one callback with the whole body, or null once it passes a 1 MB cap, so the example no longer buffers without a bound. The abort handler stays empty, since res must not be touched once it fires. Add a /db route as the benchmark's database hit, served through the microcaching overload of get(). A cached response only carries end, cork and onAborted, so it writes no content-type. The example also imports ../dist/uws.js like the other 22 examples, so it runs from a checkout.
|
You're right that it was a third-party relic; the readJson helper in it was just a copy of the JsonPost one. I rewrote the whole example along the lines you gave and pushed it as c813e73: the JSON route goes through res.collectBody with a 1 MB cap instead of accumulating onData chunks by hand, and there is now a /db route served through the microcaching overload of get(), standing in for the benchmark's database hit. Measured against the prebuilt binaries of the binaries branch (source_commit 5bb09ae, Node 22.23.2, Linux x64). / and /id/42?name=foo come back byte for byte what they did before, and POST /json still echoes {"a":1} as well as a 300 KB body split over several chunks (307211 bytes out). A body that is not JSON, and a socket destroyed after five body bytes, both ended the process with code 1 before and now leave the server answering. Two behaviours did change: a 2 MB post used to be buffered whole and echoed with a 200, it now hits the cap and the connection is closed; and /db writes no content-type, because a cached response carries only end, cork and onAborted, res.writeHeader on it being "not a function". For the cache window, 20 back-to-back requests to /db returned one single payload, still the same one 4.2 s later, and a different one at 10.2 s. The shape I gave /db ({id, randomNumber}) is my guess at what the original benchmark's database route returned, and the 1 MB cap is a round number rather than a considered one; both are a one-line change if you had something else in mind. Found by a defect-hunting pipeline I build and run (Dev-next-gen), using Claude Code with Anthropic's Claude Opus 5. |
Dev-next-gen
changed the title
2 participants
I was reading the examples next to JsonPost.mjs and noticed that the
readJsonhelper in examples/Benchmarker.mjs was a trimmed copy of the one in JsonPost.mjs that had lost its error handling. Two things went wrong, and each one took the whole server down: the error callback was registered withres.onAborted(err)and that callback didres.end('Ok'), so a client dropping the connection mid-POST threw "uWS.HttpResponse must not be accessed after uWS.HttpResponse.onAborted callback"; andJSON.parsewas called without a try/catch, so a POST to/jsonwith a body that is not JSON threw a SyntaxError out ofonData.The first commit mirrored what JsonPost.mjs does. After @uNetworkingAB said the example is a relic of some third party benchmark and that the whole thing needs a rewrite to use Microcaching and onDataV2 / collectBody, the second commit does that rewrite:
/jsongoes throughres.collectBodywith a 1 MB cap instead of hand rolling theonDataaccumulation, a/dbroute stands in for the benchmark's database hit and is served through the microcaching overload ofget(), and the file imports../dist/uws.jslike the other 22 examples so it runs from a checkout.Measured with the prebuilt binaries of the
binariesbranch (source_commit 5bb09ae), Node 22.23.2 on Linux x64, one fresh server process per case:GET /HiGET /id/42?name=foo42 foo(chunked)POST /jsonwith{"a":1}{"a":1}POST /jsonwith a 300 KB body (several chunks)POST /jsonwith bodynot json!GET /POST /json, Content-Length 100, socket destroyed after 5 body bytesGET /POST /jsonwith a 2 MB bodyGET /dbA cached response carries only
end,corkandonAborted, so/dbwrites no content-type:res.writeHeaderon it is "not a function". The shape of/db({id, randomNumber}) is a guess at the original benchmark's database route, and the 1 MB cap is a round number.Found by a defect-hunting pipeline I build and run (Dev-next-gen), using Claude Code with Anthropic's Claude Opus 5.