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.Server.close return value mismatch between the documentation and the implementation. #51787

Closed
dlOuOlb opened this issue Feb 17, 2024 · 0 comments · Fixed by #51797
Closed

Comments

@dlOuOlb
Copy link

dlOuOlb commented Feb 17, 2024

Version

v21.6.2

Platform

Microsoft Windows NT 10.0.19045.0 x64

Subsystem

No response

What steps will reproduce the bug?

import * as http from 'node:http';

const server = http.createServer();

console.info(server === server.close());

How often does it reproduce? Is there a required condition?

It seems to reproduce unconditionally, with just node "the-above-snippet.js".

What is the expected behavior? Why is that the expected behavior?

The output should be:

true

Because the documentation says:

Where the returned server instance seems to be intended to be the server instance to close.

What do you see instead?

false

Because server.close() returned undefined.

Additional information

  • It seems that the following implementation needs a proper return statement.
     Server.prototype.close = function() {
       httpServerPreClose(this);
       ReflectApply(net.Server.prototype.close, this, arguments);
     };
nodejs-github-bot pushed a commit that referenced this issue Feb 26, 2024
PR-URL: #51797
Fixes: #51787
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
marco-ippolito pushed a commit that referenced this issue Feb 27, 2024
PR-URL: #51797
Fixes: #51787
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
richardlau pushed a commit that referenced this issue Mar 25, 2024
PR-URL: #51797
Fixes: #51787
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
richardlau pushed a commit that referenced this issue Mar 25, 2024
PR-URL: #51797
Fixes: #51787
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
rdw-msft pushed a commit to rdw-msft/node that referenced this issue Mar 26, 2024
PR-URL: nodejs#51797
Fixes: nodejs#51787
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant