Transaction.begin() and PreparedStatement.prepare() skip the connected guard, emitting a spurious pool 'error' with an inconsistent code

[dhensby]

Summary

Request short-circuits with ECONNCLOSED when the pool is not open. Transaction.begin() and PreparedStatement.prepare() do not — they call parent.acquire() regardless, which rejects ENOTOPEN and emits an 'error' event on the pool for what is purely a local API misuse.

Reproduced on mssql@12.7.1, Node v24.18.0, against a real SQL Server. No stubbing, public API only.

This does not crash the process — see "What does not happen" below. It is an API-consistency and observability issue, not a stability one.

Cause

Guarded — lib/base/request.js lines 334, 413, 559, 657 (one per _query/_execute/_batch/_bulk):

if (!this.parent.connected) {
  return setImmediate(callback, new ConnectionError('Connection is closed.', 'ECONNCLOSED'))
}

Unguarded — these call parent.acquire() with no connected check:

• lib/tedious/transaction.js:43
• lib/msnodesqlv8/transaction.js:28
• lib/base/prepared-statement.js:247

ConnectionPool.acquire() (lib/base/connection-pool.js:386-390) emits on any _acquire() rejection:

return shared.Promise.resolve(this._acquire()).catch(err => {
  this.emit('error', err)
  throw err
})

and _acquire() rejects ENOTOPEN whenever !this.pool — which is the state for the whole connect window, and again after close().

Reproduction

const sql = require('mssql')
const config = { server: '127.0.0.1', port: 1433, user: 'sa', password: '<password>',
  database: 'master', options: { encrypt: false, trustServerCertificate: true } }

;(async () => {
  for (const kind of ['request', 'transaction', 'prepared']) {
    const pool = new sql.ConnectionPool(config)
    pool.on('error', e => console.log(`   [${kind}] pool 'error' EVENT: ${e.code}`))
    const connecting = pool.connect(); connecting.catch(() => {})
    try {
      if (kind === 'request') await pool.request().query('select 1')
      if (kind === 'transaction') await new sql.Transaction(pool).begin()
      if (kind === 'prepared') await new sql.PreparedStatement(pool).prepare('select 1')
    } catch (e) { console.log(`   [${kind}] rejected: ${e.code}`) }
    await connecting.catch(() => {}); await pool.close().catch(() => {})
  }
})()

Output:

   [request] rejected: ECONNCLOSED
   [transaction] pool 'error' EVENT: ENOTOPEN
   [transaction] rejected: ENOTOPEN
   [prepared] pool 'error' EVENT: ENOTOPEN
   [prepared] rejected: ENOTOPEN

Call on a pool that is not open	Rejects with	'error' emitted on pool
pool.request().query(...)	ECONNCLOSED "Connection is closed."	no
new sql.Transaction(pool).begin()	ENOTOPEN "Connection not yet open."	yes
new sql.PreparedStatement(pool).prepare(...)	ENOTOPEN "Connection not yet open."	yes

Identical results in the mid-connect window and after pool.close() — the second is a stable state, not a race, so this is reachable without any concurrency.

Impact

1. Spurious pool-level 'error' events. pool.on('error', ...) is the documented "something is wrong with the connection" channel, commonly wired to alerting or reconnect logic. Starting a transaction slightly too early, or after close() during shutdown, fires it even though nothing happened at the network level. The caller also gets the rejection, so it is reported twice.
2. Inconsistent codes for the same condition. Code branching on err.code === 'ECONNCLOSED' to retry-after-reconnect silently misses the transaction and prepared-statement paths.
3. Minor sharp edge: because the emit is inside a promise .catch(), an 'error' listener that itself throws silently replaces the reason begin()/prepare() reject with, masking the real error from the caller.

What does not happen

Stating this explicitly, since it bounds the severity:

• It does not crash. Tested with zero 'error' listeners and no uncaughtException handler: all three paths exit 0 and run to completion. The emit sits inside a promise .catch(), so Node's unhandled-'error' throw is absorbed into the promise chain and resurfaces as the same rejection.
• The call always rejects with a usable ConnectionError. No hung promise, no unhandled rejection.

That said, the no-crash behaviour is incidental to where the emit happens rather than a guarantee — a refactor moving the emit out of the promise chain would turn this into a crash.

Suggested fix

Add the same guard Request already uses, before parent.acquire() in the three call sites:

if (!this.parent.connected) {
  return setImmediate(callback, new ConnectionError('Connection is closed.', 'ECONNCLOSED'))
}

That also makes the error code consistent across all three entry points.

Workaround

Await the pool before constructing a Transaction or PreparedStatement, or guard explicitly with the public pool.connected. Treat ECONNCLOSED and ENOTOPEN as the same class in retry logic, and keep pool.on('error') handlers free of anything that can throw.

Possibly related

#1636 reports ENOTOPEN reaching the top level; that trace is from a Request path on an older version, so it may well be unrelated — noting it only in case it is useful context.

Environment

mssql 12.7.1 · tedious 20.0.0 · Node v24.18.0 · macOS arm64 · Azure SQL Edge 15.0.2000.1574 in Docker. lib/msnodesqlv8/transaction.js:28 was not executed (Windows-only driver); it is structurally identical and shares the same ConnectionPool.acquire(), so that one is inferred rather than observed.