Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

app.listen() error behavior is different in Express 4 vs. 5, if port is occupied #6191

Closed
heidemn-faro opened this issue Nov 28, 2024 · 3 comments · Fixed by expressjs/expressjs.com#1705
Assignees
Labels

Comments

@heidemn-faro
Copy link

heidemn-faro commented Nov 28, 2024

Hi there,
Great to see that Express 5 with async request handlers is finally available :-)

I have a question about behavior that differs in Express 4 vs. 5, if the listen port is already in use.
I wanted to know if that's on purpose, or if it's a bug.
If it's on purpose, it should probably be mentioned here? https://expressjs.com/en/guide/migrating-5.html
Thanks for having a look!

// minimal-express.mjs

import express from 'express';
import expressPkg from 'express/package.json' with { type: "json" };

const app = express();
app.use(express.json());
app.get('/api/health', (req, res) => {
	res.status(200).json({ status: 'ok' });
});

console.log('Express ', expressPkg.version);
console.log('Starting Express server on port 7072...');
const server = app.listen(7072, '0.0.0.0', () => {
	console.log('Callback');
	if (!server.address()) {
		// This line is only reachable in Express 5, but not in Express 4.
		throw new Error('Failed to start server (Express 5).');
	}
	console.log('Listening on http://localhost:7072/ - e.g. http://localhost:7072/api/health');
});

Enviroment: WSL2, Ubuntu 22.04, Node 20 or 22

PRETTY_NAME="Ubuntu 22.04.5 LTS"
Linux MYHOSTNAME 5.15.167.4-microsoft-standard-WSL2 #1 SMP Tue Nov 5 00:21:55 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux

Node.js v20.18.1
Node.js v22.11.0 (same result)

Results with Express 4, if port is already in use:

Express  4.21.1
Starting Express server on port 7072...
node:events:497
      throw er; // Unhandled 'error' event
      ^

Error: listen EADDRINUSE: address already in use 0.0.0.0:7072
    at Server.setupListenHandle [as _listen2] (node:net:1904:16)
    at listenInCluster (node:net:1961:12)
    at doListen (node:net:2135:7)
    at process.processTicksAndRejections (node:internal/process/task_queues:83:21)
Emitted 'error' event on Server instance at:
    at emitErrorNT (node:net:1940:8)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21) {
  code: 'EADDRINUSE',
  errno: -98,
  syscall: 'listen',
  address: '0.0.0.0',
  port: 7072
}

Results with Express 5, if port is already in use:

Express  5.0.1
Starting Express server on port 7072...
Callback
file:///some/folder/minimal-express.mjs:14
                throw new Error('Failed to start server (Express 5).');
                ^

Error: Failed to start server (Express 5).
    at Server.<anonymous> (file:///some/folder/minimal-express.mjs:14:9)
    at Server.f (/some/folder/node_modules/once/once.js:25:25)
    at Object.onceWrapper (node:events:634:26)
    at Server.emit (node:events:519:28)
    at emitErrorNT (node:net:1940:8)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21)
@heidemn-faro heidemn-faro changed the title app.listen() working differently in Express 4 vs. 5, if port is occupied app.listen() error behavior is different in Express 4 vs. 5, if port is occupied Nov 28, 2024
@dpopp07
Copy link
Contributor

dpopp07 commented Dec 3, 2024

Although I don't know for sure, the code indicates that this is on purpose. Comparing the implementation of the listen method in 4.x ...

express/lib/application.js

Lines 633 to 636 in 59fc270

app.listen = function listen() {
var server = http.createServer(this);
return server.listen.apply(server, arguments);
};

... with 5.x ...

express/lib/application.js

Lines 609 to 617 in 344b022

app.listen = function listen () {
var server = http.createServer(this)
var args = Array.prototype.slice.call(arguments)
if (typeof args[args.length - 1] === 'function') {
var done = args[args.length - 1] = once(args[args.length - 1])
server.once('error', done)
}
return server.listen.apply(server, args)
}

... the code now explicitly invokes the user-provided callback (if it exists) in the event of an error. I'm guessing this is to provide the user with more control over their errors, instead of being subject to a crashing app. However, I agree with you that this transfer of error-handling responsibility to the user in the listen method would be good to capture in the migration docs. I can open a PR for that soon.

dpopp07 added a commit to dpopp07/expressjs.com that referenced this issue Dec 3, 2024
Adds a note to the `migrating-5` guide alerting users that error events
received by a server will now cause the callback argument in `app.listen` to
be invoked. In Express 4, this was not the case and errors would be thrown.

Resolves expressjs/express#6191

Signed-off-by: Dustin Popp <dustinpopp@ibm.com>
@heidemn-faro
Copy link
Author

Okay probably the Express user is now required to check the 1st argument of the callback.

const server = app.listen(7072, '0.0.0.0', (error) => {
	if (error) {
		// This line is only reachable in Express 5, but not in Express 4.
		console.error("Failed to start server (Express 5)");
		throw error;
	}
	console.log('Listening on http://localhost:7072/ - e.g. http://localhost:7072/api/health');
});

Works fine:

$ node /tmp/express-test/minimal-express.mjs
Express  5.0.1
Starting Express server on port 7072...
Failed to start server (Express 5)
file:///tmp/express-test/minimal-express.mjs:16
                throw error;
                ^

Error: listen EADDRINUSE: address already in use 0.0.0.0:7072
    at Server.setupListenHandle [as _listen2] (node:net:1817:16)
    at listenInCluster (node:net:1865:12)
    at doListen (node:net:2014:7)
    at process.processTicksAndRejections (node:internal/process/task_queues:83:21) {
  code: 'EADDRINUSE',
  errno: -98,
  syscall: 'listen',
  address: '0.0.0.0',
  port: 7072
}

dpopp07 added a commit to dpopp07/expressjs.com that referenced this issue Dec 4, 2024
Adds a note to the `migrating-5` guide alerting users that error events
received by a server will now cause the callback argument in `app.listen` to
be invoked. In Express 4, this was not the case and errors would be thrown.

Resolves expressjs/express#6191

Signed-off-by: Dustin Popp <dustinpopp@ibm.com>
Co-authored-by: M. Heide <66078329+heidemn-faro@users.noreply.github.com>
dpopp07 added a commit to dpopp07/expressjs.com that referenced this issue Dec 4, 2024
Adds a note to the `migrating-5` guide alerting users that error events
received by a server will now cause the callback argument in `app.listen` to
be invoked. In Express 4, this was not the case and errors would be thrown.

Resolves expressjs/express#6191

Signed-off-by: Dustin Popp <dustinpopp@ibm.com>
Co-authored-by: M. Heide <66078329+heidemn-faro@users.noreply.github.com>
@wesleytodd
Copy link
Member

wesleytodd commented Dec 4, 2024

I took a quick read here, but am a bit busy with other work so don't have too much time to add more here, just wanted to link the original PR for context: #3216

EDIT: and the original one with the more context on why this change was made. #2623

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants