nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
104.51k stars 28.19k forks source link

http module documented default behavior only triggered if undocumented option is set #53035

Open earslap opened 2 weeks ago

earslap commented 2 weeks ago

Affected URL(s)

https://nodejs.org/api/http.html#responsewritechunk-encoding-callback

Description of the problem

For the http module, the documentation for response.write says:

Writing to the body is not allowed when the request method or response status do not support content. If an attempt is made to write to the body for a HEAD request or as part of a 204 or 304response, a synchronous Error with the code ERR_HTTP_BODY_NOT_ALLOWED is thrown.

However this is not the default behavior. By default, nodejs swallows the provided body and prints the debug message (enabled by NODE_DEBUG=http):

HTTP 77822: This type of response MUST NOT have a body. Ignoring write() calls.

The documented behavior (throwing ERR_HTTP_BODY_NOT_ALLOWED) happens only when rejectNonStandardBodyWrites option is set to true while creating a server with createServer. However this option is not documented, it does not exist in @node/types either.

RedYetiDev commented 2 weeks ago

Thanks! If you'd like, you can submit a PR updating the documentation, and the team will be more than happy to review :-)

earslap commented 2 weeks ago

I'd like to submit a PR but I'm not sure about which way to go about it. rejectNonStandardBodyWrites is not documented and is not part of the typescript types either. I don't know if that omission is intentional or not; I have not been able find any discussion about it.

Should I somehow add that to the documentation, and separately push it for @types/node and document the actual behavior? Or should I just remove the part from the documentation that says:

Writing to the body is not allowed when the request method or response status do not support content. If an attempt is made to write to the body for a HEAD request or as part of a 204 or 304response, a synchronous Error with the code ERR_HTTP_BODY_NOT_ALLOWED is thrown.

Not sure which way this should go right now.

RedYetiDev commented 2 weeks ago

@nodejs/http

ShogunPanda commented 2 weeks ago

@earslap The omission is not intentional. The author of the PR was supposed to sent a fix-up PR for docs (and types) but never did. See: https://github.com/nodejs/node/pull/47732#discussion_r1199839025

You are more than welcome to send your PR.