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

net.Server.address() returns a string instead of an object for Pipes #12895

Closed
Flarna opened this issue May 8, 2017 · 8 comments
Closed

net.Server.address() returns a string instead of an object for Pipes #12895

Flarna opened this issue May 8, 2017 · 8 comments
Labels
doc Issues and PRs related to the documentations. net Issues and PRs related to the net subsystem.

Comments

@Flarna
Copy link
Member

Flarna commented May 8, 2017

  • Version: 6.10.0
  • Platform: Windows
  • Subsystem: net

According to the documentation server.address() should return an object with port, family, and address properties but if I call it on a HTTP server connected via windows pipe (on IIS Node on Windows Azure) I get a plain string telling the pipe name.

Is this just a documentation issue or should the return value for the pipe case adapted somehow (even port/familiy doesn't make that much sense for a pipe).

@Flarna Flarna changed the title net.Server.address() returns a string instead of an object net.Server.address() returns a string instead of an object for Pipes May 8, 2017
@refack refack self-assigned this May 8, 2017
@refack
Copy link
Contributor

refack commented May 8, 2017

Hello @Flarna could you provide a code snippet?

@addaleax addaleax added doc Issues and PRs related to the documentations. net Issues and PRs related to the net subsystem. labels May 8, 2017
@addaleax
Copy link
Member

addaleax commented May 8, 2017

net.createServer().listen('foo', function() { console.log(this.address()); })

I’d prefer just documenting this rather than risking breakage. As @Flarna pointed out, there wouldn’t really be any point in switching to an object.

@Flarna
Copy link
Member Author

Flarna commented May 8, 2017

const http = require("http");
const util = require("util");

const tcpServer = http.createServer().listen(8000);
const pipeServer = http.createServer().listen("\\\\.\\pipe\\thePipeName");

console.log(`tcpServer: ${util.inspect(tcpServer.address())}`);
console.log(`pipeServer: ${util.inspect(pipeServer.address())}`);

It's also applicable to Linux but there it's not a named pipe it's an unix domain socket which is handled by the same class in node.

see net.js:

Server.prototype.address = function() {
  if (this._handle && this._handle.getsockname) {
    var out = {};
    this._handle.getsockname(out);
    // TODO(bnoordhuis) Check err and throw?
    return out;
  } else if (this._pipeName) {
    return this._pipeName;
  } else {
    return null;
  }
};

@Flarna
Copy link
Member Author

Flarna commented May 8, 2017

As far as I know there is no API to check if a Server is using a Pipe or TCP besides checking the type of the internal _handle member which is not that nice.
Not sure if such an API is worth to add.
Not sure if it is really possible as anything with an _handle or fd member can be passed to listen therefore it's not really possible to tell what the server is actually listening on.

@addaleax
Copy link
Member

addaleax commented May 8, 2017

@Flarna Doesn’t checking the return type of .address() basically do that? ;)

@refack
Copy link
Contributor

refack commented May 8, 2017

I’d prefer just documenting this rather than risking breakage. As @Flarna pointed out, there wouldn’t really be any point in switching to an object.

🤔 is would be nice to have a way to get the type of underlying medium, so we could know what to expect from server.address() (AFAIK is could be an; fd / UNIX socket / TCP socket / Windows Pipe)

@Flarna
Copy link
Member Author

Flarna commented May 8, 2017

As long as we have only TCP and Pipes it's fine. Not sure if all usecases of listen() nail down to this two cases.

@addaleax
Copy link
Member

addaleax commented May 8, 2017

@Flarna Yes, those two are the only supported options (for net.Servers, at least). :)

anchnk pushed a commit to anchnk/node that referenced this issue May 19, 2017
Add documentation for net.server.address() for the case it is listening
on a pipe or unix domain socket instead an IP socket.

PR-URL: nodejs#12907
Fixes: nodejs#12895
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
gibfahn pushed a commit that referenced this issue Jun 20, 2017
Add documentation for net.server.address() for the case it is listening
on a pipe or unix domain socket instead an IP socket.

PR-URL: #12907
Fixes: #12895
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
MylesBorins pushed a commit that referenced this issue Jul 11, 2017
Add documentation for net.server.address() for the case it is listening
on a pipe or unix domain socket instead an IP socket.

PR-URL: #12907
Fixes: #12895
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@refack refack removed their assignment Oct 24, 2018
rob3c referenced this issue in apollographql/apollo-server Jul 23, 2019
…rred.

This is necessary after the changes in 0f75909
and #2344, as the
`AddressInfo` type didn't become available until a future version of
`@types/node`.

Regardless, it seems to me that this should be inferred without any problem
and we don't actually rely on the `AddressInfo` type, and instead are only
looking for an interface which has `port`, `address` and `family`, as the
inferred return value from `http.Server.prototype.address()` does.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. net Issues and PRs related to the net subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants