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

http - http.ClientRequest doesn't emit 'aborted'? #15259

Closed
ronag opened this issue Sep 8, 2017 · 3 comments
Closed

http - http.ClientRequest doesn't emit 'aborted'? #15259

ronag opened this issue Sep 8, 2017 · 3 comments
Labels
doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors.

Comments

@ronag
Copy link
Member

ronag commented Sep 8, 2017 •
edited
Loading

Looking through the _http_client.js code it seems to me that the http.ClientRequest object never emits an ´aborted´ event (only close). It's just the response object that emits the aborted event.

https://github.com/nodejs/node/blob/master/lib/_http_client.js#L366

That seems to go against the docs, https://nodejs.org/api/http.html#http_event_aborted
@ronag ronag changed the title HTTP request 'aborted' HTTP request missing 'aborted' Sep 8, 2017
@ronag ronag changed the title HTTP request missing 'aborted' HTTP request missing 'aborted' event Sep 8, 2017
@ronag ronag changed the title HTTP request missing 'aborted' event http.ClientRequest doesn't emit 'aborted'? Sep 8, 2017
@bnoordhuis bnoordhuis added doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors. labels Sep 8, 2017
@bnoordhuis
Copy link
Member

Yes, it's incorrect. It may have been copy/pasted from http.IncomingMessage, which does emit it.

@ronag ronag changed the title http.ClientRequest doesn't emit 'aborted'? http - http.ClientRequest doesn't emit 'aborted'? Sep 8, 2017
@ronag ronag mentioned this issue Sep 8, 2017
Closed
@odeke-em
Copy link

For posterity @ronag I am pasting in the permalink of the line that you referenced in code

node/lib/_http_client.js

Line 366 in c79fd2b

req.res.emit('aborted');
so that when the code is shuffled around, we can always refer to it.

Also this issue precedes and seems like it is related to #15283, should we be folding #15283 in here?

@lpinca
Copy link
Member

lpinca commented Sep 19, 2017 •
edited
Loading

The documentation for this event should be moved to http.ServerResponse.

Edit: nvm.

@ronag ronag mentioned this issue Sep 19, 2017
Closed
lpinca added a commit to lpinca/node that referenced this issue Sep 19, 2017
@lpinca
doc: fix 'aborted' event documentation
cf9a7b7 
The `'aborted'` event is only emitted on `http.IncomingMessage`
instances.

Fixes: nodejs#15259
This was referenced Sep 19, 2017
Closed
Closed
addaleax pushed a commit to addaleax/ayo that referenced this issue Sep 23, 2017
@lpinca @addaleax
doc: fix 'aborted' event documentation
e892b98 
The `'aborted'` event is only emitted on `http.IncomingMessage`
instances.

PR-URL: nodejs/node#15471
Fixes: nodejs/node#15259
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
jasnell pushed a commit that referenced this issue Sep 25, 2017
@lpinca @jasnell
doc: fix 'aborted' event documentation
1fbb86c 
The `'aborted'` event is only emitted on `http.IncomingMessage`
instances.

PR-URL: #15471
Fixes: #15259
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@lpinca lpinca mentioned this issue Sep 29, 2017
Closed
MylesBorins pushed a commit that referenced this issue Oct 17, 2017
@lpinca @MylesBorins
doc: fix 'aborted' event documentation
93152fe 
The `'aborted'` event is only emitted on `http.IncomingMessage`
instances.

PR-URL: #15471
Fixes: #15259
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this issue Oct 25, 2017
@lpinca @MylesBorins
doc: fix 'aborted' event documentation
377f7b9 
The `'aborted'` event is only emitted on `http.IncomingMessage`
instances.

PR-URL: #15471
Fixes: #15259
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

doc: fix 'aborted' event documentation lpinca/node
4 participants
@bnoordhuis @lpinca @ronag @odeke-em