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

Node 10: http response.close is always triggered #21063

Closed
msmol opened this issue May 31, 2018 · 7 comments
Closed

Node 10: http response.close is always triggered #21063

msmol opened this issue May 31, 2018 · 7 comments
Labels
http Issues or PRs related to the http subsystem.

Comments

@msmol
Copy link

msmol commented May 31, 2018

  • Version: Node 10.3.0
  • Platform: Linux 4.16.11
  • Subsystem: http

index.js:

const http = require('http');

http.createServer((req, res) => {
  res.writeHead(200);
  
  res.on('finish', () => {
    console.log('finish');
  });

  res.on('close', () => {
    console.log('close');
  });
  
  res.end();

}).listen(3000);
$ nvm use 10; node index.js
Now using node v10.3.0 (npm v6.1.0) # run curl localhost:3000 now
finish
close
^C
$ nvm use 8; node index.js
Now using node v8.9.4 (npm v5.6.0) # run curl localhost:3000 now
finish
^C
$

According to the docs res.close should only be called if res.end is not. Unfortunately there doesn't seem to be an event emitted for res.end so I could not check to see whether or not it was triggered.

However, as is visible in the example above:

  • Node 8
    • finish event triggered
    • close event not triggered,
  • Node 10
    • finish event triggered
    • close event triggered
@mscdex
Copy link
Contributor

mscdex commented May 31, 2018

Related: #21047
Ref: #20611

@msmol
Copy link
Author

msmol commented May 31, 2018

If I understand correctly, #21047 implies that res.close always being called is now considered the correct behavior?

@addaleax
Copy link
Member

@nodejs/http Anything to do here?

addaleax added a commit to addaleax/node-pre-gyp that referenced this issue Jul 26, 2018
This addresses a changed behaviour in Node 10, resulting
from the fact that the `close` event is always being emitted
now. This would have lead to the `place_binary()` callback being
called twice when the HTTP request failed (with e.g. a 404).

Refs: nodejs/node#21063
Fixes: mapbox#391
@targos
Copy link
Member

targos commented Jul 26, 2018

We are in the process of reverting the change in #21809

@apapirovski
Copy link
Member

Sounds like there's nothing left to address here. The change was reverted.

@Srinivas-Vemula
Copy link

we are seeing this issue on Node V12.13.x.

@mscdex
Copy link
Contributor

mscdex commented Jan 17, 2020

@Srinivas-Vemula That's expected as the change was kept in master, it was only reverted for v10.x.

hyj1991 pushed a commit to X-Profiler/node-pre-gyp that referenced this issue Jun 16, 2023
This addresses a changed behaviour in Node 10, resulting
from the fact that the `close` event is always being emitted
now. This would have lead to the `place_binary()` callback being
called twice when the HTTP request failed (with e.g. a 404).

Refs: nodejs/node#21063
Fixes: mapbox#391
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
http Issues or PRs related to the http subsystem.
Projects
None yet
Development

No branches or pull requests

6 participants