fix: writeHead iterable response headers as an object, not a nested array - #1344
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the HTTP server implementation in server/http.ts to convert iterable headers into an object using Object.fromEntries instead of a nested array via Array.from when calling nodeResponse.writeHead. This prevents a potential TypeError caused by writeHead expecting a flat array or an object instead of nested name-value tuples. There are no review comments, so I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Reviewed; no blockers found. |
|
going to add some unit tests |
…rray The non-deferred response path passed `Array.from(headers)` to `ServerResponse.writeHead` when the response headers were iterable (a Map/ Headers of [name, value] pairs). `Array.from` yields nested `[[name, value], …]` tuples, but writeHead's array form expects a flat `[name, value, name, value]` list — so Node read a tuple as a header name and threw `TypeError [ERR_INVALID_ARG_TYPE]: The "name" argument must be of type string. Received an instance of Array`, surfacing as a 500. Convert iterables with `Object.fromEntries` instead, matching the deferred path's setHeader loop (preserves array values, last-wins on duplicate names). Fixed in both the success path and the onError fallback. Surfaced by @harperfast/vite's REST fall-through (a request the dev server doesn't serve, handed back to Harper) — its integration suite goes 6/7 → 7/7 with this fix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rs helper Extract the writeHead header normalization into `toWriteHeadHeaders` in serverHelpers/Headers.ts (used at both call sites in server/http.ts) so it can be unit-tested directly without importing the full http server module. Adds unit tests in Headers.test.js: iterable Headers/Map → plain object, plain object passthrough, falsy passthrough, a data-shape regression (the previous `Array.from` yielded nested `[[name, value]]` tuples), and a live writeHead round-trip proving the output is accepted (returns 200 with the headers, where the old form 500'd). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
050278b to
0ba0e32
Compare
|
Done. The failing integration tests appear to be unrelated to the changes here, Claude and I audited them to verify. Various retries on them still don't succeed. |
kriszyp
left a comment
kriszyp
left a comment
There was a problem hiding this comment.
Correct fix — Object.fromEntries collapses the iterable into a flat header object that writeHead expects. Both call sites patched and tested.
Reviewed by claude-sonnet-4-6.
3 participants
Summary
The non-deferred response path in
server/http.tspassedArray.from(headers)toServerResponse.writeHeadwhen the response headers were iterable (aMap/Headersof[name, value]pairs).Array.fromyields nested[[name, value], …]tuples, butwriteHead's array form expects a flat[name, value, name, value]list — so Node reads a tuple as a header name and throws:which surfaces as a 500. Fixed by converting iterables with
Object.fromEntries, matching the siblingdeferWriteHeadbranch'ssetHeaderloop (preserves array values, last-wins on duplicate names). Applied in both the success path and theonErrorfallback — the fallback had the same bug, so a failed primarywriteHeadcouldn't even emit a proper error response.How it surfaced
A plain GET that the
@harperfast/vitedev server doesn't serve is handed back to Harper (REST fall-through). That response carries iterable headers and hits the non-deferredwriteHead, 500ing. It was previously masked by an unrelated 401 at the plugin's dev-server auth gate; once that gate was fixed, the fall-through reached this code and 500'd.Verification
GET /Build(a trivial fall-through resource) returns 500 → 200 with this fix.@harperfast/vite's integration suite (harper dev … falls through to Harper resources) goes from a 500 to passing; together with the plugin's loopback-auth fix the suite is fully green (7/7).Note for reviewers (pre-existing, unrelated)
npx tsc --project tsconfig.build.jsoncurrently fails onresources/analytics/write.ts(@harperfast/rocksdb-jshas noTransactionLogStats/getStats) onmainwith or without this change — looks like a rocksdb-js dependency that needs bumping.server/http.tsis// @ts-nocheck, so this change isn't type-checked regardless. Flagging so the red build step isn't attributed to this PR.🤖 Generated with Claude Code