Loading a database table using a promise chain

Question

I am creating a util module that I'm using to communicate with a MS-SQL database. I want each public method to return a promise. I started with a private function that executes a DB query and returns a promise. The function establishes a connection to the DB then it executes the query. My current code just lets all errors fall through to a single catch, and I'm not sure if this is a best practice or not.

I was hoping someone could answer that for me and any other suggestions about code read ability or anything.

var config = require('./mainConfig')();
var sql = require('mssql');
var connection =  new sql.Connection(config.db.sql);
var Promise = require("bluebird");





exports.getAll = function(table){

    return executeQueryStatment("select * from " + table);

};



function executeQueryStatment(query, newconnection) {

    var conn =  newconnection || connection; 
    var request = new sql.Request(conn);

    return new Promise(function(res, rej){

     conn.connect().then(function(){

       return request.query(query);

     }).then(function (data) {

         res(data);
         conn.close();
     })
     .catch(function(err) {
        // plan on handling all errors here
        // is it better to handle each error individually upstream?  
        console.log(err);
        conn.close();

    })

  });


};

Answer 1

Having a catch at the very end is fine. This assumes that your errors weren't recovered or handled midway.

Promise.reject('foo').then(v => console.log(v), e => console.log(e)).catch(e => console.log(e));
> foo // from `then`

In the code above, the Promise failed. However, it had a chance to recover via then's reject handler. Thus, the promise returned after that is actually resolved rather than rejected, causing catch to not execute. To log those recoveries, do a console.log at the recovery site instead, and not rely on catch.

As for the rest of your code

I suggest you generate your query using template strings. That way, you avoid ugly string concatenation. Note that template strings use backticks (``).

I also suggest you just create a connection and leave it open. There is some overhead in creating a connection, you'd want to avoid that especially when you hit the DB hard.

Your MySQL driver appears to already use Promises. No need to wrap the entire thing in a Promise. Simply return the object created by the API.

// Assuming connection is a promise from a connect()
return connection.then(() => request.query(query)).catch(err => {
    console.log(err);
    throw err;
});