Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: DEP0066 is slightly incorrect #37641

Closed
simov opened this issue Mar 6, 2021 · 4 comments · Fixed by #37660
Closed

doc: DEP0066 is slightly incorrect #37641

simov opened this issue Mar 6, 2021 · 4 comments · Fixed by #37660
Labels
doc Issues and PRs related to the documentations. feature request Issues that request new features to be added to Node.js. good first issue Issues that are suitable for first-time contributors. http Issues or PRs related to the http subsystem.

Comments

@simov
Copy link
Contributor

simov commented Mar 6, 2021

📗 API Reference Docs Problem

  • Version: v15.11.0
  • Subsystem: http.ClientRequest

Location

Affected URL(s):

Description

DEP0066 implies that using OutgoingMessage.prototype.getHeaderNames() is equivalent to the now deprecated OutgoingMessage.prototype._headerNames property which is not the case.

OutgoingMessage.prototype._headerNames contains a mapping from lowercase to the exact header names that were sent with the request:

{authorization: "Authorization", host: "Host"}

Where OutgoingMessage.prototype.getHeaderNames() returns only lowercase names.

The now deprecated _headerNames property is useful in http debug logging modules that print the exact header names being sent, as some servers are still picky about those. Not having access to the actual header names will make debugging harder, assuming node still sends the headers as they are sent through the http.request method.

Besides the documentation being misleading about this I'd like to know if there is a way to access the headers being sent going forward, since _headerNames is now deprecated.

@simov simov added the doc Issues and PRs related to the documentations. label Mar 6, 2021
@benjamingr benjamingr added feature request Issues that request new features to be added to Node.js. http Issues or PRs related to the http subsystem. labels Mar 6, 2021
@benjamingr
Copy link
Member

@nodejs/http

@ronag
Copy link
Member

ronag commented Mar 6, 2021

What about rawHeaders?

@simov
Copy link
Contributor Author

simov commented Mar 6, 2021

What about rawHeaders?

The issue is with http.ClientRequest, rawHeaders seems to be present only on http.IncomingMessage.

@mcollina mcollina added the good first issue Issues that are suitable for first-time contributors. label Mar 8, 2021
@mcollina
Copy link
Member

mcollina commented Mar 8, 2021

I think when we deprecated _headerNames we forgot to add a response.getRawHeaderNames(), e.g. the name that will be sent on the wire. I think this will be simple to add, so I flagged it as "good first issue", if you'd like to give it a shot.

simov added a commit to simov/node that referenced this issue Mar 8, 2021
Trott pushed a commit to simov/node that referenced this issue Mar 20, 2021
Fixes: nodejs#37641

PR-URL: nodejs#37660
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@Trott Trott closed this as completed in 4686cb5 Mar 20, 2021
ruyadorno pushed a commit that referenced this issue Mar 24, 2021
Fixes: #37641

PR-URL: #37660
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
targos pushed a commit that referenced this issue May 1, 2021
Fixes: #37641

PR-URL: #37660
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. feature request Issues that request new features to be added to Node.js. good first issue Issues that are suitable for first-time contributors. http Issues or PRs related to the http subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants