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

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

Closed
earslap opened this issue May 17, 2024 · 4 comments · Fixed by #53396
Closed

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

earslap opened this issue May 17, 2024 · 4 comments · Fixed by #53396
Labels
doc Issues and PRs related to the documentations. http Issues or PRs related to the http subsystem.

Comments

@earslap
Copy link

earslap commented May 17, 2024

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.

@earslap earslap added the doc Issues and PRs related to the documentations. label May 17, 2024
@RedYetiDev
Copy link
Member

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

@earslap
Copy link
Author

earslap commented May 17, 2024

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
Copy link
Member

@nodejs/http

@ShogunPanda
Copy link
Contributor

@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: #47732 (comment)

You are more than welcome to send your PR.

@VoltrexKeyva VoltrexKeyva added the http Issues or PRs related to the http subsystem. label May 18, 2024
nodejs-github-bot pushed a commit that referenced this issue Jun 11, 2024
PR-URL: #53396
Fixes: #53035
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Ulises Gascón <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
targos pushed a commit that referenced this issue Jun 20, 2024
PR-URL: #53396
Fixes: #53035
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Ulises Gascón <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
EliphazBouye pushed a commit to EliphazBouye/node that referenced this issue Jun 20, 2024
PR-URL: nodejs#53396
Fixes: nodejs#53035
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Ulises Gascón <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
bmeck pushed a commit to bmeck/node that referenced this issue Jun 22, 2024
PR-URL: nodejs#53396
Fixes: nodejs#53035
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Ulises Gascón <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
marco-ippolito pushed a commit that referenced this issue Jul 19, 2024
PR-URL: #53396
Fixes: #53035
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Ulises Gascón <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
marco-ippolito pushed a commit that referenced this issue Jul 19, 2024
PR-URL: #53396
Fixes: #53035
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Ulises Gascón <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Matteo Collina <[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. http Issues or PRs related to the http subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants