stop() returning a promise and not decorating the server

[novemberborn]

I'm curious whether folks are interested in revisiting some of the design decisions.

For one it'd be interesting if stoppable() could return the stop() function, rather than assigning it to the server value. Calling code could still do server.stop = stoppable(server). Personally I don't find the chaining argument convincing.

It'd also be neat if stop() would return a promise, rather than taking a callback. It wouldn't match the server.close() API but in these glorious days of async / await I think returning a promise would be more usable.

Once stop() no longer takes a callback it'd be possible to pass grace directly.

These last two changes combined make it possible to use stoppable with https://www.npmjs.com/package/@digicat/termination-manager (which, full disclaimer, I wrote). That library can vary the grace period based on whether the process is crashing or merely terminating, which doesn't work with stoppable as-is.

If folks disagree I'm happy to publish a fork elsewhere but I figured I'd ask first. These nits aside stoppable is the nicest library I've found for stopping servers 🥇

Thanks for reading this far!

[mvasilkov]

Wrt stop(): it can be kept backward-compatible if you still accept callback, and return promise if the callback is undefined. That's what e.g. levelup (the LevelDB wrapper) does with their APIs.

[ronkorving]

@novemberborn I like it 👍

[boneskull]

I would rather not change the API of stop, as returning a Promise when the callback is missing implies stuff about exception handling that may shock users who are calling stop() without the (optional) callback.

How about just adding stopAsync()?

[Anthonyzou]

You can use something like this in your code if you want to use a promise

new Promise((resolve, reject) => {
  stoppableServer.stop((err, status) => (err ? reject(err) : resolve(status)));
});

[pimterry]

I'd appreciate promises here too. Almost all my code is promises & async/await now, so the callback here is a bit awkward. I'd prefer to just make the callback optional on stop(), and return a promise if it's not provided. It seems that that's what most other libraries have done, but either option would be a nice improvement.