From 70199c3d9d03d7e5387893cb4ea57d4ed0de1608 Mon Sep 17 00:00:00 2001 From: Dan Hensby Date: Thu, 15 Oct 2026 18:33:40 +0100 Subject: [PATCH 1/4] fix: stop msnodesqlv8 crashing when a stream held before its first result set is cancelled A stream paused before its query reaches a result set, as toReadableStream() leaves one until it is read, holds msnodesqlv8's query paused. The query still runs the statements ahead of that result set, which have none, and msnodesqlv8 cancels a paused query by freeing its statement. Once the server has sent the row count of one of those statements, msnodesqlv8 is reading on to the next result when that free lands, which can crash the process. It did so in nearly every run with each msnodesqlv8 version tried, 2.7 to 5.6.0. A held query is now cancelled once it stops at its first result set, where it has nothing under way, and one that ends first completes as cancelled. Such a cancel no longer cuts short the statements ahead of that result set: it takes effect when they end, or when the request times out. With no request timeout, nothing limits the wait. msnodesqlv8 6 fixes the crash itself, and emits nothing for a paused query until it is resumed, so there a held query is cancelled at once, as before. Co-Authored-By: Claude Opus 5.5 --- README.md | 6 +- lib/msnodesqlv8/request.js | 37 ++++- test/common/tests.js | 53 ++++++ test/common/unit.js | 280 +++++++++++++++++++++++++++++++- test/msnodesqlv8/msnodesqlv8.js | 1 + test/tedious/tedious.js | 1 + 6 files changed, 367 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 61d01e7c..644840cf 100644 --- a/README.md +++ b/README.md @@ -1281,7 +1281,11 @@ request.cancel() A cancelled stream emits an `error` event with code `ECANCEL`, followed by `done`. -With the `msnodesqlv8` driver, a request that is already running can only be cancelled from msnodesqlv8 5.6. Earlier versions can cancel a paused stream, but let any other running request run to the end and complete as if `cancel()` had not been called. A paused stream is cancelled once the driver stops reading, at the end of a batch of 50 rows or the start of the next result set; a query that ends first, or times out, completes as cancelled. +With the `msnodesqlv8` driver, a request that is already running can only be cancelled from msnodesqlv8 5.6. Earlier versions can cancel a paused stream, but let any other running request run to the end and complete as if `cancel()` had not been called. A paused stream is cancelled once the driver stops reading, at the end of a batch of 50 rows or the start of the next result set. + +Before msnodesqlv8 6, that includes a stream paused before its query reaches its first result set, as `toReadableStream()` leaves one until it is read. Cancelling it while msnodesqlv8 is still working through the statements ahead of that result set can crash the process, so the cancel waits for them: they run to the end and keep their effects, however long they take. The request timeout ends the wait if it is at least a second. With `requestTimeout: 0`, or anything under a second, which msnodesqlv8 takes as no timeout, nothing ends it. msnodesqlv8 6 cancels such a stream at once. + +A query that ends before the cancel takes effect, or times out, completes as cancelled, after emitting any error it ended with. ## Transaction diff --git a/lib/msnodesqlv8/request.js b/lib/msnodesqlv8/request.js index cfa11783..c872569d 100644 --- a/lib/msnodesqlv8/request.js +++ b/lib/msnodesqlv8/request.js @@ -10,6 +10,18 @@ const { PARSERS: UDT } = require('../udt') const Table = require('../table') const { valueHandler } = require('../shared') +// msnodesqlv8 6 emits nothing for a paused query until it is resumed, 'meta' and 'done' included, +// and ends a paused query it cancels once nothing is under way, without freeing its statement +// first. What the request does for earlier versions while a query is held back would wait for +// those events forever +const driverHoldsPausedQueries = () => { + try { + return Number(require('msnodesqlv8/package.json').version.split('.')[0]) >= 6 + } catch (err) { + return false + } +} + const JSON_COLUMN_ID = 'JSON_F52E2B61-18A1-11d1-B105-00805F49916B' const XML_COLUMN_ID = 'XML_F52E2B61-18A1-11d1-B105-00805F49916B' const EMPTY_BUFFER = Buffer.alloc(0) @@ -403,6 +415,8 @@ class Request extends BaseRequest { let driverPaused = false // a cancel of a paused stream waits for the driver to reach a point where it can be paused let cancelDeferred = false + // the cancel waits for a query held back before its first result set + let cancelBeforeResults = false // msnodesqlv8 has freed the statement, which it does without 'done' after an error ends the query let freed = false let resuming = false @@ -626,13 +640,18 @@ class Request extends BaseRequest { const pauseAndCancelDriver = () => { cancelDeferred = false // msnodesqlv8 5.0 to 5.5 send a query one cancel, which a cancel made before the query - // started has used up and they refused, so the query runs on + // started has used up and they refused, so the query runs on. A query held back before its + // first result set was paused when that cancel went out, as it is when msnodesqlv8 cancels + // it itself on a timeout, so there the cancel went through and the query is ending if (typeof req.getCancelSent === 'function' && req.getCancelSent()) { + if (cancelBeforeResults) return true cancelRequested = false return false } - driverPaused = true - req.pauseQuery() + if (!driverPaused) { + driverPaused = true + req.pauseQuery() + } cancelDriver() return true } @@ -696,6 +715,18 @@ class Request extends BaseRequest { } cancelRequested = true cancelAttempts++ + // A query held back before its first result set still runs the statements ahead of it, + // which have none. msnodesqlv8 < 6 cancels a paused query by freeing its statement (5.6 not + // while the first statement is still executing), and a free that lands while msnodesqlv8 + // reads on to the next result can crash the process. Nothing tells the request when the + // query moves on, so a held query is cancelled once it stops at its first result set, and + // one that ends first is reported as cancelled. The statements ahead run to their end, or + // to the request timeout. msnodesqlv8 6 cancels it at once. + if (driverPaused && !columns && !driverHoldsPausedQueries()) { + cancelDeferred = true + cancelBeforeResults = true + return + } if (!this.stream || !this.paused || driverPaused) { return cancelDriver() } diff --git a/test/common/tests.js b/test/common/tests.js index baf36584..2069c47d 100644 --- a/test/common/tests.js +++ b/test/common/tests.js @@ -3219,6 +3219,59 @@ module.exports = (sql, driver) => { }).catch(done) }, + 'cancelling a stream held back while its query moves on to its first result set' (done) { + // a stream paused before its query runs, as toReadableStream() leaves it, and cancelled once + // msnodesqlv8 has moved on from a statement with no result set: the server has sent that + // statement's row count, so msnodesqlv8 is reading on when the cancel arrives, and freeing + // the statement then can crash the process. With one connection in the pool, the query that + // follows the stream runs on the connection the stream had + const config = readConfig() + config.driver = driver + config.pool = { max: 1, min: 0 } + let timer + let held = false + + const pool = new sql.ConnectionPool(config) + pool.connect().then(() => new Promise((resolve, reject) => { + const errors = [] + const rows = [] + const req = new sql.Request(pool) + req.stream = true + req.on('error', err => errors.push(err.code)) + req.on('row', row => rows.push(row)) + timer = setTimeout(() => { + held = true + reject(new Error('the cancelled stream should complete')) + }, 10000) + req.once('done', () => { + clearTimeout(timer) + resolve({ errors, rows }) + }) + req.pause() + req.query('set nocount off; declare @t table (a int); insert @t values (1); raiserror(\'flush\', 0, 1) with nowait; waitfor delay \'00:00:02\'; select 1 as a') + setTimeout(() => req.cancel(), 500) + })).then(({ errors, rows }) => { + assert.deepStrictEqual(errors, ['ECANCEL'], 'the stream should end as cancelled') + assert.deepStrictEqual(rows, [], 'no row should be streamed') + return new Promise((resolve, reject) => { + timer = setTimeout(() => { + held = true + reject(new Error('the next query on the connection should run')) + }, 5000) + pool.request().query('select 1 as a').then(resolve, reject) + }) + }).then(result => { + clearTimeout(timer) + assert.deepStrictEqual(result.recordset, [{ a: 1 }], 'the next query should return its result') + return pool.close() + }).then(() => done(), err => { + clearTimeout(timer) + // a connection that is held up cannot be closed either, so the pool is left as it is + if (held) return done(err) + pool.close().then(() => done(err), () => done(err)) + }) + }, + 'pausing and resuming a stream once it is over' (ending, done) { // msnodesqlv8 does not check that a query is still running when it is paused or resumed, // and can put one that has ended back on its connection's work queue, where it holds up diff --git a/test/common/unit.js b/test/common/unit.js index f91730a8..8163e2b8 100644 --- a/test/common/unit.js +++ b/test/common/unit.js @@ -2002,6 +2002,9 @@ describe('msnodesqlv8 query events', () => { // reaches into it for TVPs, so a bare object stands in for it, and each test plays the // events a version of it emits on the query it is given. const STUB = 'msnodesqlv8-stub' + // the version of msnodesqlv8 the request is told it runs on + const STUB_PACKAGE = 'msnodesqlv8-stub-package' + const driverPackage = { version: '5.6.0' } const REQUEST = require.resolve('../../lib/msnodesqlv8/request') let resolveFilename let Request @@ -2009,18 +2012,27 @@ describe('msnodesqlv8 query events', () => { before(() => { resolveFilename = Module._resolveFilename Module._resolveFilename = function (request, ...rest) { - return request === 'msnodesqlv8' ? STUB : resolveFilename.call(this, request, ...rest) + if (request === 'msnodesqlv8') return STUB + if (request === 'msnodesqlv8/package.json') return STUB_PACKAGE + return resolveFilename.call(this, request, ...rest) } require.cache[STUB] = { id: STUB, filename: STUB, loaded: true, exports: {} } + require.cache[STUB_PACKAGE] = { id: STUB_PACKAGE, filename: STUB_PACKAGE, loaded: true, exports: driverPackage } Request = require(REQUEST) }) after(() => { Module._resolveFilename = resolveFilename delete require.cache[STUB] + delete require.cache[STUB_PACKAGE] delete require.cache[REQUEST] }) + afterEach(() => { + driverPackage.version = '5.6.0' + require.cache[STUB_PACKAGE] = { id: STUB_PACKAGE, filename: STUB_PACKAGE, loaded: true, exports: driverPackage } + }) + const odbcError = (message, code, sqlstate) => Object.assign(new Error(message), { code, sqlstate }) const PRIMARY_KEY = () => odbcError('Violation of PRIMARY KEY constraint \'PK_t\'.', 2627, '23000') const TERMINATED = () => odbcError('The statement has been terminated.', 3621, '01000') @@ -2814,8 +2826,8 @@ describe('msnodesqlv8 query events', () => { // unless it was paused on the batch's last row. `timeOutAtFetch` names a fetch that does not // come back before the request times out, and `failAtEnd` ends the query on an error; a query // that ends on an error emits 'free' and no 'done', as msnodesqlv8 does. `beginDelay` holds - // back the end of the first statement, and `stallBegin` keeps it from ending. - const reader = (version, recordsets, { timeOutAtFetch, failAtEnd, beginDelay, stallBegin } = {}) => { + // back the end of the first statement. + const reader = (version, recordsets, { timeOutAtFetch, failAtEnd, beginDelay } = {}) => { const pausedAt = [] const calls = [] let notify = null @@ -2958,7 +2970,7 @@ describe('msnodesqlv8 query events', () => { cancelSent = true sendCancel(callback) } - if (stallBegin || !running) return + if (!running) return notify.emit('meta', meta()) dispatch() } @@ -3324,8 +3336,10 @@ describe('msnodesqlv8 query events', () => { assert.strictEqual(rows.length, 10, 'no row should be emitted after the stream was paused') }) - it('should cancel a paused stream whose first statement does not end', async () => { - const { request, calls, queried } = setupReader('5.6', [['a', 75]], { stallBegin: true }) + // 2.x to 4.x send a cancel as soon as it is made, where 5.x hold one made before the query has a + // statement until it has one + it('should cancel a paused stream whose first statement is still running once the statement ends', async () => { + const { request, calls, fetches, queried, pool } = setupReader('2.x to 4.x', [['a', 75]], { beginDelay: 20 }) request.stream = true const errors = [] request.on('error', err => errors.push(err.code)) @@ -3335,9 +3349,13 @@ describe('msnodesqlv8 query events', () => { while (!queried()) await tick() request.pause() request.cancel() + const running = calls.slice() await completed - assert.deepStrictEqual(calls, ['pause', 'cancel'], 'the driver should be paused and cancelled at once') + assert.deepStrictEqual(running, ['pause'], 'the query should not be cancelled while its first statement runs') + assert.deepStrictEqual(calls, ['pause', 'cancel'], 'the query should be cancelled where it stops, at its first result set') + assert.strictEqual(fetches(), 0, 'the driver should fetch nothing') assert.deepStrictEqual(errors, ['ECANCEL'], 'the cancel should be streamed as an error') + assert.strictEqual(pool.released, 1, 'the connection should be released once') }) it('should cancel a stream paused before its first statement ends, after a cancel made while it was flowing', async () => { @@ -4008,6 +4026,254 @@ describe('msnodesqlv8 query events', () => { assert.deepStrictEqual(errors, [3621], 'the termination should be streamed') assert.deepStrictEqual(calls, [], 'the cancel should not reach the query, which had ended') }) + // every version: msnodesqlv8 cancels a paused query by freeing its statement, and a query held + // back before its first result set still runs the statements ahead of it, which have none. A free + // that lands while msnodesqlv8 reads on to the next result can crash the process + const cancellable = (calls, query = {}) => pausable(calls, { + // msnodesqlv8 ends a paused query once its cancel is through + cancelQuery (callback) { + calls.push('cancel') + setImmediate(() => { + this.emit('done') + callback(null) + }) + }, + ...query + }) + + // a held stream that is cancelled should complete without being resumed + const cancelled = (request, run) => { + let timer + return Promise.race([ + completion(request, run), + new Promise((resolve, reject) => { + timer = setTimeout(reject, 1000, new Error('the cancelled stream should complete')) + }) + ]).finally(() => clearTimeout(timer)) + } + + it('should cancel a query held back before its first result set once it stops at one', async () => { + const calls = [] + let running + let movingOn + let atResultSet + const { request, pool } = setup(async notify => { + request.cancel() + running = calls.slice() + // the row count of a statement ahead, and a message that has SQL Server send it at once + notify.emit('rowcount', 1) + notify.emit('info', odbcError('flush', 50000, '01000'), false) + await tick() + movingOn = calls.slice() + notify.emit('meta', [{ name: 'a', sqlType: 'int', size: 4, nullable: false }]) + atResultSet = calls.slice() + }, cancellable(calls)) + request.stream = true + const errors = [] + request.on('error', err => errors.push(err.code)) + // as toReadableStream() pauses a request before it runs it + request.pause() + await cancelled(request, r => r.query('insert into t values (1); raiserror(\'flush\', 0, 1) with nowait; waitfor delay \'00:00:01\'; select a from t')) + assert.deepStrictEqual(running, ['pause'], 'the query should not be cancelled while it runs') + assert.deepStrictEqual(movingOn, ['pause'], 'the query should not be cancelled while it moves on to its result set') + assert.deepStrictEqual(atResultSet, ['pause', 'cancel'], 'the query should be cancelled once it stops at its result set') + assert.deepStrictEqual(errors, ['ECANCEL'], 'the cancel should be streamed as an error') + assert.strictEqual(pool.released, 1, 'the connection should be released once') + }) + + it('should cancel a query held back before it starts once it stops at its first result set', async () => { + const calls = [] + let beforeStart + const { request, pool } = setup(async notify => { + request.cancel() + beforeStart = calls.slice() + // msnodesqlv8 starts a query it has been sent, though it is paused + notify.emit('submitted') + notify.emit('rowcount', 1) + await tick() + notify.emit('meta', [{ name: 'a', sqlType: 'int', size: 4, nullable: false }]) + }, cancellable(calls)) + request.stream = true + const errors = [] + request.on('error', err => errors.push(err.code)) + // as toReadableStream() pauses a request before it runs it + request.pause() + await cancelled(request, r => r.query('update t set a = 1; select a from t')) + assert.deepStrictEqual(beforeStart, ['pause'], 'the query should not be cancelled before it starts, as it goes on to run') + assert.deepStrictEqual(calls, ['pause', 'cancel'], 'the query should be cancelled once it stops at its result set') + assert.deepStrictEqual(errors, ['ECANCEL'], 'the cancel should be streamed as an error') + assert.strictEqual(pool.released, 1, 'the connection should be released once') + }) + + it('should keep a query held back before its first result set paused when the stream is resumed after a cancel', async () => { + const calls = [] + let resumed + const { request, pool } = setup(async notify => { + request.cancel() + // as a readable stream that is read after its request is cancelled resumes the request + request.resume() + resumed = calls.slice() + await tick() + notify.emit('meta', [{ name: 'a', sqlType: 'int', size: 4, nullable: false }]) + }, cancellable(calls)) + request.stream = true + const errors = [] + request.on('error', err => errors.push(err.code)) + // as toReadableStream() pauses a request before it runs it + request.pause() + await cancelled(request, r => r.query('update t set a = 1; select a from t')) + assert.deepStrictEqual(resumed, ['pause'], 'the query should not be resumed while it is being cancelled') + assert.deepStrictEqual(calls, ['pause', 'cancel'], 'the query should be cancelled once it stops at its result set') + assert.deepStrictEqual(errors, ['ECANCEL'], 'the cancel should be streamed as an error') + assert.strictEqual(pool.released, 1, 'the connection should be released once') + }) + + // msnodesqlv8 < 5.6 can give the last error of a statement that fails more=false, and still carry + // on to the statements after it + it('should cancel a query held back before its first result set at that result set when it is cancelled after an error', async () => { + const calls = [] + let afterError + const { request, pool } = setup(async notify => { + notify.fail(PRIMARY_KEY(), false) + request.cancel() + afterError = calls.slice() + await tick() + notify.emit('meta', [{ name: 'a', sqlType: 'int', size: 4, nullable: false }]) + }, cancellable(calls)) + request.stream = true + const errors = [] + request.on('error', err => errors.push(err.code)) + // as toReadableStream() pauses a request before it runs it + request.pause() + await cancelled(request, r => r.query('insert into t values (1); select a from t')) + assert.deepStrictEqual(afterError, ['pause'], 'the query should not be cancelled while it moves on to its result set') + assert.deepStrictEqual(calls, ['pause', 'cancel'], 'the query should be cancelled once it stops at its result set') + assert.deepStrictEqual(errors, ['EREQUEST', 'ECANCEL'], 'the error and then the cancel should be streamed') + assert.strictEqual(pool.released, 1, 'the connection should be released once') + }) + + it('should report a query held back before its first result set that ends without one as cancelled', async () => { + const calls = [] + const { request, pool } = setup(async notify => { + request.cancel() + notify.emit('rowcount', 1) + await tick() + notify.emit('done') + notify.emit('free') + }, cancellable(calls)) + request.stream = true + const errors = [] + request.on('error', err => errors.push(err.code)) + // as toReadableStream() pauses a request before it runs it + request.pause() + await cancelled(request, r => r.query('update t set a = 1')) + assert.deepStrictEqual(calls, ['pause'], 'the cancel should not reach the query, which needed none to end') + assert.deepStrictEqual(errors, ['ECANCEL'], 'the cancel should be streamed as an error') + assert.strictEqual(pool.released, 1, 'the connection should be released once') + }) + + // the statements ahead of the first result set run on, and an error among them is streamed + it('should report a query held back before its first result set that times out first as cancelled, after the timeout', async () => { + const calls = [] + const { request, pool } = setup(async notify => { + request.cancel() + await tick() + notify.fail(odbcError('Query timeout expired', 0, 'HYT00'), false) + setImmediate(() => notify.emit('free')) + }, cancellable(calls)) + request.stream = true + const errors = [] + request.on('error', err => errors.push(err.code)) + // as toReadableStream() pauses a request before it runs it + request.pause() + await cancelled(request, r => r.query('waitfor delay \'00:00:05\'; select a from t')) + assert.deepStrictEqual(calls, ['pause'], 'the cancel should not reach the query, which the timeout ended') + assert.deepStrictEqual(errors, ['EREQUEST', 'ECANCEL'], 'the timeout and then the cancel should be streamed') + assert.strictEqual(pool.released, 1, 'the connection should be released once') + }) + + // msnodesqlv8 5.x sends a query one cancel. A cancel made before the query had a statement goes + // out once it has one, and on Linux with ODBC Driver 17 msnodesqlv8 cancels a query itself on a + // timeout; either way the query held back was paused, so that cancel goes through + it('should report a query held back before its first result set as cancelled when msnodesqlv8 has sent it a cancel by then', async () => { + const calls = [] + let cancelSent = false + const { request, pool } = setup(async notify => { + request.cancel() + cancelSent = true + await tick() + notify.emit('meta', [{ name: 'a', sqlType: 'int', size: 4, nullable: false }]) + await tick() + notify.emit('done') + notify.emit('free') + }, cancellable(calls, { getCancelSent: () => cancelSent })) + request.stream = true + const errors = [] + request.on('error', err => errors.push(err.code)) + // as toReadableStream() pauses a request before it runs it + request.pause() + await cancelled(request, r => r.query('update t set a = 1; select a from t')) + assert.deepStrictEqual(calls, ['pause'], 'msnodesqlv8 should not be sent a second cancel') + assert.deepStrictEqual(errors, ['ECANCEL'], 'the cancel should be streamed as an error') + assert.strictEqual(pool.released, 1, 'the connection should be released once') + }) + + // msnodesqlv8 6 holds a cancelled paused query back itself until nothing is under way, and then + // ends it: the cancel's callback, HY008, 'done' and 'free' + const cancellableOn6 = calls => pausable(calls, { + cancelQuery (callback) { + calls.push('cancel') + setImmediate(() => { + callback(null) + this.fail(odbcError('Operation canceled', 0, 'HY008'), false) + this.emit('done') + this.emit('free') + }) + } + }) + + it('should cancel a query held back before its first result set at once with msnodesqlv8 6', async () => { + driverPackage.version = '6.0.0' + const calls = [] + let running + const { request, pool } = setup(async notify => { + notify.emit('submitted') + await tick() + request.cancel() + running = calls.slice() + }, cancellableOn6(calls)) + request.stream = true + const errors = [] + request.on('error', err => errors.push(err.code)) + // as toReadableStream() pauses a request before it runs it + request.pause() + await cancelled(request, r => r.query('insert into t values (1); raiserror(\'flush\', 0, 1) with nowait; waitfor delay \'00:00:01\'; select a from t')) + assert.deepStrictEqual(running, ['pause', 'cancel'], 'the query should be cancelled at once') + assert.deepStrictEqual(errors, ['ECANCEL'], 'only the cancel should be streamed as an error') + assert.strictEqual(pool.released, 1, 'the connection should be released once') + }) + + it('should cancel a query held back before its first result set once it stops at one when the msnodesqlv8 version is unknown', async () => { + delete require.cache[STUB_PACKAGE] + const calls = [] + let running + const { request, pool } = setup(async notify => { + request.cancel() + running = calls.slice() + await tick() + notify.emit('meta', [{ name: 'a', sqlType: 'int', size: 4, nullable: false }]) + }, cancellable(calls)) + request.stream = true + const errors = [] + request.on('error', err => errors.push(err.code)) + // as toReadableStream() pauses a request before it runs it + request.pause() + await cancelled(request, r => r.query('update t set a = 1; select a from t')) + assert.deepStrictEqual(running, ['pause'], 'the query should not be cancelled while it runs') + assert.deepStrictEqual(calls, ['pause', 'cancel'], 'the query should be cancelled once it stops at its result set') + assert.deepStrictEqual(errors, ['ECANCEL'], 'the cancel should be streamed as an error') + assert.strictEqual(pool.released, 1, 'the connection should be released once') + }) }) describe('pool acquire', () => { diff --git a/test/msnodesqlv8/msnodesqlv8.js b/test/msnodesqlv8/msnodesqlv8.js index 1c12b7a9..6c4bf08c 100644 --- a/test/msnodesqlv8/msnodesqlv8.js +++ b/test/msnodesqlv8/msnodesqlv8.js @@ -316,6 +316,7 @@ describe('msnodesqlv8', function () { it('pausing and resuming a stream once it has completed', done => TESTS['pausing and resuming a stream once it is over']('completed', done)) it('pausing and resuming a stream once an error has ended it', done => TESTS['pausing and resuming a stream once it is over']('failed', done)) it('pausing and resuming a stream once it has been cancelled', done => TESTS['pausing and resuming a stream once it is over']('cancelled', done)) + it('cancelling a stream held back while its query moves on to its first result set', done => TESTS['cancelling a stream held back while its query moves on to its first result set'](done)) after(done => sql.close(done)) }) diff --git a/test/tedious/tedious.js b/test/tedious/tedious.js index 2105fd67..d6a2a4b9 100644 --- a/test/tedious/tedious.js +++ b/test/tedious/tedious.js @@ -345,6 +345,7 @@ describe('tedious', () => { it('pausing and resuming a stream once it has completed', done => TESTS['pausing and resuming a stream once it is over']('completed', done)) it('pausing and resuming a stream once an error has ended it', done => TESTS['pausing and resuming a stream once it is over']('failed', done)) it('pausing and resuming a stream once it has been cancelled', done => TESTS['pausing and resuming a stream once it is over']('cancelled', done)) + it('cancelling a stream held back while its query moves on to its first result set', done => TESTS['cancelling a stream held back while its query moves on to its first result set'](done)) after(done => sql.close(done)) }) From 821c13913b022cd46c5293bb406b5c60be7086bf Mon Sep 17 00:00:00 2001 From: Dan Hensby Date: Sun, 25 Oct 2026 22:18:31 +0300 Subject: [PATCH 2/4] ci: test against msnodesqlv8 v6 on Node 22 and 24 msnodesqlv8 6 requires Node 22 or later, and ships a Node-API binary for win32-x64, so the Windows jobs run its tests on Node 22 and 24 after the v5 run. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/nodejs.yml | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/.github/workflows/nodejs.yml b/.github/workflows/nodejs.yml index 06805b2d..b0b7bc4d 100644 --- a/.github/workflows/nodejs.yml +++ b/.github/workflows/nodejs.yml @@ -162,7 +162,9 @@ jobs: # a combo without a prebuild falls back to a node-gyp source build, which is # brittle on the GitHub Windows images (e.g. the VS 2026 rollout on # windows-2025 that node-gyp cannot detect). Coverage: - # v2: Node 18 v3: Node 18-20 v4: Node 18-24 v5: Node 20-24 + # v2: Node 18 v3: Node 18-20 v4: Node 18-24 v5: Node 20-24 v6: Node 22-24 + # + # msnodesqlv8 6 requires Node 22 or later. # # v5 is split: msnodesqlv8 5.2.2 dropped the Node 20 (ABI 115) win32-x64 # prebuild, so `@^5` is gated to Node 22-24 and Node 20 is pinned to 5.2.1, @@ -201,6 +203,12 @@ jobs: - name: Run msnodesqlv8 v5 tests if: ${{ matrix.node == '22.x' || matrix.node == '24.x' }} run: npm run test-msnodesqlv8 + - name: Install msnodesqlv8 v6 + if: ${{ matrix.node == '22.x' || matrix.node == '24.x' }} + run: npm install --no-save msnodesqlv8@^6 + - name: Run msnodesqlv8 v6 tests + if: ${{ matrix.node == '22.x' || matrix.node == '24.x' }} + run: npm run test-msnodesqlv8 release: name: Release concurrency: release From 3b7ff3f63645680817f0f41221712c8e40e57204 Mon Sep 17 00:00:00 2001 From: Dan Hensby Date: Thu, 15 Oct 2026 08:44:47 +0100 Subject: [PATCH 3/4] fix: stop msnodesqlv8 streams failing when resumed while their query executes msnodesqlv8 fetches as soon as a query is resumed, whatever the query is doing. A stream that is paused before its query reaches a result set, as toReadableStream() leaves one until it is read, pauses the query straight away. Resumed while the query was still executing, or moving on through statements with no result, it had msnodesqlv8 fetch from a statement with nothing to fetch yet. Depending on timing, the stream reported a spurious 'Function sequence error' (HY010), lost the error the query raised, or crashed the process. A query held back once msnodesqlv8 has started it is now resumed only when it has stopped at a result set, a tick after its 'meta', and not at all if it ends without one. One resumed before msnodesqlv8 starts it is resumed at once, as before, so that it starts unpaused. Co-Authored-By: Claude Opus 5.5 --- lib/msnodesqlv8/request.js | 26 ++++++- test/common/tests.js | 31 ++++++++ test/common/unit.js | 125 +++++++++++++++++++++++++++++++- test/msnodesqlv8/msnodesqlv8.js | 2 + test/tedious/tedious.js | 2 + 5 files changed, 181 insertions(+), 5 deletions(-) diff --git a/lib/msnodesqlv8/request.js b/lib/msnodesqlv8/request.js index c872569d..24c5c1c9 100644 --- a/lib/msnodesqlv8/request.js +++ b/lib/msnodesqlv8/request.js @@ -667,16 +667,26 @@ class Request extends BaseRequest { } // until the first result set the driver has read nothing, so a stream paused before then, - // or before its query starts, holds it back at once + // or before its query starts, holds it back at once. But msnodesqlv8's resume() fetches at + // once, whatever the query is doing, and once the query has started ('submitted') it may + // still be executing, or moving on through statements with no result, when the fetch fails + // with a 'Function sequence error' or the result set is fetched twice. So a query held back + // then is resumed only once it has stopped at a result set, a tick after its 'meta', and not + // at all if it ends first. Until msnodesqlv8 starts it, a resume only stops it starting paused + let heldBeforeResults = false + let submitted = false + let atResultSet = false + req.once('submitted', () => { submitted = true }) this._pauseStream = () => { if (!columns && !driverPaused && !finished && reachable()) { driverPaused = true + heldBeforeResults = true req.pauseQuery() } } if (this.stream && this.paused) this._pauseStream() - this._resumeStream = () => { + const resumeStream = () => { // a listener that resumes the stream while it is emitting what was waiting leaves the // rest to this loop, so that every listener gets the events in order if (resuming) return @@ -692,11 +702,13 @@ class Request extends BaseRequest { // stream completes as cancelled straight away, as it does when it is paused first if (finished && cancelRequested && this.paused && pending.length) emitPendingOnCancel() // a query that is being cancelled stays paused, so that the cancel can go through - if (!pending.length && !this.paused && driverPaused && !finished && !cancelRequested && reachable()) { + if (!pending.length && !this.paused && driverPaused && !finished && !cancelRequested && reachable() && + (!heldBeforeResults || !submitted || atResultSet)) { driverPaused = false req.resumeQuery() } } + this._resumeStream = resumeStream const cancel = () => { debug('request(%d): cancel', IDS.get(this)) @@ -756,6 +768,14 @@ class Request extends BaseRequest { // later, once it has stopped at the result set if (!this.paused && driverPaused && !cancelRequested) setImmediate(this._resumeStream) } + // msnodesqlv8 calls dispatch() once 'meta' is handled, which returns while the query is + // paused, so a tick later a query held back before its first result set has stopped there + if (heldBeforeResults && !atResultSet) { + setImmediate(() => { + atResultSet = true + resumeStream() + }) + } if (row) { if (isChunkedRecordset) { diff --git a/test/common/tests.js b/test/common/tests.js index 2069c47d..5f6063d7 100644 --- a/test/common/tests.js +++ b/test/common/tests.js @@ -3272,6 +3272,37 @@ module.exports = (sql, driver) => { }) }, + 'resuming a stream held back while its query executes' (failing, done) { + // a stream paused before its query runs, as toReadableStream() leaves it, and read while + // the query still executes: msnodesqlv8 fetches from the query at once, which can fail with a + // 'Function sequence error' (HY010), lose the error the query raises, or crash the process + const rows = [] + const errors = [] + const req = new sql.Request() + req.stream = true + req.on('row', row => rows.push(row)) + req.on('error', err => errors.push(err)) + req.once('done', () => { + try { + if (failing) { + assert.deepStrictEqual(errors.map(e => e.number), [208], `only the error the query raised should be streamed, not: ${errors.map(e => e.message).join(' / ')}`) + assert.deepStrictEqual(rows, [], 'no row should be streamed') + } else { + assert.deepStrictEqual(errors.map(e => e.message), [], 'no error should be streamed') + assert.deepStrictEqual(rows, [{ a: 1 }], 'the row should be streamed once') + } + done() + } catch (e) { + done(e) + } + }) + req.pause() + req.query(failing + ? 'waitfor delay \'00:00:01\'; select * from notexistingtable' + : 'waitfor delay \'00:00:01\'; select 1 as a') + setTimeout(() => req.resume(), 300) + }, + 'pausing and resuming a stream once it is over' (ending, done) { // msnodesqlv8 does not check that a query is still running when it is paused or resumed, // and can put one that has ended back on its connection's work queue, where it holds up diff --git a/test/common/unit.js b/test/common/unit.js index 8163e2b8..093b9642 100644 --- a/test/common/unit.js +++ b/test/common/unit.js @@ -3632,12 +3632,19 @@ describe('msnodesqlv8 query events', () => { it('should pause a query on an error that does not end the batch once the error is handled', async () => { const calls = [] let paused + let resumed const { request } = setup(async notify => { + notify.emit('submitted') notify.fail(odbcError('Something went wrong', 50000, '42000'), true) await tick() paused = calls.slice() request.resume() - rows('a', 1)(notify) + resumed = calls.slice() + // the driver stops at the next result set while it is paused, and reads on once resumed + rows('a', 0)(notify) + await tick() + notify.emit('row', 0) + notify.emit('column', 0, 0, false) notify.emit('done') notify.emit('free') }, pausable(calls)) @@ -3647,7 +3654,8 @@ describe('msnodesqlv8 query events', () => { request.on('row', row => received.push(row)) await completion(request, r => r.query('raiserror(N\'Something went wrong\', 16, 1); select 0 as a')) assert.deepStrictEqual(paused, ['pause'], 'a pause on an error that does not end the batch should reach the query') - assert.deepStrictEqual(calls, ['pause', 'resume'], 'the resume after it should reach the query too') + assert.deepStrictEqual(resumed, ['pause'], 'the resume after it should wait until the query stops at its next result set') + assert.deepStrictEqual(calls, ['pause', 'resume'], 'the resume after it should reach the query then') assert.deepStrictEqual(received, [{ a: 0 }], 'the rows after the error should be streamed') }) @@ -4274,6 +4282,119 @@ describe('msnodesqlv8 query events', () => { assert.deepStrictEqual(errors, ['ECANCEL'], 'the cancel should be streamed as an error') assert.strictEqual(pool.released, 1, 'the connection should be released once') }) + + // every version: msnodesqlv8 starts a query a tick or more after queryRaw(), and emits 'submitted' + // once it has. Its resume() fetches at once, and a query that is still executing, or moving on + // through statements with no result, fails the fetch with a 'Function sequence error' (HY010). + // After 'meta' the driver calls dispatch(), which returns while it is paused + it('should not resume a query held back before its first result set until it stops at one', async () => { + const calls = [] + let executing + let atMeta + const { request } = setup(async notify => { + notify.emit('submitted') + await tick() + // as a readable stream that is read while the query still executes resumes the request + request.resume() + executing = calls.slice() + notify.emit('meta', [{ name: 'a', sqlType: 'int', size: 4, nullable: false }]) + atMeta = calls.slice() + await tick() + notify.emit('row', 0) + notify.emit('column', 0, 1, false) + notify.emit('done') + notify.emit('free') + }, pausable(calls)) + request.stream = true + const received = [] + request.on('row', row => received.push(row)) + // a listener that resumes the stream at its result set does so while the driver emits 'meta' + request.on('recordset', () => { + request.pause() + request.resume() + }) + // as toReadableStream() pauses a request before it runs it + request.pause() + await completion(request, r => r.query('waitfor delay \'00:00:01\'; select 1 as a')) + assert.deepStrictEqual(executing, ['pause'], 'the query should not be resumed while it executes') + assert.deepStrictEqual(atMeta, ['pause'], 'the query should not be resumed before it stops at its result set') + assert.deepStrictEqual(calls, ['pause', 'resume'], 'the query should be resumed once it stops at its result set') + assert.deepStrictEqual(received, [{ a: 1 }], 'the row should be streamed once') + }) + + it('should not resume a query held back once it runs until it stops at its first result set', async () => { + const calls = [] + let movingOn + let atMeta + const { request } = setup(async notify => { + notify.emit('submitted') + notify.emit('rowcount', 1) + await tick() + request.resume() + movingOn = calls.slice() + notify.emit('meta', [{ name: 'a', sqlType: 'int', size: 4, nullable: false }]) + atMeta = calls.slice() + await tick() + notify.emit('row', 0) + notify.emit('column', 0, 1, false) + notify.emit('done') + notify.emit('free') + }, pausable(calls)) + request.stream = true + const received = [] + request.on('row', row => received.push(row)) + request.once('rowsaffected', () => request.pause()) + await completion(request, r => r.query('update t set a = 1; select a from t')) + assert.deepStrictEqual(movingOn, ['pause'], 'the query should not be resumed while it moves on to its result set') + assert.deepStrictEqual(atMeta, ['pause'], 'the query should not be resumed before it stops at its result set') + assert.deepStrictEqual(calls, ['pause', 'resume'], 'the query should be resumed once it stops at its result set') + assert.deepStrictEqual(received, [{ a: 1 }], 'the row should be streamed once') + }) + + it('should not resume a query held back before its first result set that ends without one', async () => { + const calls = [] + const { request } = setup(async notify => { + notify.emit('submitted') + await tick() + request.resume() + notify.emit('rowcount', 1) + await tick() + notify.emit('done') + notify.emit('free') + }, pausable(calls)) + request.stream = true + const affected = [] + request.on('rowsaffected', count => affected.push(count)) + // as toReadableStream() pauses a request before it runs it + request.pause() + await completion(request, r => r.query('waitfor delay \'00:00:01\'; update t set a = 1')) + assert.deepStrictEqual(calls, ['pause'], 'the query should not be resumed, as it needed no resume to end') + assert.deepStrictEqual(affected, [1], 'the rows affected should be streamed') + }) + + // msnodesqlv8 < 5.6 refuses to cancel a query that is not paused, which then runs to the end, but + // cancels a paused one by freeing its statement, even while the query still executes + it('should resume a query held back before it runs as soon as it is resumed', async () => { + const calls = [] + let beforeStart + const { request } = setup(async notify => { + // as a readable stream that is read before msnodesqlv8 starts the query resumes the request + request.resume() + beforeStart = calls.slice() + notify.emit('submitted') + rows('a', 1)(notify) + notify.emit('done') + notify.emit('free') + }, pausable(calls)) + request.stream = true + const received = [] + request.on('row', row => received.push(row)) + // as toReadableStream() pauses a request before it runs it + request.pause() + await completion(request, r => r.query('waitfor delay \'00:00:01\'; select 0 as a')) + assert.deepStrictEqual(beforeStart, ['pause', 'resume'], 'the query should be resumed before it starts, so that it starts unpaused') + assert.deepStrictEqual(received, [{ a: 0 }], 'the row should be streamed once') + }) }) describe('pool acquire', () => { diff --git a/test/msnodesqlv8/msnodesqlv8.js b/test/msnodesqlv8/msnodesqlv8.js index 6c4bf08c..48de3267 100644 --- a/test/msnodesqlv8/msnodesqlv8.js +++ b/test/msnodesqlv8/msnodesqlv8.js @@ -317,6 +317,8 @@ describe('msnodesqlv8', function () { it('pausing and resuming a stream once an error has ended it', done => TESTS['pausing and resuming a stream once it is over']('failed', done)) it('pausing and resuming a stream once it has been cancelled', done => TESTS['pausing and resuming a stream once it is over']('cancelled', done)) it('cancelling a stream held back while its query moves on to its first result set', done => TESTS['cancelling a stream held back while its query moves on to its first result set'](done)) + it('resuming a stream held back while its query executes', done => TESTS['resuming a stream held back while its query executes'](false, done)) + it('resuming a stream held back while its failing query executes', done => TESTS['resuming a stream held back while its query executes'](true, done)) after(done => sql.close(done)) }) diff --git a/test/tedious/tedious.js b/test/tedious/tedious.js index d6a2a4b9..af6fe7fe 100644 --- a/test/tedious/tedious.js +++ b/test/tedious/tedious.js @@ -346,6 +346,8 @@ describe('tedious', () => { it('pausing and resuming a stream once an error has ended it', done => TESTS['pausing and resuming a stream once it is over']('failed', done)) it('pausing and resuming a stream once it has been cancelled', done => TESTS['pausing and resuming a stream once it is over']('cancelled', done)) it('cancelling a stream held back while its query moves on to its first result set', done => TESTS['cancelling a stream held back while its query moves on to its first result set'](done)) + it('resuming a stream held back while its query executes', done => TESTS['resuming a stream held back while its query executes'](false, done)) + it('resuming a stream held back while its failing query executes', done => TESTS['resuming a stream held back while its query executes'](true, done)) after(done => sql.close(done)) }) From 76f4f2d0202cd7a04b962c45571711ca738b0bd2 Mon Sep 17 00:00:00 2001 From: Dan Hensby Date: Thu, 15 Oct 2026 16:36:53 +0100 Subject: [PATCH 4/4] fixup! fix: stop msnodesqlv8 streams failing when resumed while their query executes Co-Authored-By: Claude Opus 5.5 --- lib/msnodesqlv8/request.js | 9 ++-- test/common/tests.js | 46 ++++++++++++++++ test/common/unit.js | 95 +++++++++++++++++++++++++++++++-- test/msnodesqlv8/msnodesqlv8.js | 1 + test/tedious/tedious.js | 1 + 5 files changed, 144 insertions(+), 8 deletions(-) diff --git a/lib/msnodesqlv8/request.js b/lib/msnodesqlv8/request.js index 24c5c1c9..40476f8b 100644 --- a/lib/msnodesqlv8/request.js +++ b/lib/msnodesqlv8/request.js @@ -667,12 +667,13 @@ class Request extends BaseRequest { } // until the first result set the driver has read nothing, so a stream paused before then, - // or before its query starts, holds it back at once. But msnodesqlv8's resume() fetches at - // once, whatever the query is doing, and once the query has started ('submitted') it may + // or before its query starts, holds it back at once. But msnodesqlv8 < 6's resume() fetches + // at once, whatever the query is doing, and once the query has started ('submitted') it may // still be executing, or moving on through statements with no result, when the fetch fails // with a 'Function sequence error' or the result set is fetched twice. So a query held back // then is resumed only once it has stopped at a result set, a tick after its 'meta', and not - // at all if it ends first. Until msnodesqlv8 starts it, a resume only stops it starting paused + // at all if it ends first. Until msnodesqlv8 starts it, a resume only stops it starting paused. + // msnodesqlv8 6 resumes only what it has held back, so there it is resumed at once let heldBeforeResults = false let submitted = false let atResultSet = false @@ -703,7 +704,7 @@ class Request extends BaseRequest { if (finished && cancelRequested && this.paused && pending.length) emitPendingOnCancel() // a query that is being cancelled stays paused, so that the cancel can go through if (!pending.length && !this.paused && driverPaused && !finished && !cancelRequested && reachable() && - (!heldBeforeResults || !submitted || atResultSet)) { + (!heldBeforeResults || !submitted || atResultSet || driverHoldsPausedQueries())) { driverPaused = false req.resumeQuery() } diff --git a/test/common/tests.js b/test/common/tests.js index 5f6063d7..23fe20c5 100644 --- a/test/common/tests.js +++ b/test/common/tests.js @@ -3303,6 +3303,52 @@ module.exports = (sql, driver) => { setTimeout(() => req.resume(), 300) }, + 'cancelling a stream held back while its query executes' (done) { + // a stream paused before its query runs, as toReadableStream() leaves it, read while the query + // executes and then cancelled: the query stays held back until its first result set, and is + // cancelled there. With one connection in the pool, the query that follows the stream runs on + // the connection the stream had + const config = readConfig() + config.driver = driver + config.pool = { max: 1, min: 0 } + let timer + let held = false + + const pool = new sql.ConnectionPool(config) + pool.connect().then(() => new Promise(resolve => { + const errors = [] + const rows = [] + const req = new sql.Request(pool) + req.stream = true + req.on('error', err => errors.push(err.code)) + req.on('row', row => rows.push(row)) + req.once('done', () => resolve({ errors, rows })) + req.pause() + req.query('waitfor delay \'00:00:01\'; select 1 as a') + setTimeout(() => req.resume(), 200) + setTimeout(() => req.cancel(), 400) + })).then(({ errors, rows }) => { + assert.deepStrictEqual(errors, ['ECANCEL'], 'the stream should end as cancelled') + assert.deepStrictEqual(rows, [], 'no row should be streamed') + return new Promise((resolve, reject) => { + timer = setTimeout(() => { + held = true + reject(new Error('the next query on the connection should run')) + }, 5000) + pool.request().query('select 1 as a').then(resolve, reject) + }) + }).then(result => { + clearTimeout(timer) + assert.deepStrictEqual(result.recordset, [{ a: 1 }], 'the next query should return its result') + return pool.close() + }).then(() => done(), err => { + clearTimeout(timer) + // a connection that is held up cannot be closed either, so the pool is left as it is + if (held) return done(err) + pool.close().then(() => done(err), () => done(err)) + }) + }, + 'pausing and resuming a stream once it is over' (ending, done) { // msnodesqlv8 does not check that a query is still running when it is paused or resumed, // and can put one that has ended back on its connection's work queue, where it holds up diff --git a/test/common/unit.js b/test/common/unit.js index 093b9642..d6c7c9cb 100644 --- a/test/common/unit.js +++ b/test/common/unit.js @@ -2825,8 +2825,8 @@ describe('msnodesqlv8 query events', () => { // result set, move on to the next one a second time; 5.6 waits for the rest of the batch, // unless it was paused on the batch's last row. `timeOutAtFetch` names a fetch that does not // come back before the request times out, and `failAtEnd` ends the query on an error; a query - // that ends on an error emits 'free' and no 'done', as msnodesqlv8 does. `beginDelay` holds - // back the end of the first statement. + // that ends on an error emits 'free' and no 'done', as msnodesqlv8 does. The query emits + // 'submitted' as soon as it is sent, and `beginDelay` holds back the end of its first statement. const reader = (version, recordsets, { timeOutAtFetch, failAtEnd, beginDelay } = {}) => { const pausedAt = [] const calls = [] @@ -2974,6 +2974,7 @@ describe('msnodesqlv8 query events', () => { notify.emit('meta', meta()) dispatch() } + notify.emit('submitted') if (beginDelay) { setTimeout(begin, beginDelay) } else { @@ -3376,6 +3377,36 @@ describe('msnodesqlv8 query events', () => { assert.deepStrictEqual(errors, ['ECANCEL'], 'the cancel should be streamed as an error') }) + // a stream resumed while its query executes leaves the query held back, and so paused, until it + // stops at its first result set, and a cancel made meanwhile is made there + for (const version of ['2.x to 4.x', '5.0 to 5.5', '5.6']) { + it(`should cancel a stream resumed while its query executes once the query stops at its first result set with msnodesqlv8 ${version}`, async () => { + const { request, calls, fetches, queried, pool } = setupReader(version, [['a', 75]], { beginDelay: 10 }) + request.stream = true + const rows = [] + const errors = [] + request.on('error', err => errors.push(err.code)) + request.on('row', row => rows.push(row.a)) + const completed = new Promise(resolve => request.on('done', resolve)) + // as toReadableStream() pauses a request before it runs it, and is read, and its request + // cancelled, while the query executes + request.pause() + request.query('select a') + + while (!queried()) await tick() + request.resume() + request.cancel() + const executing = calls.slice() + await completed + assert.deepStrictEqual(executing, ['pause'], 'the query should be neither resumed nor cancelled while it executes') + assert.deepStrictEqual(calls, ['pause', 'cancel'], 'the query should be cancelled once it stops at its first result set, rather than resumed') + assert.strictEqual(fetches(), 0, 'the driver should fetch nothing') + assert.deepStrictEqual(errors, ['ECANCEL'], 'the cancel should be streamed as an error') + assert.deepStrictEqual(rows, [], 'no row should be emitted') + assert.strictEqual(pool.released, 1, 'the connection should be released once') + }) + } + it('should not emit the rows of a cancelled paused stream that a listener resumes', async () => { const { request, notify } = setupReader('5.6', [['a', 75], ['b', 30]]) request.stream = true @@ -4372,8 +4403,37 @@ describe('msnodesqlv8 query events', () => { assert.deepStrictEqual(affected, [1], 'the rows affected should be streamed') }) - // msnodesqlv8 < 5.6 refuses to cancel a query that is not paused, which then runs to the end, but - // cancels a paused one by freeing its statement, even while the query still executes + // a cancel made while the query of a resumed stream is still held back is made where the query + // stops, as it is for a stream that was never resumed + it('should cancel a query held back while it moves on to its first result set without resuming it', async () => { + const calls = [] + let movingOn + const { request, pool } = setup(async notify => { + notify.emit('submitted') + notify.emit('rowcount', 1) + await tick() + // as a readable stream that is read, and its request then cancelled, while the query moves + // on through statements with no result + request.resume() + request.cancel() + movingOn = calls.slice() + await tick() + notify.emit('meta', [{ name: 'a', sqlType: 'int', size: 4, nullable: false }]) + }, cancellable(calls)) + request.stream = true + const errors = [] + request.on('error', err => errors.push(err.code)) + // as toReadableStream() pauses a request before it runs it + request.pause() + await cancelled(request, r => r.query('update t set a = 1; waitfor delay \'00:00:01\'; select a from t')) + assert.deepStrictEqual(movingOn, ['pause'], 'the query should be neither resumed nor cancelled while it moves on to its result set') + assert.deepStrictEqual(calls, ['pause', 'cancel'], 'the query should be cancelled once it stops at its result set, rather than resumed') + assert.deepStrictEqual(errors, ['ECANCEL'], 'the cancel should be streamed as an error') + assert.strictEqual(pool.released, 1, 'the connection should be released once') + }) + + // until msnodesqlv8 starts a query, resuming it fetches nothing: it only has the query start + // unpaused, as it did before it('should resume a query held back before it runs as soon as it is resumed', async () => { const calls = [] let beforeStart @@ -4395,6 +4455,33 @@ describe('msnodesqlv8 query events', () => { assert.deepStrictEqual(beforeStart, ['pause', 'resume'], 'the query should be resumed before it starts, so that it starts unpaused') assert.deepStrictEqual(received, [{ a: 0 }], 'the row should be streamed once') }) + + // msnodesqlv8 6 raises what a paused query brings back only once it is resumed + it('should resume a query held back before its first result set at once with msnodesqlv8 6', async () => { + driverPackage.version = '6.0.0' + const calls = [] + let executing + const { request } = setup(async notify => { + notify.emit('submitted') + await tick() + // as a readable stream that is read while the query still executes resumes the request + request.resume() + executing = calls.slice() + notify.emit('meta', [{ name: 'a', sqlType: 'int', size: 4, nullable: false }]) + notify.emit('row', 0) + notify.emit('column', 0, 1, false) + notify.emit('done') + notify.emit('free') + }, pausable(calls)) + request.stream = true + const received = [] + request.on('row', row => received.push(row)) + // as toReadableStream() pauses a request before it runs it + request.pause() + await completion(request, r => r.query('waitfor delay \'00:00:01\'; select 1 as a')) + assert.deepStrictEqual(executing, ['pause', 'resume'], 'the query should be resumed at once') + assert.deepStrictEqual(received, [{ a: 1 }], 'the row should be streamed once') + }) }) describe('pool acquire', () => { diff --git a/test/msnodesqlv8/msnodesqlv8.js b/test/msnodesqlv8/msnodesqlv8.js index 48de3267..af50df7f 100644 --- a/test/msnodesqlv8/msnodesqlv8.js +++ b/test/msnodesqlv8/msnodesqlv8.js @@ -319,6 +319,7 @@ describe('msnodesqlv8', function () { it('cancelling a stream held back while its query moves on to its first result set', done => TESTS['cancelling a stream held back while its query moves on to its first result set'](done)) it('resuming a stream held back while its query executes', done => TESTS['resuming a stream held back while its query executes'](false, done)) it('resuming a stream held back while its failing query executes', done => TESTS['resuming a stream held back while its query executes'](true, done)) + it('cancelling a stream held back while its query executes', done => TESTS['cancelling a stream held back while its query executes'](done)) after(done => sql.close(done)) }) diff --git a/test/tedious/tedious.js b/test/tedious/tedious.js index af6fe7fe..a2b2c26f 100644 --- a/test/tedious/tedious.js +++ b/test/tedious/tedious.js @@ -348,6 +348,7 @@ describe('tedious', () => { it('cancelling a stream held back while its query moves on to its first result set', done => TESTS['cancelling a stream held back while its query moves on to its first result set'](done)) it('resuming a stream held back while its query executes', done => TESTS['resuming a stream held back while its query executes'](false, done)) it('resuming a stream held back while its failing query executes', done => TESTS['resuming a stream held back while its query executes'](true, done)) + it('cancelling a stream held back while its query executes', done => TESTS['cancelling a stream held back while its query executes'](done)) after(done => sql.close(done)) })