0

I'm new to Node JS, and I'm struggling to handle error properly when using promises. Here is what I currently have:

I have a module call db.js with an init function used to set up and start the db connection:

function initDb(){
return new Promise((resolve, reject) => {
    if(database){
        console.warn("Trying to init DB again !")
    }else{
        client.connect(config.db.url, {useNewUrlParser: true, useUnifiedTopology: true})
        .then((client) => {
            console.log("DB initialized - connected to database: " + config.db.name)
            database = client.db(config.db.name)
        })
        .catch((err) => {
            reject(err)
        })
    }
    resolve(database)
})}

This function will return a promise, and it will be called into the index.js:

initDb()
.then(() => {
    app.listen(8081, function () {
        console.log('App is running on port 8081')
    })
})
.catch((error) => {
    console.log(error)
})

As you can see I have two catch. One in the db module and the other one in the index.js

It seams weird to catch error in two places... What is the good pattern to use to handle error in this case ?

RomainV
  • 293
  • 4
  • 16
  • why are you setting up a new Promise with a promise inside of it? client.connect also returns a promise if I read your code correctly – Karlan Sep 06 '19 at 10:29

2 Answers2

2

You'll want to avoid the Promise constructor antipattern. Also don't store the database itself in a variable, store the promise for it instead.

let databasePromise;
function initDb() {
    if (databasePromise) {
        console.debug("DB already initialised");
        return databasePromise;
    }
    databasePromise = client.connect(config.db.url, {useNewUrlParser: true, useUnifiedTopology: true})
    .then((client) => {
        console.log("DB initialized - connected to database: " + config.db.name)
        return client.db(config.db.name)
    });
    return databasePromise;
}
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • Thanks, really interesting post ! I just have one question. What I currently have is the initDb() to connect and store the db object in a variable, and another function: getDb() , to retrieve the DB object, so I can import this in other modules and use it really easily. But you mentionned not to store the database object. But if I store the promise then I lost the ability of calling the db easily. The code will look like : getDb().then((db) => { return db.collection('foo').find() }) , instead of just getDb().collection.find() . Am I missing something ? – RomainV Sep 06 '19 at 12:49
  • 1
    How else would you make sure that the database calling code runs only after the connection has been initialised? Maybe instead of having a `getDb` function that you can import wherever you like, make the `db` an explicit parameter of the functions that need it. (Good also for inversion of control) – Bergi Sep 06 '19 at 12:56
-1

If you don't write catch in the initDb function in db.js module, you can catch that error in the calling function, so it's okay if you don't write .catch((err) => { reject(err)}) in db.js module, the error will go to calling function in index.js and you can directly handle it there.

It is not weird to catch the error at both the place, in fact in coming versions of node this will be a recommended practice to write catch(err) for handling errors at all promises.