node-mssql multiple prepared statements with single connection

Question

I'm banging my head against this library. I tried to setup a singleton intermediary class that would start the start the connection and then share that connection via static methods. My problem is that I'm having difficulties to setup things so that the connection is already open when it's time to run queries, but without having to reopen it again. Since the opening of the connection is of course async, I cannot just put everything in the opening callback, because that happens totally somewhere else at another time... the only thing I can do is to share the mssql.Connection, which is "connecting": true. That's why I don't do connection.connect() in Database.connect()

How can I open the connection and move on to prepare statements and run queries knowing that the connection is open?

My problem is that whenever my code reaches connection.connect() from the second time on, it will hit the error EALREADYCONNECTING, because the connection is already being opened.

I thought about doing some sort of Promise pool of queries to be resolved once connect itself get resolved via a Promise, but right now my brain is very confused!

let mssql = require('mssql');
let fs = require('fs');

class Database
{

  static connect(username, password, server, database)
  {
    if (Database.connection !== null) {
      return Database.connection;
    }

    let storedUsername = null;
    let storedPassword = null;
    let storedServer = null;
    let storedDatabase = null;

    try {
      fs.accessSync(__dirname + '/../../config.json');

      let data = fs.readFileSync(__dirname + '/../../config.json')
      data = JSON.parse(data);
      storedUsername = data.sql.username;
      storedPassword = data.sql.password;
      storedServer = data.sql.server;
      storedDatabase = data.sql.database;

    } catch (e) {
      // Do nothing
    }

    var config = {
      user: username || storedUsername || '',
      password: password || storedPassword || '',
      server: server || storedServer || 'localhost',
      database: database || storedDatabase || '',
    }

    Database.connection = new mssql.Connection(config);

    return Database.connection;
  }

  static getConnection()
  {
    if (Database.connection === null) {
      try {
        Database.connect();
      } catch (e) {
        throw new Error('Database.getConnection: Database not connected.');
      }
    }

    return Database.connection;
  }

  static getInstance()
  {
    return mssql;
  }

  static query(query, fields)
  {
    if (typeof query !== 'string' || typeof fields !== 'object') {
      throw new Error("Invalid parameters");
    }

    let db = Database.getInstance();
    let connection = Database.getConnection();
    let ps = new db.PreparedStatement(connection);
    let values = {};

    fields.forEach(function(current, index) {
      ps.input(current.name, current.type);
      values[current.name] = current.value;
    });

    connection.connect(function(err) {
      if (err) {
        throw err;
      }

      ps.prepare(query, function(err) {
        if (err) {
          throw new Error(err);
        }

        ps.execute(values, function(err, recordset, affected) {
          if (err) {
            ps.unprepare(function(err) {
              if (err) {
                throw new Error(err);
              }
            });
            throw new Error(err);
          }

          ps.unprepare(function(err) {
            if (err) {
              throw new Error(err);
            }
          });
        });
      });
    });
  }
}

Database.connection = null;

module.exports = Database;

Answer 1

Not quite sure if the pattern you are following would be useful with node.js it would be great for for non event driven programming but to make it work with node.js you need to loop as suggested in the comments. That just defeats the purpose.

The second point is that you are essentially creating a wrapper for the mssql class which adds another layer of complexity, possibly introduces bugs and makes maintenance a lot harder. The next person who works on this code will know mssql but will not know the class you are creating and the work arounds that you have had to implement to make it work.

The best way to use one connection would be to put all your queries in the on connected call back

try {
  fs.accessSync(__dirname + '/../../config.json');

  let data = fs.readFileSync(__dirname + '/../../config.json')
  data = JSON.parse(data);
  storedUsername = data.sql.username;
  storedPassword = data.sql.password;
  storedServer = data.sql.server;
  storedDatabase = data.sql.database;

} catch (e) {
  // Actually you must do something here. If nothing else
  // at least log it so that later on you are not left wondering 
  // why nothing seems to work.
}

var config = {
  user: username || storedUsername || '',
  password: password || storedPassword || '',
  server: server || storedServer || 'localhost',
  database: database || storedDatabase || '',
}

Database.connection = new mssql.Connection(config);
connection.connect(function(err) {
    // do everything here    
});

Answer 2

It's not really mentioned in the module documentation, but you can listen to connection events. So if you'd like to keep the structure organized and avoid repetitions, you can listen on the Connection for the connect event. That seems to be perfect answer for me.

let mssql = require('mssql');
let fs = require('fs');

class Database
{

  static connect(username, password, server, database)
  {
    if (Database.connection !== null) {
      return Database.connection;
    }

    let storedUsername = null;
    let storedPassword = null;
    let storedServer = null;
    let storedDatabase = null;

    try {
      fs.accessSync(__dirname + '/../../config.js');

      let config = require(__dirname + '/../../config')

      storedUsername = config.sql.username;
      storedPassword = config.sql.password;
      storedServer = config.sql.server;
      storedDatabase = config.sql.database;

    } catch (err) {
      console.log(err);
    }

    let configuration = {
      user: username || storedUsername || '',
      password: password || storedPassword || '',
      server: server || storedServer || 'localhost',
      database: database || storedDatabase || '',
    }

    Database.connection = new mssql.Connection(configuration);

    Database.connection.connect();
  }

  static disconnect()
  {
    Database.connection.close();
  }

  static getConnection()
  {
    if (Database.connection === null) {
      try {
        Database.connect();
      } catch (e) {
        throw new Error('Database.getConnection: Database not connected.');
      }
    }

    return Database.connection;
  }

  static getInstance()
  {
    return mssql;
  }

  static query(query, fields)
  {
    if (typeof query !== 'string' || typeof fields !== 'object') {
      throw new Error("Invalid parameters");
    }

    let db = Database.getInstance();
    let connection = Database.getConnection();
    let ps = new db.PreparedStatement(connection);
    let values = {};

    fields.forEach(function(current, index) {
      ps.input(current.name, current.type);
      values[current.name] = current.value;
    });

    connection.on('connect', function(err) {
      if (err) {
        throw err;
      }

      ps.prepare(query, function(err) {
        if (err) {
          throw new Error(err);
        }

        ps.execute(values, function(err, recordset, affected) {
          if (err) {
            ps.unprepare(function(err) {
              if (err) {
                throw new Error(err);
              }
            });
            throw new Error(err);
          }

          ps.unprepare(function(err) {
            if (err) {
              throw new Error(err);
            }
          });
        });
      });
    });
  }
}

Database.connection = null;

module.exports = Database;

Now maybe I should Promise-fy those callbacks in the query() method.