★ wanayoo — archive 1999 https://github.com/nodejs/node/pull/21670Nouvelle recherche | Portail wanayoo
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: fix HTTP res 'finish' description #21670

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
4 participants
@tadjik1
Copy link
Contributor

tadjik1 commented Jul 5, 2018

In addition to #21047, after #20611 close event will be emitted on response object after finish.

Checklist
@tadjik1

This comment has been minimized.

Copy link
Contributor Author

tadjik1 commented Jul 5, 2018 •

And one thought in addition to that: before #21047 close event was the only way to handle connection terminated by client because this event was emitted only in that case.
Now code for handling these situations will look like:

res.on('close', _ => {
  if (res.finished) return;
  
  // res.finished is false, which means that connection was terminated
  // so do some cleanup
});

Probably it is worth to be mentioned somewhere in documentation.

@addaleax

This comment has been minimized.

Copy link
Member

addaleax commented Jul 10, 2018 •

Landed in f386c7e 🎉

@addaleax addaleax closed this Jul 10, 2018

addaleax added a commit that referenced this pull request Jul 10, 2018

doc: fix HTTP res 'finish' description
PR-URL: #21670
Reviewed-By: James M Snell <jasnell@gmail.com>

targos added a commit that referenced this pull request Jul 12, 2018

doc: fix HTTP res 'finish' description
PR-URL: #21670
Reviewed-By: James M Snell <jasnell@gmail.com>

@targos targos referenced this pull request Jul 17, 2018

Merged

v10.7.0 proposal #21851

@Trott Trott referenced this pull request Aug 11, 2018

Closed

doc: require two approvals to land changes #22255

3 of 3 tasks complete

@refack refack referenced this pull request Oct 18, 2018

Merged

build: spawn `make test-ci` with `-j1` #23733

3 of 3 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.