Skip to content

fix(ext/node): throw ERR_INVALID_ARG_VALUE for odd-length headers array in http.ServerResp - #33820

Merged
littledivy merged 1 commit into
denoland:mainfrom
divybot:claude/test-http-write-head
May 4, 2026
Merged

littledivy merged 1 commit into
denoland:mainfrom
divybot:claude/test-http-write-head

Conversation

@divybot

@divybot divybot commented May 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Enables test-http-write-head in node_compat suite.

Test plan

  • cargo test --test node_compat -- test-http-write-head

…ay in http.ServerResp

Enables tests/node_compat/runner/suite/test/parallel/test-http-write-head.js

Co-authored-by: Divy Srivastava <[email protected]>

@fibibot fibibot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Tiny semantic fix — odd-length headers array passed to ServerResponse.writeHead() now throws ERR_INVALID_ARG_VALUE instead of ERR_INVALID_ARG_TYPE, matching Node's behavior.

What I verified

Error class change (ext/node/polyfills/_http_server.js:417): ERR_INVALID_ARG_TYPE("headers", "Array with even length", obj) → ERR_INVALID_ARG_VALUE("headers", obj). The semantic distinction is correct here — the argument IS an array (right type), but the value is malformed (odd length). Node's convention reserves ERR_INVALID_ARG_TYPE for cross-type mismatches and uses ERR_INVALID_ARG_VALUE for in-type malformations. ✓

Upstream test alignment (tests/node_compat/runner/suite/test/parallel/test-http-write-head.js): the upstream test asserts:

assert.throws(() => {
  res.writeHead(200, ['invalid', 'headers', 'args']);  // 3 elements, odd
}, { code: 'ERR_INVALID_ARG_VALUE' });

This PR's emission matches exactly. ✓

Test enablement (tests/node_compat/config.jsonc:1644): parallel/test-http-write-head.js added in alphabetical order between test-http-write-head-after-set-header.js and test-http-zero-length-write.js. ✓

CI

All test node_compat shards (debug + release × all 6 platforms × 3 shards = 36 jobs) pass. The newly-enabled test-http-write-head.js would land in one of the shards on every platform — all green confirms the change works end-to-end.

91 success, 8 in-flight (release linux-x86_64 integration + specs + unit + wpt + test specs debug windows-aarch64), 1 skipped, 0 fail. The remaining shards aren't in the _http_server.js blast radius.

@littledivy
littledivy merged commit a34906b into denoland:main May 4, 2026
136 checks passed
littledivy added a commit to crowlKats/deno that referenced this pull request Jun 10, 2026
…ay in http.ServerResp (denoland#33820)

## Summary

Enables `test-http-write-head` in node_compat suite.

## Test plan
- [x] `cargo test --test node_compat -- test-http-write-head`

Co-authored-by: divybot <[email protected]>
Co-authored-by: Divy Srivastava <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants