-1

I have a mongoose connect function in which I try to wait for reconnecting if the first attempt fails:

async connect() {
    const options = {...}
    try {
        console.log("starting")
        await this._connectWithRetry(options)
        console.log("finished")
    } catch (err) {
        winston.error(`Could not connect to Mongo with error: ${err}`)
    }
}

private async _connectWithRetry(options) {
    return new Promise( async (resolve, reject) => {
        try {
            winston.info("Connecting to mongo...")
            await mongoose.connect(this.dbURI, options)
            winston.info("Connection successful.")
            resolve()
        } catch (err) {
            winston.info("Failed to connect to mongo. Retrying in 5 seconds...")
            setTimeout( async () => {
                await this._connectWithRetry(options)
            }, 5000)
        }
    })
}

It successfully waits until I'm connected. But once I connect, the second console line is not hit ("finished"). so I figure that my promise resolution is buggy. What am I doing wrong?

isherwood
  • 58,414
  • 16
  • 114
  • 157
phoebus
  • 1,280
  • 1
  • 16
  • 36
  • 4
    Hint: `Promise.resolve()` is not the same as `resolve()`. Also, if the connection fails, the recursive call will create a new Promise and the original will disappear without settling, either, so that's no good. – IceMetalPunk Jan 17 '20 at 20:00
  • `Promise.resolve()` returns a *new resolved promise*. However, if you call the `resolve()` argument, you are *resolving the promise it belongs to*. – VLAZ Jan 17 '20 at 20:01
  • If you go in the `catch` then you'd never resolve nor reject the first promise you create. If the second call succeeds, then you'd resolve a *different promise* while the first one would still be pending. – VLAZ Jan 17 '20 at 20:04
  • Try to remove async from inside Promise( async (resolve, reject) just Promise( (resolve, reject). That's my guess ;) – Rodrigo Jan 17 '20 at 20:54
  • Does it print "connection successful"? – Ben Aston Jan 17 '20 at 21:11
  • 1
    [Never pass an `async function` as the executor to `new Promise`](https://stackoverflow.com/q/43036229/1048572)! – Bergi Jan 17 '20 at 22:55

1 Answers1

0

Your code "works" if the connection to the DB is established first time around.

If the retry mechanism is used, you will see the error you describe.

The Promise instantiated by the first call to mongoDBConnect is never resolved in the retry execution path.

This is because subsequent invocations of mongoDBConnect are made in a totally separate execution context on a future tick of the event loop, controlled by the setTimeout - and each invocation instantiates a new Promise totally disconnected from your connect function.

This refactoring should fix the issue:

const delay = (interval) => new Promise(resolve => setTimeout(resolve, interval))

async connect() {
    const options = {...}
    try {
        console.log("starting")
        await this._connectWithRetry(options)
        console.log("finished")
    } catch (err) {
        winston.error(`Could not connect to Mongo with error: ${err}`)
    }
}

private async _connectWithRetry(options) {
    try {
        winston.info("Connecting to mongo...")
        await mongoose.connect(this.dbURI, options)
        winston.info("Connection successful.")
    } catch (err) {
        winston.info("Failed to connect to mongo. Retrying in 5 seconds...")
        await delay(5000)
        await this._connectWithRetry(options)
    }
}

Test harness:

let retryCount = 0

const mongoose = {
    connect: ()=>retryCount++ === 2 ? Promise.resolve() : Promise.reject('fake error')
}

async function connect() {
    try {
        console.log("starting")
        await connectWithRetry()
        console.log("finished")
    } catch (err) {
        console.error(`connect error`, err)
    }
}

async function connectWithRetry() {
    try {
        console.log("Connecting to mongo...")
        await mongoose.connect()
        console.log("Connection successful.")
    } catch (err) {
        console.log("Retrying in 1 second...", err)
        await delay(1000)
        await connectWithRetry()
    }
}

const delay = (interval) => new Promise(resolve => setTimeout(resolve, interval))

connect()
Ben Aston
  • 53,718
  • 65
  • 205
  • 331
  • 1
    Inside `_connectWithRetry()`, `return this._connectWithRetry(options)` would (in general) make more sense than `await this._connectWithRetry(options)`. We can get away with `await` in this case as we're relying only on settlement of the returned Promise, not on its ability to deliver a result. – Roamer-1888 Jan 18 '20 at 02:32