Node: Promisify server.listen

[caub]

Problem:

const server = app.listen();
console.log(`Server listening on: ${server.address().port}`)

can fail, as the server is created asynchronously and using the callback is not always practical/flexible

Proposal

either server.listenPromise(port), or even better, have server.listen(port) return a Promise if no callback is provided

Example

// .... 

const start = async () => {
    await config.load();
    await DB.connect(dbOptions);
    router(app);
    app.use((err, req, res, next) => { // error handler
        // ...
    });

    const server = http.createServer(app);
    // proposal:
    return server.listen(process.env.PORT || 3000); // if .listen would be a promise resolving to server
    // or await server.listen(...); return server; else

    // currently I have to do:
    // return new Promise((res, rej) => server.listen(process.env.PORT || 3000, err => err ? rej(err) : res()));

    // or using express's app.listen, which wraps createServer and listen:
    // return new Promise((res, rej) => app.listen(process.env.PORT || 3000, function (err) {
    //      if (err) return rej(err);
    //      res(this) // this is node's http Server
    // }));

    // using util.promisify doesn't look much better:
    // return promisify(server.listen).bind(server)(process.env.PORT || 3000);
    // or
    // return promisify(cb => server.listen(process.env.PORT || 3000, cb))();
};

then using it somewhere else:

start()
        .then(server => console.log(`Server listening on: ${server.address().port}`))
        .catch(err => {
           // ...
        });

Edit: I'll try to submit this request to express, and have a similar promise method near https://github.com/expressjs/express/blob/master/lib/application.js#L616-L619

Edit2: the discussion in expressjs/express#3675 has interesting details

[jsmrcaga]

From what i see here, there would never be a real .catch, since the callback is being forwarded to the Server.on('listening' event (except for two edge cases, you would still need server.on('error')). This tricks you into believing your callback is being called once the server.listen function has executed properly.

I can introduce a Promise return and start a PR if you like (as well as for Server.prototype.close and Server.prototype.getConnections), but I think this particular case needs more attention than a single promise return. 🤔

If so, should the returned Promise be conditional on the presence of a callback ? This could lead to some confusion, but then you might argue that we need that for retrocomp. 😄

[jsmrcaga]

I have been checking and experimenting with promisifying server.listen + server.close and server.getConnections. There are two options, the 'lie to the devs' and return a Promise.resolve() or a more real option that would wrap the "maybe" callback and return a Promise.

I can launch the PR if anyone is willing to read it and @node/collaborators think this is a good idea

[caub]

@jsmrcaga Sorry but I didn't really understand how what you proposed would look like

just adding here another concise way suggested by @bmeck a while ago

await { then(r, f) { server.on('listening', r); server.on('error', f); } };

But I still think a shortcut for this would be handy

[shellscape]

@caub please forgive the naiveté, but is that a new fancy syntax capability for await or is that pseudo code?

[addaleax]

@shellscape It’s the (existing) ability of await to handle any value after it that has a .then() method that behaves like the Promise.prototype.then() one. This might be a slightly better example:

> await ({ then(resolve) { resolve(42) } })
42

I’m not sure it’s the most readable style to use await with, though. :smile:

[shellscape]

@addaleax thanks! just learned something 😀

[MartinMuzatko]

@caub I was pretty surprised, that util.promisfy needed to .bind() the scope :O

Otherwise, I am fine with promisify and bind, it does look ugly but it is better than nothing 👍

[axkibe]

For the record thats an example that works (within an async function). As @MartinMuzatko pointed out, you need bind(), which is not quite obvious.

const server = https.createServer( options, handler );
const promise = util.promisify( server.listen.bind( server ) );
await promise( port, listen );

I'd too prefer that listen() would return a Promise whenever there isn't a callback provided.

For example the mongodb driver does this in a very nice way, which makes using it in async flow controls easy and compact.

PS: It would be cool if this would work with all asynchronous Node.js core library functions. If there isn't a callback, return a promise...

[strarsis]

@axkibe: Using your code, listen is undefined though afterwards, I need a reference to it for calling close on app shutdown.

[stavalfi]

@jsmrcaga Sorry but I didn't really understand how what you proposed would look like

just adding here another concise way suggested by @bmeck a while ago

await { then(r, f) { server.on('listening', r); server.on('error', f); } };

But I still think a shortcut for this would be handy

don't you need to unsubscribe from the 'error' when 'listening' is fired? and the opposite as well?

[webarnes]

You can use the "once" helper from "events":

const { once } = require('events');
...
const server = app.listen();
await once(server, 'listening');

More lines but fewer characters than the above solution and no need to unsubscribe.

That said, I'd love to just write const server = await app.listen();

Edit: Use CJS instead of ESM.

[SrBrahma]

@jsmrcaga Sorry but I didn't really understand how what you proposed would look like
just adding here another concise way suggested by @bmeck a while ago

await { then(r, f) { server.on('listening', r); server.on('error', f); } };

But I still think a shortcut for this would be handy

don't you need to unsubscribe from the 'error' when 'listening' is fired? and the opposite as well?

How to use it? I really can't understand it.

Tried this without success, stays stuck on '0'.

const app = require('express')();

async function listen() {
  const server = app.listen(3000);
  console.log(0);
  const r = () => console.log(1);
  const f = () => console.log(2);

  await { then(r, f) { server.on('listening', r); server.on('error', f); } };
  console.log(3);
}

listen();

[jsmrcaga]

@SrBrahma I'm not sure your 'listening' event exists, I don't see it nor on the official docs Node 14.2 or the express v4 docs.

My advice is to:

const app = express();

function listen() {
    return new Promise((resolve, reject) => {
         app.listen(3000, (err) => {
            if(err) {  return reject(err); }
            return resolve();
        });
    });
}

listen().then().catch()

if you wish to use async/await:

await listen();
// the rest of your code

[SrBrahma]

Hey @jsmrcaga! I don't know if it shows up in docs, but I saw it in a couple of places, and also typescript intelisense shows it:

Your example, for some reason, don't call the callback (edit: looks like, on error, the callback isn't called). Having two instances of node running this, returns:

