Node-mssql: Concurrent requests results in 'Connection is closed' error for one of the requests.

[dw1284]

SQL Server Version: Azure SQL
nodes-mssql Version: 3.3.0

When executing two different requests concurrently, one request has a good chance of throwing a 'Connection is closed' error. Sometimes Request A throws the error, other times Request B throws the error. A very small percentage of the time, they will both complete successfully.

My theory, based on no knowledge whatsoever, is that the below race condition (or something similar) could be taking place:

1. Request A starts a new connection
2. Request B tries to start a new connection, but instead piggy-backs off of Request A's already open connection (speculation)
3. Request A makes a call to its stored proc
4. Request B makes a call to its stored proc
5. Request A finishes first and closes the connection (speculation)
6. Request B errs out before finishing because request A already closed the connection

// To replicate, create a test app and require this module. Start your
// test app over and over and you will see that most of the time,
// you get a 'connection is closed' error on one of the requests.

// Might have to use Azure SQL with Encrypt=true to replicate.
// I haven't tried it with regular SQL Server.

const promise = require('bluebird');
const dbConfig = require('config').get('dbConfig');

// Request A
this.getStatistics = function () {
  let sql = require('mssql');
  let storedProcName = 'GetStatistics'

  return new promise(function (resolve, reject) {
    sql.connect(dbConfig).then(function () {
      new sql.Request()
        .execute(storedProcName).then(function (recordset) {
          if (recordset.length > 0 && recordset[0].length === 1) {
            resolve(recordset[0][0]);
          } else {
            resolve(null);
          }
        }).catch (function (err) {
          reject(err.originalError || err);
        });
    }).catch(function (err) {
      reject(err.originalError || err);;
    });
  });
};

// Request B
this.getResults = function () {
  let sql = require('mssql');
  let storedProcName = 'GetResults'

  return new promise(function (resolve, reject) {
    sql.connect(dbConfig).then(function () {
      new sql.Request()
        .execute(storedProcName).then(function (recordset) {
          if (recordset.length > 0 && recordset[0].length === 1) {
            resolve(recordset[0][0]);
          } else {
            resolve(null);
          }
        }).catch (function (err) {
          reject(err.originalError || err);
        });
    }).catch(function (err) {
      reject(err.originalError || err);;
    });
  });
};

this.getStatistics().then(function () {
  console.log('Request A Successful')
}).catch(function (err) {
  console.log(err)
});

this.getResults().then(function () {
  console.log('Request B Successful')
}).catch(function (err) {
  console.log(err)
});

module.exports = this;

[dw1284]

I figured out a solution. I was debugging mssql to figure out why the connections weren't mapping back to the originating request properly, and I found out that there is some logic that attempts to guess which connection to use unless you pass in your connection object to the new Request() function. If you do NOT provide your connection as an argument, which the documentation encourages you not to I would argue, the module will get its wires crossed half the time and choose an unopened connection as opposed to the one it should use.

I got around this specifically telling the Request() function which connection to use.

The documentation for multiple connections is buried underneath a bunch of other examples, and I didn't even know I needed to handle multiple connections to be honest. My fault mainly I guess, but I would seriously encourage anybody using this module to pass in the connection param....even if you aren't anticipating the need to juggle multiple connections. You will almost certainly end up with multiple connections when you have two users hit your API or website at the same time. There really is no downside to passing in your connection object as a param, in fact, I would update the documentation so that all examples pass it in. Just my opinion.

Anyway, here is the solution for anybody else who may run into this problem:

Changed from:

sql.connect(dbConfig).then(function () {
      new sql.Request()
        .execute(storedProcName).then(function (recordset) {

Changed to:

sql.connect(dbConfig).then(function (connection) {
      new sql.Request(connection)
        .execute(storedProcName).then(function (recordset) {

[Eric2XU]

Fixed it for me too... thanks @dw1284