Shared MSSQL-CN connection pool is torn down prematurely and on every transient error — two related bugs

[tkopitsc]

Summary

When multiple MSSQL query nodes share a single MSSQL-CN connection config node, the underlying pool gets closed far too aggressively due to two separate bugs. In production this causes ConnectionError: Connection is closed (ECONNCLOSED) and tarn "aborted" / "operation timed out for an unknown reason" errors to cascade across all nodes sharing a connection whenever only one of them has a problem.

Bug 1 — connectedNodes reference counter is never populated

connection() (the MSSQL-CN node) declares a reference counter meant to track how many query nodes are using the shared pool:

node.connectedNodes = [];
...
node.disconnect = function (nodeId) {
    const index = node.connectedNodes.indexOf(nodeId);
    if (index >= 0) {
        node.connectedNodes.splice(index, 1);
    }
    if (node.connectedNodes.length === 0) {
        node.connectionCleanup();
    }
};

(src/mssql.js, around lines 323 and 454-461)

Every MSSQL query node calls mssqlCN.disconnect(node.id) on close:

node.on('close', function () {
    mssqlCN.disconnect(node.id);
});

However, nothing ever pushes a node id into connectedNodes — there's no corresponding mssqlCN.connectedNodes.push(node.id) anywhere in the query node's setup (function mssql(config)). This means connectedNodes.length is always 0, so if (node.connectedNodes.length === 0) is always true.

Impact: the very first query node to close (e.g. during a partial/"modified nodes" redeploy) immediately closes the entire shared pool via connectionCleanup(), even while sibling query nodes on the same MSSQL-CN are still executing queries. This aborts their in-flight requests via tarn:

Error: aborted
    at PendingOperation.abort (tarn/dist/PendingOperation.js:25:21)
    at tarn/dist/Pool.js:208:25

Suggested fix: register each query node with the config node on creation:

function mssql(config) {
    RED.nodes.createNode(this, config);
    const mssqlCN = RED.nodes.getNode(config.mssqlCN);
    const node = this;

    if (mssqlCN && Array.isArray(mssqlCN.connectedNodes) && mssqlCN.connectedNodes.indexOf(node.id) === -1) {
        mssqlCN.connectedNodes.push(node.id);
    }
    ...

Bug 2 — any single pool error tears down the entire shared pool

node.pool = new sql.ConnectionPool(node.config);
node.pool.on('error', err => {
    node.error(err);
    node.connectionCleanup();
});

(src/mssql.js, around line 344)

mssql's ConnectionPool.acquire() re-emits any rejected acquire (including a single tarn acquire/create timeout on one connection) as a pool-level 'error' event:

// mssql/lib/base/connection-pool.js
acquire (requester, callback) {
  const acquirePromise = shared.Promise.resolve(this._acquire()).catch(err => {
    this.emit('error', err)
    throw err
  })
  ...

So a single transient timeout/hiccup on one connection immediately triggers connectionCleanup(), force-closing every connection in the shared pool — turning a one-off, recoverable event into an outage for every node using that config node. This is unnecessary: node.execSql() already resets node.poolConnect = null in its own catch block on a per-query basis, so individual failed queries already recover correctly without needing a full pool teardown.

Suggested fix: don't force-close the whole pool on a generic pool error — just log/report it and let tarn/mssql self-heal the individual connection:

node.pool.on('error', err => {
    node.error(err);
});

Reproduction

1. Create one MSSQL-CN config node and 2+ MSSQL query nodes referencing it.
2. Trigger queries on all of them concurrently, then redeploy (modified nodes) so only one query node restarts.
3. Observe the pool closing and in-flight queries on the other, non-restarted nodes failing with "aborted".
4. Separately: simulate one connection timing out (e.g. firewall/network blip) while others are healthy — observe all connections on that MSSQL-CN reset simultaneously.

Environment

• node-red-contrib-mssql-plus: 0.13.1
• node-red: 4.1.8
• Node.js: 20/22

Fix available

We've verified both fixes locally (via a patch-package patch) and confirmed they resolve the premature-teardown behavior without regressing normal reconnect/error handling. Happy to open a PR with these two changes if useful.