events.js:287
throw er; // Unhandled 'error' event
^

Error: listen EADDRINUSE: address already in use :::3000

[SrBrahma]

Managed to get to this working solution:

const app = express();

function listen(port) {
  return new Promise((resolve, reject) => {
    app.listen(port)
      .once('listening', resolve)
      .once('error', reject);
  });
}
// Then / catch
listen(3000).then(() => console.log(1)).catch(err => console.log(2, err));

// Async / await
async function main() {
  try {
    await listen(3000);
    console.log('success');
  }
  catch (err) {
    console.log('oh no');
    // console.log(err);
  }
}
main();

This really should be in node / express.

[jsmrcaga]

@SrBrahma i'm happy you got it working!
I'm gonna take another look at node's code to see what we can do, but promisifying one function means promisifying the whole package (like it was done for fs), so it could or not be an easy task.

In any case, one solution might be to take your snippet and make it into a server.listenPromise function.
I'll take a look at it tomorrow

[SrBrahma]

:)

Maybe, asyncListen().

Have a good weekend!

Em sáb, 9 de mai de 2020 04:37, Jo Colina [e-mail]
escreveu:

@SrBrahma https://github.com/SrBrahma i'm happy you got it working!
I'm gonna take another look at node's code to see what we can do, but
promisifying one function means promisifying the whole package (like it was
done for fs), so it could or not be an easy task.

In any case, one solution might be to take your snippet and make it into a
server.listenPromise function.
I'll take a look at it tomorrow

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/nodejs/node/issues/21482#issuecomment-626122686, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AGW7JDR2ZVPYT4FQGZS5PETRQUB2TANCNFSM4FGSXBFA
.

[axkibe]

Any updates on this? I was just restructering my code to use fs.promises instead of all those promisify wrappers and came onto this promisify instance that seems to be still needed.

Can I please get a good explanation what is wrong with all of the core API just returning a promise in case of no callback provided? Similar I don't quite get, why fs needs a fs.promises declaration on the top. Not that it's a big issue tough.