5.6.0: pausing on the last row of a result set's final batch skips the next result set #480

#470 fixed pausing part way through a result set's last, short batch. One case is left: when the pause lands on the last row of that batch, the reader moves on to the next result set twice, and the result set in between is never read. No error is raised, and done / free follow as normal.

Repro

5.6.0 on Node 22, and 5.2.1 on Node 20 (same result). Windows, SQL Server 2022, Native Client 11. The connection is usable afterwards.

const sql = require('msnodesqlv8')

const connectionString = 'Driver={SQL Server Native Client 11.0};Server=...;Database=master;Uid=...;Pwd=...;'

const QUERY = 'select top 51 object_id as a from sys.all_objects; select top 51 object_id as b from sys.all_objects'

function run (conn, pauseAtRow) {
  return new Promise((resolve, reject) => {
    const q = conn.query(QUERY)
    const meta = []
    const rowsPerResultSet = []
    let rows = 0
    q.on('meta', m => { meta.push(m.map(c => c.name).join()); rowsPerResultSet.push(0) })
    q.on('error', reject)
    q.on('row', () => {
      rowsPerResultSet[rowsPerResultSet.length - 1]++
      if (++rows === pauseAtRow) {
        q.pauseQuery()
        setTimeout(() => q.resumeQuery(), 50)
      }
    })
    q.on('free', () => resolve({ meta, rows, rowsPerResultSet }))
  })
}

sql.open(connectionString, async (err, conn) => {
  if (err) throw err
  for (const pauseAtRow of [50, 51, 52]) {
    console.log(`pause at row ${pauseAtRow}:`, JSON.stringify(await run(conn, pauseAtRow)))
  }
  conn.close(() => {})
})

Every run should read 102 rows, 51 from each result set. Pausing on row 51 reads none of the second:

pause at row 50: {"meta":["a","b"],"rows":102,"rowsPerResultSet":[51,51]}
pause at row 51: {"meta":["a","b"],"rows":51,"rowsPerResultSet":[51,0]}
pause at row 52: {"meta":["a","b"],"rows":102,"rowsPerResultSet":[51,51]}

Row 51 is the only row of the first result set's last batch. Pausing on row 50 (the end of a full batch) or row 52 (part way through a batch) is fine.

Cause

dispatch() fetches a batch and hands it over in one loop. The guard added in #470 (reader.js L403-L407) stops only while rows of the batch are still to come. Paused on the batch's last row, dispatch() goes on (L408-L415): the batch has endOfRows, so it calls nextResult(). moveToNextResult() then emits the next result set's meta and calls dispatch(), which returns because the query is paused. batchData is still the previous result set's last batch.

On resumeQuery(), resume() (L490-L500) finds that stale batch fully handed over, with endOfRows set, and calls nextResult() a second time. That moves past the result set whose meta was just emitted, without reading any of its rows.

On the last result set, the same path means resume() calls moveToNextResult(this.batchData) again after the query has ended. I have only found this by reading the code, so I don't know what the second queryHandler.end() does.

A batch that dispatch() fetches is handed over in one synchronous loop. A row handler that pauses therefore pauses on the batch's last row whenever that batch has one row (a result set of 51, 101, ... rows). Back pressure that pauses on every row, as Node.js streams do, hits it every time. That is how it showed up in tediousjs/node-mssql, where piped streams lost whole result sets.

Possible fix

resume() should move on from batchData only when dispatch() hasn't already done so. It needs to remember that it has: nextResult() is asynchronous, so resume() can run before moveToNextResult() does.

Stopping in dispatch() when paused at the end of a batch with endOfRows would also fix it, and leave resume() to move on. But then pausing on the very last row would no longer complete, which #470 says pause.test.js relies on.