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.

Sign up for GitHub

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

Jump to bottom

doc: fix improper http.get sample code in http.markdown #4263

Closed
wants to merge 1 commit into from
Closed

doc: fix improper http.get sample code in http.markdown #4263

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions doc/api/http.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -966,6 +966,8 @@ Example:

http.get("http://www.google.com/index.html", function(res) {
console.log("Got response: " + res.statusCode);
// consume response body
res.resume();
Copy link
Contributor

Choose a reason for hiding this comment

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

should this be above console.log as seen in @chrisdickinson's original example?

might make sense to have the response enter flowing mode before accessing it.

Copy link
Member

Choose a reason for hiding this comment

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

I don't think that's necessary, statusCode is a property that exists when the function is called and isn't attached to the streaming nature of res in any way (beyond this point at least). I'm more concerned with the lack lack of information on what res.resume() does but explaining it here, even "enter flowing mode", is going to detract from the point of the example and likely take up too much additional space.

Copy link
Contributor

Choose a reason for hiding this comment

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

So I dug a bit further into this. The response in the callback is an instance of http.IncomingMessage... and finding that out was not exactly obvious. http.IncomingMessage implements the Readable stream interface, which is where it inherits .resume from.

Quite a bit of digging is necessary to find out what this function does. While it may not make sense to duplicate documentation, perhaps we can find a better way to discover inherited properties

Any thoughts @nodejs/documentation?

If there is an interest in this I'll move it to a separate issue to avoid derailing this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Correct. IncomingMessage would probably better be HTTPMessage.

}).on('error', function(e) {
console.log("Got error: " + e.message);
});
Expand Down