Conversation
|
Updated 5:48 PM PT - Mar 20th, 2025
❌ @DonIsaac, your commit f208be2 has some failures in 🧪 try this PR locally: bunx bun-pr 18340 |
| // { | ||
| // code: 'ERR_METHOD_NOT_IMPLEMENTED', | ||
| // name: 'Error', | ||
| // message: 'The _implicitHeader() method is not implemented' |
There was a problem hiding this comment.
Not sure if node intentionally does not implement _implicitHeader or if they just... didn't implement it
There was a problem hiding this comment.
Please avoid modifying Node.js tests unnecessarily.
DonIsaac
requested review from
a team and
paperclover
and removed request for
a team
| } | ||
|
|
||
| // this[kUniqueHeaders] = parseUniqueHeadersOption(options.uniqueHeaders); | ||
| this[kUniqueHeaders] = new Set(); |
There was a problem hiding this comment.
Can we make this a getter so it's lazy?
There was a problem hiding this comment.
or if it's unused, then can we avoid the work?
| } | ||
|
|
||
| const validateHeaderName = (name, label?) => { | ||
| function validateHeaderName(name: unknown, label?: string): asserts name is string { |
There was a problem hiding this comment.
we should move this whole function to native code
There was a problem hiding this comment.
it likely already is being done
| throw $ERR_HTTP_HEADERS_SENT("set"); | ||
| } | ||
| validateHeaderName(name); | ||
| validateHeaderValue(name, value); |
There was a problem hiding this comment.
yeah, setHeader should just do all of these validations including probably creating the header too.
|
|
||
| addTrailers(headers) { | ||
| throw new Error("not implemented"); | ||
| this._trailer = ""; |
There was a problem hiding this comment.
I suspect we don't actually support trailers in the HTTP server.
|
superseded by #18482 |
3 participants
What does this PR do?
Gets
parallel/test-http-outgoing-proto.jspassing.How did you verify your code works?
There's a test