diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 6e0b1b0f..755b6f5a 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -64,6 +64,7 @@ Driver index files use `Object.assign()` to merge base exports with driver-speci ### Test patterns - **Unit tests** (`test/common/unit.js`): No database needed. Tests type mapping, table parsing, error construction. +- **msnodesqlv8 event stubs**: the `msnodesqlv8 query events` block at the end of `test/common/unit.js` stands a bare object in for `require('msnodesqlv8')` and plays the events a given msnodesqlv8 version emits. The msnodesqlv8 integration suite only runs on Windows CI, so extend these stubs for any change to how `lib/msnodesqlv8/request.js` handles driver events. - **Integration tests** (`test/common/tests.js`): Exported as a factory `(sql, driver) => { ... }` — called by each driver's test file. Many tests exercise three execution styles: callback, promise, and streaming. - **Framework**: Mocha + Node.js built-in `assert` (no chai/sinon). - **Keeping this file up to date:** If a change affects the architecture, conventions, build process, or any other information documented here, update this file as part of the same PR. diff --git a/.github/workflows/nodejs.yml b/.github/workflows/nodejs.yml index bc583e6c..06805b2d 100644 --- a/.github/workflows/nodejs.yml +++ b/.github/workflows/nodejs.yml @@ -165,11 +165,12 @@ jobs: # v2: Node 18 v3: Node 18-20 v4: Node 18-24 v5: Node 20-24 # # v5 is split: msnodesqlv8 5.2.2 dropped the Node 20 (ABI 115) win32-x64 - # prebuild, so `@^5` (5.2.3+) is gated to Node 22-24 and Node 20 is pinned - # to 5.2.1 — the last v5 release that ships one. Upstream now builds - # prebuilds for Node 22/24/26 only, so a floating `@^5` will not regain a - # Node 20 prebuild; leaving Node 20 on `@^5` source-builds via node-gyp, - # which is slow on windows-2022 and fails outright on windows-2025. + # prebuild, so `@^5` is gated to Node 22-24 and Node 20 is pinned to 5.2.1, + # the last release before that drop. A combination without a prebuild + # source-builds via node-gyp, which is slow on windows-2022 and fails + # outright on windows-2025. From 5.3.0 msnodesqlv8 ships a single Node-API + # binary instead of one per ABI, which may load on Node 20 again but has not + # been tried here. - name: Install msnodesqlv8 v2 if: ${{ matrix.node == '18.x' }} run: npm install --no-save msnodesqlv8@^2 diff --git a/README.md b/README.md index 7035c9dd..c05d8e7f 100644 --- a/README.md +++ b/README.md @@ -1246,6 +1246,10 @@ request.query('waitfor delay \'00:00:05\'; select 1 as number', (err, result) => 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. + ## Transaction **IMPORTANT:** always use `Transaction` class to create transactions - it ensures that all your requests are executed on one connection. Once you call `begin`, a single connection is acquired from the connection pool and all subsequent requests (initialized with the `Transaction` object) are executed exclusively on this connection. After you call `commit` or `rollback`, connection is then released back to the connection pool. diff --git a/lib/msnodesqlv8/request.js b/lib/msnodesqlv8/request.js index 692064c4..750f79d5 100644 --- a/lib/msnodesqlv8/request.js +++ b/lib/msnodesqlv8/request.js @@ -334,13 +334,22 @@ class Request extends BaseRequest { let handleOutput = false let isChunkedRecordset = false let chunksBuffer = null + let terminated = null + // set while the error just collected is being dispatched, along with whatever came with it + let failing = false + let cancelRequested = false + // 'done' ends the request. msnodesqlv8 >= 5.6 emits it as soon as a cancel is through, and + // the query can go on emitting after it, so the listeners ignore anything that follows + let finished = false const handleError = (req, connection, info, moreErrors) => { - const doReturn = !moreErrors - if ((typeof info.sqlstate === 'string') && (info.sqlstate.toLowerCase() === '08s01')) { - connection.hasError = true + // a cancel is reported from 'done'. msnodesqlv8 >= 5.6 also sends an 'Operation canceled' + // error after a cancel, and one that arrives before 'done' is dropped here + if (cancelRequested && info.sqlstate === 'HY008') { + return } + const doReturn = !moreErrors const err = new RequestError(info, 'EREQUEST') err.code = 'EREQUEST' @@ -360,13 +369,8 @@ class Request extends BaseRequest { debug('request(%d): failed', IDS.get(this), err) } - let previous if (errors.length) { - previous = errors.pop() - if (!err.precedingErrors) { - err.precedingErrors = [] - } - err.precedingErrors.push(previous) + err.precedingErrors = errors.splice(0) } hasReturned = true @@ -376,6 +380,10 @@ class Request extends BaseRequest { // we must collect errors even in stream mode errors.push(err) + if (!failing) { + failing = true + queueMicrotask(() => { failing = false }) + } } // nested = function is called by this.execute @@ -482,8 +490,19 @@ class Request extends BaseRequest { this._cancel = () => { debug('request(%d): cancel', IDS.get(this)) + // newer msnodesqlv8 versions drop a repeat cancel without calling back, so the + // outcome of the first one stands + if (typeof req.getCancelSent === 'function' && req.getCancelSent()) { + return + } + cancelRequested = true req.cancelQuery(err => { - if (err) debug('request(%d): failed to cancel', IDS.get(this), err) + if (err) { + debug('request(%d): failed to cancel', IDS.get(this), err) + // msnodesqlv8 < 5.6 refuses to cancel a query that is neither paused nor polling, + // and the query runs to the end + if (!hasReturned) cancelRequested = false + } // this fixes an issue where paused connections don't emit a done event try { if (req.isPaused()) req.emit('done') @@ -494,6 +513,8 @@ class Request extends BaseRequest { } req.on('meta', metadata => { + if (finished) return + if (row) { if (isChunkedRecordset) { const concatenatedChunks = chunksBuffer.join('') @@ -571,7 +592,7 @@ class Request extends BaseRequest { }) req.on('row', rownumber => { - if (row && isChunkedRecordset) return + if (finished || (row && isChunkedRecordset)) return if (this.arrayRowMode) { row = [] @@ -583,6 +604,8 @@ class Request extends BaseRequest { }) req.on('column', (idx, data, more) => { + if (finished) return + if (isChunkedRecordset) { chunksBuffer.push(data) } else { @@ -622,13 +645,17 @@ class Request extends BaseRequest { }) req.on('rowcount', rowCount => { + if (finished) return + rowsAffected.push(rowCount) if (this.stream) { this.emit('rowsaffected', rowCount) } }) - req.on('info', msg => { + req.on('info', (msg, more) => { + if (finished) return + const parsedMessage = (/^\[Microsoft\]\[SQL Server Native Client 11\.0\](?:\[SQL Server\])?([\s\S]*)$/).exec(msg.message) if (parsedMessage) { msg.message = parsedMessage[1] @@ -646,16 +673,75 @@ class Request extends BaseRequest { // query terminated if (msg.code === 3621 && !hasReturned) { - // if the query has been terminated it's probably best to throw the last meaningful error if there was one - // pop it off the errors array so it doesn't get put in twice - const error = errors.length > 0 ? errors.pop() : msg - handleError(req, connection, error.originalError || error, false) + if (failing && !more) { + // msnodesqlv8 2.0 ends a statement's error with more=true and this message, and then + // nothing follows: the error just collected is the one that terminated the statement. + // A stream has already emitted it + if (!this.stream) { + const error = errors.pop() + handleError(req, connection, error.originalError || error, false) + } + } else if (!terminated) { + // msnodesqlv8 >= 5.6 emits a failed statement's info messages ahead of its error, and + // carries on after it, so any error collected so far is an earlier statement's. The + // error still to come, or 'done', settles the request; 'done' falls back to this + terminated = msg + } } }) - req.on('error', errorHandlers.error = handleError.bind(null, req, connection)) + req.on('error', errorHandlers.error = (info, moreErrors) => { + // a dead link is recorded even after 'done', so the pool does not lend the connection out again + if ((typeof info.sqlstate === 'string') && (info.sqlstate.toLowerCase() === '08s01')) { + connection.hasError = true + } + + if (finished) return + + handleError(req, connection, info, moreErrors) + }) req.once('done', () => { + finished = true + + if (hasReturned) { + return + } + + if (cancelRequested) { + // msnodesqlv8 emits 'done' once a cancel is through: for a paused query in every + // version, and for a running one from 5.6. What was read up to then is not a result. + // The error listener stays attached and drops the 'Operation canceled' error that can + // still arrive after 'done'. + hasReturned = true + delete this._cancel + // released now, as at any 'done', rather than at the statement's later 'free': + // msnodesqlv8 queues whatever the connection is sent next behind the cancelled + // statement, and a transaction can carry on as soon as the cancel is reported + this.parent.release(connection) + + debug('request(%d): canceled', IDS.get(this)) + + const err = new RequestError('Canceled.', 'ECANCEL') + if (this.stream) { + // as with tedious, a cancelled stream reports the cancel as an error and still + // completes with what was read up to it + this.emit('error', err) + return callback(null, null, output, rowsAffected, recordsetcolumns) + } + return callback(err) + } + + if (terminated && !errors.length) { + // the statement was terminated, but no error followed to say why + handleError(req, connection, terminated, false) + } else if (errors.length && !this.stream) { + // msnodesqlv8 >= 5.6 reports an error that does not end the batch with more=true and + // carries on to 'done', so the request has failed although no error said it was the last + const error = errors.pop() + handleError(req, connection, error.originalError || error, false) + } + if (hasReturned) { return } diff --git a/test/common/tests.js b/test/common/tests.js index 1ec40a38..0d4800ac 100644 --- a/test/common/tests.js +++ b/test/common/tests.js @@ -538,6 +538,34 @@ module.exports = (sql, driver) => { }) }, + 'query with an error that does not end the batch' (done) { + const req = new TestRequest() + req.query('raiserror(N\'Something went wrong\', 16, 1); select 1 as a', err => { + assert.ok(err instanceof sql.RequestError, 'the query should fail') + assert.strictEqual(err.message, 'Something went wrong', 'the error should be the one the batch raised') + assert.strictEqual(err.number, 50000, 'the error should be the one the batch raised') + + done() + }) + }, + + 'query with a constraint violation' (afterAResult, done) { + const statements = [ + // the batch fails before it returns anything + 'declare @t table (id int primary key); insert into @t values (1), (1)' + ] + if (afterAResult) { + // a statement after a result fails + statements.push('declare @t table (id int primary key); insert into @t values (1); insert into @t values (1)') + } + statements.reduce((previous, statement) => previous.then(() => new TestRequest().query(statement).then(() => { + throw new Error(`${statement} should fail`) + }, err => { + assert.ok(err instanceof sql.RequestError, `${statement} should fail with a RequestError`) + assert.strictEqual(err.number, 2627, `${statement} should fail with the violation, not the statement's termination`) + })), Promise.resolve()).then(() => done(), done) + }, + 'query with raiseerror' (done) { const notices = [] const req = new TestRequest() @@ -1712,7 +1740,11 @@ module.exports = (sql, driver) => { const req = tran.request() req.input('date', sql.TinyInt, 1561651515615) - req.execute('someStoreProc').catch(() => { + // the transaction is rolled back either way, so a failure is reported rather than + // left holding a connection + req.execute('someStoreProc').then(() => { + tran.rollback(() => done(new Error('executing a procedure that does not exist should fail'))) + }, () => { tran.rollback(done) }) }).catch(done) @@ -1729,6 +1761,20 @@ module.exports = (sql, driver) => { req.cancel() }, + 'cancel a running request' (cancels, done) { + const req = new TestRequest() + req.query('waitfor delay \'00:00:05\'; select 1 as a').then(result => { + assert.ok(!cancels, 'the request should be cancelled') + assert.deepStrictEqual(result.recordset, [{ a: 1 }], 'a request that cannot be cancelled should run to the end') + }, err => { + assert.ok(cancels, `the request should not fail: ${err.message}`) + assert.strictEqual(err.code, 'ECANCEL', 'the request should fail as cancelled') + }).then(() => done(), done) + + // by then the query is running + setTimeout(() => req.cancel(), 1000) + }, + 'repeat calls to connect resolve' (config, done) { const pool = new sql.ConnectionPool(config) Promise.all([pool.connect(), pool.connect()]) diff --git a/test/common/unit.js b/test/common/unit.js index a206755d..ee832aa3 100644 --- a/test/common/unit.js +++ b/test/common/unit.js @@ -1,6 +1,6 @@ 'use strict' -/* globals describe, it, before, afterEach */ +/* globals describe, it, before, after, afterEach */ const sql = require('../../') const assert = require('node:assert') @@ -1690,3 +1690,460 @@ describe('procedure name validation', () => { } }) }) + +describe('msnodesqlv8 query events', () => { + const Module = require('node:module') + const { EventEmitter } = require('node:events') + const RequestError = require('../../lib/error/request-error') + + // msnodesqlv8 is a native module that only the Windows CI jobs install. The request only + // 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' + const REQUEST = require.resolve('../../lib/msnodesqlv8/request') + let resolveFilename + let Request + + before(() => { + resolveFilename = Module._resolveFilename + Module._resolveFilename = function (request, ...rest) { + return request === 'msnodesqlv8' ? STUB : resolveFilename.call(this, request, ...rest) + } + require.cache[STUB] = { id: STUB, filename: STUB, loaded: true, exports: {} } + Request = require(REQUEST) + }) + + after(() => { + Module._resolveFilename = resolveFilename + delete require.cache[STUB] + delete require.cache[REQUEST] + }) + + 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') + const CANCELED = () => odbcError('[msnodesql] Operation canceled', 0, 'HY008') + const REFUSED = () => new Error('Error: [msnodesql] cancel only supported for statements where polling is enabled.') + const tick = () => new Promise(resolve => setImmediate(resolve)) + + // the request's pool, its connection and the query that connection runs; `play` is given + // the query once it is running and emits on it what msnodesqlv8 would + const setup = (play, query = {}) => { + const notify = Object.assign(new EventEmitter(), { + isPaused: () => false, + cancelQuery () {} + }, query) + // msnodesqlv8 does not emit an error that nothing listens for + notify.fail = (err, more) => { + if (notify.listenerCount('error') > 0) notify.emit('error', err, more) + } + const connection = { + queryRaw () { + setImmediate(play, notify) + return notify + } + } + const pool = { + connected: true, + config: {}, + released: 0, + acquire (request, callback) { callback(null, connection, { requestTimeout: 15000 }) }, + release () { this.released++ } + } + return { request: new Request(pool), notify, pool, connection } + } + + const result = (meta, value) => notify => { + notify.emit('meta', [{ name: meta, sqlType: 'int', size: 4, nullable: false }]) + notify.emit('row', 0) + notify.emit('column', 0, value, false) + } + + it('should reject when an error ends the batch', async () => { + const { request, pool } = setup(notify => { + notify.fail(PRIMARY_KEY(), false) + notify.emit('done') + }) + await assert.rejects(request.query('insert into t values (1)'), err => { + assert.ok(err instanceof RequestError, 'the error should be a RequestError') + assert.strictEqual(err.number, 2627, 'the error should be the one that ended the batch') + assert.strictEqual(err.code, 'EREQUEST', 'the error should be a request error') + return true + }) + assert.strictEqual(pool.released, 1, 'the connection should be released once') + }) + + // msnodesqlv8 >= 5.6 + it('should reject when an error does not end the batch', async () => { + const { request, pool } = setup(notify => { + notify.fail(odbcError('Something went wrong', 50000, '42000'), true) + result('a', 1)(notify) + notify.emit('done') + }) + await assert.rejects(request.query('raiserror(N\'Something went wrong\', 16, 1); select 1 as a'), err => { + assert.strictEqual(err.message, 'Something went wrong', 'the error should be the one the batch reported') + assert.strictEqual(err.code, 'EREQUEST', 'the error should be a request error') + return true + }) + assert.strictEqual(pool.released, 1, 'the connection should be released once') + }) + + // msnodesqlv8 >= 5.6 + it('should keep the errors that came before the last one in a batch that carries on', async () => { + const { request } = setup(notify => { + notify.fail(odbcError('first', 50000, '42000'), true) + result('a', 1)(notify) + notify.fail(odbcError('second', 50000, '42000'), true) + result('b', 2)(notify) + notify.fail(odbcError('third', 50000, '42000'), true) + result('c', 3)(notify) + notify.emit('done') + }) + await assert.rejects(request.query('select 1'), err => { + assert.strictEqual(err.message, 'third', 'the error should be the last one reported') + assert.deepStrictEqual(err.precedingErrors.map(e => e.message), ['first', 'second'], 'the earlier errors should precede it') + return true + }) + }) + + // msnodesqlv8 >= 5.6, which carries on to the select of the return value + it('should reject an execute of a procedure that cannot be found', async () => { + const { request, pool } = setup(notify => { + notify.fail(odbcError('Could not find stored procedure \'missing\'.', 2812, '42000'), true) + result('___return___', 0)(notify) + notify.emit('done') + }) + await assert.rejects(request.execute('missing'), err => { + assert.strictEqual(err.number, 2812, 'the error should be the missing procedure') + return true + }) + assert.strictEqual(pool.released, 1, 'the connection should be released once') + }) + + it('should stream an error that does not end the batch, then complete', async () => { + const { request } = setup(notify => { + notify.fail(odbcError('Something went wrong', 50000, '42000'), true) + result('a', 1)(notify) + notify.emit('done') + }) + request.stream = true + const errors = [] + const rows = [] + request.on('error', err => errors.push(err)) + request.on('row', row => rows.push(row)) + await new Promise(resolve => { + request.on('done', resolve) + request.query('select 1') + }) + assert.deepStrictEqual(errors.map(e => e.message), ['Something went wrong'], 'the error should be streamed once') + assert.deepStrictEqual(rows, [{ a: 1 }], 'the rows after the error should still be streamed') + }) + + // msnodesqlv8 >= 5.6 emits a failed statement's info messages ahead of its error + it('should reject with the error a statement was terminated by when it follows the message', async () => { + const { request } = setup(notify => { + notify.emit('info', TERMINATED(), false) + notify.fail(PRIMARY_KEY(), false) + notify.emit('done') + }) + await assert.rejects(request.query('insert into t values (1)'), err => { + assert.strictEqual(err.number, 2627, 'the error should be the one that terminated the statement') + return true + }) + }) + + // msnodesqlv8 >= 5.6, when the statement fails after an earlier result and the batch carries on + it('should reject with the error a statement was terminated by when it follows the message and does not end the batch', async () => { + const { request } = setup(notify => { + notify.emit('info', TERMINATED(), false) + notify.fail(PRIMARY_KEY(), true) + result('___return___', 0)(notify) + notify.emit('done') + }) + await assert.rejects(request.execute('insert_t'), err => { + assert.strictEqual(err.number, 2627, 'the error should be the one that terminated the statement') + return true + }) + }) + + // msnodesqlv8 2.0 could stop after the message that the statement was terminated (node-mssql #1126) + it('should reject with the error a statement was terminated by when it comes before the message', async () => { + const { request } = setup(notify => { + notify.fail(PRIMARY_KEY(), true) + notify.emit('info', TERMINATED(), false) + }) + await assert.rejects(request.query('insert into t values (1)'), err => { + assert.strictEqual(err.number, 2627, 'the error should be the one that terminated the statement') + return true + }) + }) + + it('should reject with the message that a statement was terminated when no error says why', async () => { + const { request } = setup(notify => { + notify.emit('info', TERMINATED(), false) + notify.emit('done') + }) + await assert.rejects(request.query('insert into t values (1)'), err => { + assert.strictEqual(err.number, 3621, 'the error should be the termination') + return true + }) + }) + + // msnodesqlv8 >= 5.6: the batch carries on after an earlier statement's error, and the + // message that a later statement was terminated comes ahead of the error that terminated it + it('should reject with the last error when a later statement is terminated', async () => { + const { request, pool } = setup(async notify => { + notify.fail(odbcError('Something went wrong', 50000, '42000'), true) + await tick() + notify.emit('info', TERMINATED(), false) + notify.fail(PRIMARY_KEY(), true) + await tick() + result('a', 1)(notify) + notify.emit('done') + }) + await assert.rejects(request.query('raiserror(N\'Something went wrong\', 16, 1); insert into t values (1); select 1 as a'), err => { + assert.strictEqual(err.number, 2627, 'the error should be the last one the batch reported') + assert.deepStrictEqual(err.precedingErrors.map(e => e.number), [50000], 'the earlier error should precede it') + return true + }) + assert.strictEqual(pool.released, 1, 'the connection should be released once') + }) + + // msnodesqlv8 >= 5.6: a procedure whose first statement fails reports the error ahead of the + // message, and carries on to the select of the return value + it('should reject an execute whose first statement is terminated, once the batch is done', async () => { + let releasedBeforeDone + const { request, pool } = setup(notify => { + notify.fail(PRIMARY_KEY(), true) + notify.emit('info', TERMINATED(), true) + result('___return___', 0)(notify) + releasedBeforeDone = pool.released + notify.emit('done') + }) + await assert.rejects(request.execute('insert_t'), err => { + assert.strictEqual(err.number, 2627, 'the error should be the one that terminated the statement') + return true + }) + assert.strictEqual(releasedBeforeDone, 0, 'the connection should not be released before the batch is done') + assert.strictEqual(pool.released, 1, 'the connection should be released once') + }) + + it('should stream an error a statement was terminated by once', async () => { + const { request } = setup(notify => { + notify.fail(PRIMARY_KEY(), false) + notify.emit('info', TERMINATED(), false) + notify.emit('done') + }) + request.stream = true + const errors = [] + request.on('error', err => errors.push(err)) + await new Promise(resolve => { + request.on('done', resolve) + request.query('insert into t values (1)') + }) + assert.deepStrictEqual(errors.map(e => e.number), [2627], 'the error should be streamed once') + }) + + // msnodesqlv8 >= 5.6: 'done' as soon as the cancel is through, then an error from the read + // the cancel cut short, then 'free' once the statement is released + const cancels = { + cancelQuery (callback) { + setImmediate(() => { + this.emit('done') + callback(null) + setImmediate(() => { + this.fail(CANCELED(), false) + setImmediate(() => this.emit('free')) + }) + }) + } + } + + // msnodesqlv8 >= 5.6. The connection goes back with the cancel, not at the statement's 'free' + it('should reject a running query that is cancelled', async () => { + const { request, notify, pool } = setup(notify => { + result('a', 1)(notify) + request.cancel() + }, cancels) + const freed = new Promise(resolve => notify.once('free', resolve)) + await assert.rejects(request.query('waitfor delay \'00:00:05\'; select 1 as a'), err => { + assert.strictEqual(err.code, 'ECANCEL', 'the error should be a cancel') + return true + }) + assert.strictEqual(pool.released, 1, 'the connection should be released with the cancel') + await freed + assert.strictEqual(pool.released, 1, 'the connection should be released once') + }) + + // msnodesqlv8 >= 5.6 + it('should stream a cancel once, then complete', async () => { + const { request } = setup(notify => { + result('a', 1)(notify) + request.cancel() + }, cancels) + request.stream = true + const errors = [] + let done = 0 + request.on('error', err => errors.push(err)) + await new Promise(resolve => { + request.on('done', () => { + done++ + resolve() + }) + request.query('select * from streaming') + }) + // the error the cancel cut the read short with comes after 'done' + await tick() + await tick() + assert.deepStrictEqual(errors.map(e => e.code), ['ECANCEL'], 'only the cancel should be streamed as an error') + assert.strictEqual(done, 1, 'the stream should complete once') + }) + + // msnodesqlv8 >= 5.6 can report a cancel the statement refused as done, and the query goes on + it('should stream nothing a cancelled query emits after done', async () => { + const { request, pool } = setup(notify => { + result('a', 1)(notify) + request.cancel() + }, { + cancelQuery (callback) { + setImmediate(() => { + this.emit('done') + callback(null) + setImmediate(() => { + result('b', 2)(this) + this.emit('rowcount', 1) + this.emit('info', odbcError('Still running', 0, '01000'), false) + this.fail(odbcError('Something went wrong', 50000, '42000'), false) + this.emit('done') + }) + }) + } + }) + request.stream = true + const events = [] + request.on('recordset', () => events.push('recordset')) + request.on('row', () => events.push('row')) + request.on('rowsaffected', () => events.push('rowsaffected')) + request.on('info', () => events.push('info')) + request.on('error', err => events.push(`error ${err.code}`)) + request.on('done', () => events.push('done')) + request.query('select * from streaming') + while (!events.includes('done')) await tick() + await tick() + await tick() + assert.deepStrictEqual(events, ['recordset', 'row', 'error ECANCEL', 'done'], 'nothing should be streamed after done') + assert.strictEqual(pool.released, 1, 'the connection should be released once') + }) + + // msnodesqlv8 < 5.6 + it('should complete a query whose cancel is refused', async () => { + const { request } = setup(notify => { + result('a', 1)(notify) + request.cancel() + setImmediate(() => setImmediate(() => notify.emit('done'))) + }, { + cancelQuery (callback) { + setImmediate(callback, REFUSED()) + } + }) + const { recordset } = await request.query('select 1 as a') + assert.deepStrictEqual(recordset, [{ a: 1 }], 'the query should complete') + }) + + // msnodesqlv8 5.1 to 5.5 note the first cancel, refused or not, and drop any after it + it('should complete a query whose first cancel is refused and later ones dropped', async () => { + let sent = false + const { request } = setup(notify => { + result('a', 1)(notify) + request.cancel() + setImmediate(() => setImmediate(() => { + request.cancel() + notify.emit('done') + })) + }, { + getCancelSent: () => sent, + cancelQuery (callback) { + if (sent) return + sent = true + setImmediate(callback, REFUSED()) + } + }) + const { recordset } = await request.query('select 1 as a') + assert.deepStrictEqual(recordset, [{ a: 1 }], 'the query should complete') + }) + + // every msnodesqlv8 version cancels a paused query, and ends it with 'done' before calling back + it('should stream a cancel of a paused query once, then complete with what was read', async () => { + let paused = false + const { request, pool } = setup(notify => { + result('a', 1)(notify) + notify.emit('rowcount', 1) + result('b', 2)(notify) + }, { + isPaused: () => paused, + pauseQuery () { paused = true }, + resumeQuery () { paused = false }, + cancelQuery (callback) { + setImmediate(() => { + this.emit('done') + callback(null) + }) + } + }) + request.stream = true + const errors = [] + let completed = null + request.on('error', err => errors.push(err)) + request.on('row', row => { + if (row.b) { + request.pause() + request.cancel() + } + }) + await new Promise(resolve => { + request.on('done', result => { + completed = result + resolve() + }) + request.query('select * from streaming') + }) + await tick() + assert.deepStrictEqual(errors.map(e => e.code), ['ECANCEL'], 'the cancel should be streamed as an error') + assert.deepStrictEqual(completed.rowsAffected, [1], 'the stream should complete with the rows affected so far') + assert.strictEqual(pool.released, 1, 'the connection should be released once') + }) + + // msnodesqlv8 >= 5.6 can fail the read a cancel cut short before the cancel is through + it('should reject a cancelled query whose read fails before the cancel is through', async () => { + const { request, pool } = setup(notify => request.cancel(), { + cancelQuery (callback) { + setImmediate(() => { + this.fail(CANCELED(), false) + this.emit('done') + callback(null) + }) + } + }) + await assert.rejects(request.query('waitfor delay \'00:00:05\''), err => { + assert.strictEqual(err.code, 'ECANCEL', 'the error should be a cancel') + return true + }) + assert.strictEqual(pool.released, 1, 'the connection should be released once') + }) + + it('should mark the connection broken when the link fails after a cancel', async () => { + const { request, connection } = setup(notify => request.cancel(), { + cancelQuery (callback) { + setImmediate(() => { + this.emit('done') + callback(null) + setImmediate(() => this.fail(odbcError('Communication link failure', 0, '08S01'), false)) + }) + } + }) + await assert.rejects(request.query('select 1')) + await tick() + await tick() + assert.strictEqual(connection.hasError, true, 'a link failure after a cancel should mark the connection as broken') + }) +}) diff --git a/test/msnodesqlv8/msnodesqlv8.js b/test/msnodesqlv8/msnodesqlv8.js index 9b952408..ef04cf97 100644 --- a/test/msnodesqlv8/msnodesqlv8.js +++ b/test/msnodesqlv8/msnodesqlv8.js @@ -10,6 +10,16 @@ const TIMES = require('../common/times.js')(sql, 'msnodesqlv8') const versionHelper = require('../common/versionhelper') const { readFileSync } = require('node:fs') +const [major, minor, patch] = require('msnodesqlv8/package.json').version.split('.').map(Number) + +// before 5.1.9 msnodesqlv8 reads the next result's metadata after SQLMoreResults has failed, and +// that call replaces an error that follows an earlier result in the batch with one of its own +const reportsErrorsAfterAResult = major > 5 || (major === 5 && (minor > 1 || (minor === 1 && patch >= 9))) + +// msnodesqlv8 cancels a running query from 5.6; before that it only cancels one that is paused +// or polling, and lets any other run to the end +const cancelsRunningQueries = major > 5 || (major === 5 && minor >= 6) + const config = function () { const cfg = JSON.parse(readFileSync(join(__dirname, '../.mssql.json'))) cfg.driver = 'msnodesqlv8' @@ -93,6 +103,8 @@ describe('msnodesqlv8', function () { it('parameters with length as max', done => TESTS['parameters with length as max'](done)) it('output parameters with a falsy initial value', done => TESTS['output parameters with a falsy initial value'](done)) it('query with error', done => TESTS['query with error'](done)) + it('query with an error that does not end the batch', done => TESTS['query with an error that does not end the batch'](done)) + it('query with a constraint violation', done => TESTS['query with a constraint violation'](reportsErrorsAfterAResult, done)) it('query with multiple errors (not supported by msnodesqlv8)', done => TESTS['query with multiple errors'](done)) it.skip('query with raiseerror (not supported by msnodesqlv8)', done => TESTS['query with raiseerror'](done)) it('query with toReadableStream', done => TESTS['query with toReadableStream'](done)) @@ -119,6 +131,7 @@ describe('msnodesqlv8', function () { it.skip('transaction with error (XACT_ABORT set to ON) (not supported by msnodesqlv8)', done => TESTS['transaction with error'](done)) it('transaction with synchronous error', done => TESTS['transaction with synchronous error'](done)) it('cancel request', done => TESTS['cancel request'](done)) + it('cancel a running request', done => TESTS['cancel a running request'](cancelsRunningQueries, done)) it('allows repeat calls to connect', done => TESTS['repeat calls to connect resolve'](config(), done)) it('calls to close during connection throw', done => TESTS['calls to close during connection throw'](config(), done)) it('calls to close during connection call back exactly once', done => TESTS['calls to close during connection call back exactly once'](config(), done)) diff --git a/test/tedious/tedious.js b/test/tedious/tedious.js index 4d874938..d64142d7 100644 --- a/test/tedious/tedious.js +++ b/test/tedious/tedious.js @@ -100,6 +100,8 @@ describe('tedious', () => { it('parameters with length as max', done => TESTS['parameters with length as max'](done)) it('output parameters with a falsy initial value', done => TESTS['output parameters with a falsy initial value'](done)) it('query with error', done => TESTS['query with error'](done)) + it('query with an error that does not end the batch', done => TESTS['query with an error that does not end the batch'](done)) + it('query with a constraint violation', done => TESTS['query with a constraint violation'](true, done)) it('query with multiple errors', done => TESTS['query with multiple errors'](done)) it('query with raiseerror', done => TESTS['query with raiseerror'](done)) it('query with toReadableStream', done => TESTS['query with toReadableStream'](done)) @@ -125,6 +127,7 @@ describe('tedious', () => { it('transaction with error (XACT_ABORT set to ON)', done => TESTS['transaction with error'](done)) it('transaction with synchronous error', done => TESTS['transaction with synchronous error'](done)) it('cancel request', done => TESTS['cancel request'](done, /Canceled./)) + it('cancel a running request', done => TESTS['cancel a running request'](true, done)) it('allows repeat calls to connect', done => TESTS['repeat calls to connect resolve'](config(), done)) it('calls to close during connection throw', done => TESTS['calls to close during connection throw'](config(), done)) it('calls to close during connection call back exactly once', done => TESTS['calls to close during connection call back exactly once'](config(), done))