0

I want to rewrite this function to resolve the promise instead of calling the callback function in an attempt to better understand working with promises.

export const connect = (callback: CallableFunction|void): void => {
    LOG.debug("Connecting to DB at %s", URI);

    connectToMongoDb(URI, opts)
        .then((result) => {
            LOG.debug("Connection established");
            connection = result;

            if (callback) {
                callback();
            }
        })
        .catch((err) => {
            LOG.error("Could not establish connection to %s, retrying...", URI);
            LOG.error(err);
            setTimeout(() => connect(callback), 5000);
        });
};

However, I don't seem to be able to. I already tried the naive approach of doing

export const connect = (): Promise<void> => new Promise((resolve): void => {
    // ...´
        .then((result) => {
            LOG.debug("Connection established");
            connection = result;
            resolve();
        })
    // ...
});

But this doesn't correctly resolve when the connection was established.

What am I doing wrong? How can I rewrite this to properly use and resolve the Promise instead of using a callback function?

F.P
  • 17,421
  • 34
  • 123
  • 189
  • 1
    how are you using it? is `connect` never resolved? – Federkun Oct 09 '19 at 18:44
  • 1
    Avoid the [`Promise` constructor antipattern](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it)! Make a promisified `delay` function that wraps the `setTimeout`, then use that with proper promise chaining. – Bergi Oct 09 '19 at 19:13
  • @Federkun I want to call `connect().then()` that resolves only when the `connectToMongo` resolves. What happens however is that `connect` never resolves – F.P Oct 09 '19 at 19:46
  • @Bergi I'm not sure how to apply the suggestions you linked to my example. I can't simply return the Promise of `connectToMongoDb` because that will just move the problem to the calling code... ? – F.P Oct 09 '19 at 19:46
  • 1
    Yes you can. `return connectToMongoDb(URI, opts).then(…).catch(err => { …; return delay().then(connect); });`. – Bergi Oct 09 '19 at 19:50

1 Answers1

2

As per the comments, I was able to refactor the function to properly return a promises that resolves only when the connections is established:

const delay = (ms: number): Promise<void> =>
    new Promise((resolve): void => {
        setTimeout(resolve, ms);
    });

export const connect = (): Promise<void> => {
    LOG.info("Connecting to DB at %s", URI);

    return connectToMongoDb(URI, opts)
        .then((result) => {
            LOG.info("Connection to DB established");
            connection = result;
        })
        .catch((err) => {
            LOG.error("Could not establish connection to %s, retrying...", URI);
            LOG.error(err);
            return delay(5000).then(connect);
        });
};
F.P
  • 17,421
  • 34
  • 123
  • 189
  • Instead of setting a global `connection` variable, you should fulfill your promise with it by doing `return result` – Bergi Oct 12 '19 at 00:16
  • I changed that part after all because mongoose handles the connection pool internally anyway. I don't need the returned connection object actually. – F.P Oct 12 '19 at 03:56