4
\$\begingroup\$

This is some code that fetches a result from a database. It connects, makes a query, processes the query, then has to close the DB (under all possible code paths) and return the result.

It seems like the code for closing the database under all circumstances is a bit messy. I'm wondering if there's a cleaner way to do this?

function getBlocked(data) {
    let openDb;

    close() {
        if (openDb) {
            openDb.close().catch(err => {
                console.log("Error closing db: ", err);
            })
        }
    }

    return MongoClient.connect(url).then(db => {
        openDb = db;
        const blQ = {blocked_user:data.tag_search_mail};
        return db.collection("block_list").find(blQ,{"_id":0}).toArray();
    }).then(results => {
        let blocked = results.map(item => item.blocker);
        close();
        return blocked;     // make this the resolved value of the promise
    }).catch(err => {
        close();
        throw err;          // rethrow to keep the promise rejected
    })
}

Usage:

getBlocked(data).then(blocked => {
    // use blocked array here
}).catch(err => {
    // handle error here
});

FYI, it is intentional to return the result without waiting for the DB to close (thus why it's not inserted in the promise chain) and to only log the error, not fail the main operation if closing the db fails.

\$\endgroup\$
0
\$\begingroup\$

I've discovered that node.js v10 supports .finally() for promises (or you could employ a polyfill) so it could be slightly improved by moving the close() logic into a .finally() handler.

function getBlocked(data) {
    let openDb;

    return MongoClient.connect(url).then(db => {
        openDb = db;
        const blQ = {blocked_user:data.tag_search_mail};
        return db.collection("block_list").find(blQ,{"_id":0}).toArray();
    }).then(results => {
        let blocked = results.map(item => item.blocker);
        return blocked;     // make this the resolved value of the promise
    }).finally(() => {
        if (openDb) {
            openDb.close().catch(err => {
                console.log("Error closing db: ", err);
            })
        }
    });
}

Now, it seems as an additional improvement for this particular usage, you can move the .finally() clause to a place where you don't have to save the db variable to the higher scope because all paths with a successfully open db go through one promise chain (assuming no exception in the .then() handler is thrown before db.collection() is called:

function getBlocked(data) {
    return MongoClient.connect(url).then(db => {
        const blQ = {blocked_user:data.tag_search_mail};
        return db.collection("block_list").find(blQ,{"_id":0}).toArray().finally(() => {
            db.close().catch(err => {
                console.log("Error closing db: ", err);
            });
        });
    }).then(results => {
        let blocked = results.map(item => item.blocker);
        return blocked;     // make this the resolved value of the promise
    });
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.