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

"Mean" behavior: express does not complain if the port is undefined #3364

Closed
paulbrie opened this issue Jul 12, 2017 · 13 comments
Closed

"Mean" behavior: express does not complain if the port is undefined #3364

paulbrie opened this issue Jul 12, 2017 · 13 comments

Comments

@paulbrie
Copy link

app.listen(process.env.PORT, function () {
  console.log(`Server is running on port ${port}`)
})

In my case, process.env.PORT was not defined and ${port} was. Therefore I was having the impression that the server was running on the port displayed into the console.

It would be great if a check was done on the port parameter. Is there a specific reason why it's not?

Thanks,
Paul

@dougwilson
Copy link
Contributor

Hi @paulbrie the app.listen is just a wrapper around Node.js core https://nodejs.org/dist/latest-v8.x/docs/api/http.html#http_server_listen_port_hostname_backlog_callback . All the arguments are passed as-in directed into Node.js core. I agree there should be an error, and as far as I know, this has been fixed in Node.js already. What version of Node.js are you running?

@dougwilson
Copy link
Contributor

For reference, here is what that gives on Node.js 6.11.1:

$ node -v
v6.11.1
$ node -pe 'require("express")().listen(undefined, function(){})'
internal/net.js:17
    throw new RangeError('"port" argument must be >= 0 and < 65536');
    ^

RangeError: "port" argument must be >= 0 and < 65536
    at assertPort (internal/net.js:17:11)
    at Server.listen (net.js:1389:5)
    at EventEmitter.listen (express/lib/application.js:618:24)
    at [eval]:1:22
    at ContextifyScript.Script.runInThisContext (vm.js:25:33)
    at Object.runInThisContext (vm.js:97:38)
    at Object.<anonymous> ([eval]-wrapper:6:22)
    at Module._compile (module.js:570:32)
    at evalScript (bootstrap_node.js:353:27)
    at run (bootstrap_node.js:122:11)

@paulbrie
Copy link
Author

Hi Doug,

I'm on Mac Sierra.

node -v
v7.9.0

Here's the end of my main file:

// start the server
console.log('process.env.PORT', process.env.PORT)

app.listen(process.env.PORT, function () {
  console.log(`Server is running on port ${port}`)
})

The output in the console:

node index.js
process.env.PORT undefined
Server is running on port 3001

image

@dougwilson
Copy link
Contributor

dougwilson commented Jul 12, 2017

Hm, I'm not sure. Regardless, the app.listen is just a wrapper around Node.js core https://nodejs.org/dist/latest-v8.x/docs/api/http.html#http_server_listen_port_hostname_backlog_callback . All the arguments are passed as-in directed into Node.js core.

Does the following app have an error on Node.js 7.9.0 ? It does on 6.11.1, at least. If it doesn't, looks like maybe a regression you need to report to Node.js folks :)

var http = require('http')
var app = function () {}

var server = http.createServer(app)
server.listen(undefined, function () {
  console.log(`Server is running`)
})

And here is a one-liner:

$ node -pe 'require("http").createServer(function(){}).listen(undefined, function(){})'
internal/net.js:17
    throw new RangeError('"port" argument must be >= 0 and < 65536');
    ^

RangeError: "port" argument must be >= 0 and < 65536
    at assertPort (internal/net.js:17:11)
    at Server.listen (net.js:1389:5)
    at [eval]:1:44
    at ContextifyScript.Script.runInThisContext (vm.js:25:33)
    at Object.runInThisContext (vm.js:97:38)
    at Object.<anonymous> ([eval]-wrapper:6:22)
    at Module._compile (module.js:570:32)
    at evalScript (bootstrap_node.js:353:27)
    at run (bootstrap_node.js:122:11)
    at run (bootstrap_node.js:389:7)

@paulbrie
Copy link
Author

Hi Doug,

I like the one-liner! :) And here's the result for me:

node -pe 'require("http").createServer(function(){}).listen(undefined, function(){})'
Server {
  domain: null,
  _events:
   { request: [Function],
     connection: [Function: connectionListener],
     listening: { [Function: bound onceWrapper] listener: [Function] } },
  _eventsCount: 3,
  _maxListeners: undefined,
  _connections: 0,
  _handle:
   TCP {
     bytesRead: 0,
     _externalStream: {},
     fd: 11,
     reading: false,
     owner: [Circular],
     onread: null,
     onconnection: [Function: onconnection],
     writeQueueSize: 0 },
  _usingSlaves: false,
  _slaves: [],
  _unref: false,
  allowHalfOpen: true,
  pauseOnConnect: false,
  httpAllowHalfOpen: false,
  timeout: 120000,
  _pendingResponseData: 0,
  maxHeadersCount: null,
  _connectionKey: '6::::0' }

I'll go and post this to them.
Thank you for your support! :)

@dougwilson
Copy link
Contributor

Yep, looks like the undefined port argument is being interpreted as "listen on a random port" in Node.js 7.9.0, at lest. It's also possible that there was a change at some point in Node.js where now undefined is a valid value to mean "listen on any port", though I doubt that, so it really seems like a regression at some point.

@paulbrie
Copy link
Author

You were right!

lsof -n -i4TCP | grep node
> node      13509 paulbrie   11u  IPv6 0xa2129ac75a2ae969      0t0  TCP *:49653 (LISTEN)

I killed the process and restarted it

lsof -n -i4TCP | grep node
> node      13509 paulbrie   11u  IPv6 0xa2129ac75a2ae969      0t0  TCP *:49740 (LISTEN)

A bit counter intuitive. Any interesting reason for randomising the port?

@dougwilson
Copy link
Contributor

I'm not sure. Probably a question for Node.js core folks :)

@wesleytodd
Copy link
Member

wesleytodd commented Jul 12, 2017

I do believe this changed recently, but I believe that it was that it no longer accepts null to do a random port, and now requires 0 for node 8. But I did not look up docs on this, so I could be wrong.

EDIT: Ok, just tried it with node 8 and 6. In node 6 null and 0 work, undefined fails. In node 8, null fails and 0 and undefined work.

@wesleytodd
Copy link
Member

wesleytodd commented Jul 12, 2017

As for "why" using a random port, we use it for running multiple instances of a service on one machine and then registering with a service discovery layer like Consul. So we tell all our prod services to open on any random open port picked by the OS, then send that to Consul so the other services know how to reach it.

EDIT: using port 0 is a unix systems thing to tell it to pick a random tcp port.

@paulbrie
Copy link
Author

Hi Wesley, thanks so much for the explanation. Now it makes sense and I get why this behavior has been implemented.

@ghost
Copy link

ghost commented Jul 13, 2017

I think you have defined a variable called port somewhere?

@paulbrie
Copy link
Author

Yes, I did somewhere above an it was receiving a default value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants