5.6.0: on cancel, 'done' is emitted before the query has ended, and also when nothing was cancelled

Version: 5.6.0. The code involved is older, but #470 means it now runs for every query.
Platform:

Seen in CI: Windows (GitHub windows-2022 and windows-2025 images), Node 22.23.3 and 24.21.0, SQL Server 2016, 2017, 2019 and 2022, SQL Server Native Client 11.0.
Points 2 and 3 below come from reading the 5.6.0 source only. I could not run msnodesqlv8 myself (macOS, no ODBC).

Thanks for #470 and #473. Being able to cancel an ordinary streaming query is a real improvement, and node-mssql's request.cancel() now actually stops a running query. I have three questions about the events a cancel produces. They came up while working out why node-mssql's CI went red on 5.6.0 (run 38225080240). We have already changed node-mssql to cope with the current behaviour (tediousjs/node-mssql#1917), so none of this is urgent for us. I'm reporting it so the event contract around cancel is settled, and other event-API consumers don't have to work it out the way we did.

1. 'done' arrives before the query ends, and the HY008 'error' comes after it (seen in CI)

When you cancel a running, non-polling query, the sequence is done → cancel callback → error HY008 [msnodesql] Operation canceled (more=false) → free.

node-mssql's test a cancelled stream emits done event cancels select * from streaming at row 10. It failed the same way in all 16 Node 22 and 24 jobs. In the Node 24 / SQL Server 2019 job, for example, 'done' was handled at 09:31:55.380Z and the HY008 followed about 3 ms later, after the test had already finished:

RequestError: [msnodesql] Operation canceled
    at handleError (lib/msnodesqlv8/request.js:344:21)          <- node-mssql's 'error' listener
    at node_modules/msnodesqlv8/lib/reader.js:97:20
    at Query.routeStatementError (node_modules/msnodesqlv8/lib/reader.js:87:19)
    at Query.end (node_modules/msnodesqlv8/lib/reader.js:306:12)
    at node_modules/msnodesqlv8/lib/reader.js:418:12
  { state: 'HY008', number: 0, class: 0, serverName: '', lineNumber: 0 }

The empty serverName and the 0 fields suggest this is the synthetic error from return_cancelled(), not one reported by the driver.

2. 'done' is emitted when the cancel callback gets an error (from code; plain JS, so it should be deterministic)

connection.cancelQuery emits 'done' before it passes on e, whatever e is. So when DriverMgr.cancel fails with cannot cancel query (empty queue|not found), the query gets another 'done'. One way to hit this is to cancel a query that has already finished.

On 5.5.0 a non-polling query never reached this code, because the guard sent back cancel only supported… instead. Polling queries already took this path. Upstream's own test cancel single query and cancel again goes through it; it checks the error but not the extra 'done'.

3. A cancel that the native statement refuses is reported as success (from code, not run; please check)

This is the one I'm least sure of, and the one that matters most if it's right.

OdbcStatementLegacy::Cancel() only calls SQLCancelHandle for a non-polling statement in STATEMENT_SUBMITTED or STATEMENT_READING. In any other state it pushes a cancel only supported… error and returns false.
return_odbc_error() sets STATEMENT_ERROR whenever there is any diagnostic record, and that includes info records. try_execute_direct calls it on SQL_SUCCESS_WITH_INFO, and READING is only set once, straight after SQLExecDirect.
So a query whose execute returns info has state STATEMENT_ERROR while it streams. A leading PRINT would do it. Unless I've missed somewhere that resets the state, I'd also expect it after a fix: prints, errors and results after a failed statement in a batch were lost #475 carry-on error.
For that query:
- The native cancel is refused.
- CancelWorker::OnOK calls back (null, false), and forwardCancel only treats an array as an error.
- JS therefore sees success: 'done' is emitted and the callback gets null.
- Meanwhile the query keeps streaming rows and later emits its own 'done'.
Because nothing is cancelled and nothing reports an error, I think this case is new in 5.6.0 in practice. On 5.5.0 the JS guard refused it first. On the paused path the statement is freed anyway, and polling always returns true.
I haven't traced whether the cancel only supported… record, which Cancel() writes to _errors from the cancel thread, comes out later as an 'error'.

Repro (msnodesqlv8 only)

// MSSQL_CS="Driver={ODBC Driver 18 for SQL Server};Server=...;UID=...;PWD=...;TrustServerCertificate=yes" node cancel-order.js
'use strict'
const sql = require('msnodesqlv8')
const { version } = require('msnodesqlv8/package.json')
const big = 'select top 5000 a.name from sys.all_objects a cross join sys.all_objects b'

function trace (q, log) {
  q.on('error', (e, more) => log(`error ${e.sqlstate} ${e.message} more=${more}`))
  q.on('done', () => log('done'))
  q.on('free', () => log('free'))
}

// A / C: cancel a running, non-polling query at row 100
function runningCancel (conn, text) {
  return new Promise(resolve => {
    const out = []
    let rows = 0
    const q = conn.query(text)
    q.on('row', () => {
      if (++rows === 100) {
        out.push('cancelQuery()')
        q.cancelQuery(e => out.push(`cancel cb: ${e ? e.message : 'no error'}`))
      }
    })
    trace(q, m => out.push(`${m} [rows=${rows}]`))
    q.on('free', () => setTimeout(() => resolve(out), 200))
  })
}

// B: cancel a query that has already finished
function lateCancel (conn) {
  return new Promise(resolve => {
    const out = []
    const q = conn.query('select 1 as n')
    trace(q, m => out.push(m))
    q.on('free', () => setTimeout(() => {
      out.push('cancelQuery()')
      q.cancelQuery(e => {
        out.push(`cancel cb: ${e ? e.message : 'no error'}`)
        setTimeout(() => resolve(out), 200)
      })
    }, 50))
  })
}

sql.open(process.env.MSSQL_CS, async (err, conn) => {
  if (err) throw err
  console.log(`msnodesqlv8 ${version} ${process.platform} node ${process.version}`)
  console.log('A running cancel\n  ' + (await runningCancel(conn, big)).join('\n  '))
  console.log('B late cancel\n  ' + (await lateCancel(conn)).join('\n  '))
  console.log('C running cancel after a print\n  ' + (await runningCancel(conn, `print 'start'; ${big}`)).join('\n  '))
  conn.close(() => {})
})

Sequences

These are derived from the code. CI observed only the order of done and error in A. The row count at the cancel depends on batch timing.

	5.5.0	5.6.0 (actual)	Suggested
A running cancel	`cancelQuery()` → cb: `cancel only supported…` → `done` [rows=5000] → `free`	`cancelQuery()` → `done` [rows≈100] → cb: no error → `error HY008 … more=false` → `free`	`cancelQuery()` → cb: no error → `error HY008 … more=false` → `done` → `free`
B late cancel	`done` → `free` → `cancelQuery()` → cb: `cancel only supported…`	`done` → `free` → `cancelQuery()` → `done` → cb: `cannot cancel query (empty queue)…`	`done` → `free` → `cancelQuery()` → cb: `cannot cancel query…`
C cancel after a `PRINT`	as A on 5.5.0	`cancelQuery()` → `done` [rows≈100] → cb: no error → rows continue → `done` [rows=5000] → `free`	as A, or the cb gets an error and there is no early `done`

With sql.PollingQuery(...), I'd expect 5.5.0 to already give the 5.6.0 order for A and B, because the 'done' emit has been there since c6b394a (2019, #120).

Cause

lib/connection.js:356-360 and :366-369: notify.emit('done') runs in the driverMgr.cancel callback, before callback(e), whatever e is.
- That callback fires as soon as SQLCancelHandle returns.
- Since fix: segv cancelling a non polling query while rows are fetched #473, the worker thread ends the query afterwards: return_cancelled(), odbc_statement_legacy.cpp:680-698.
lib/reader.js:293-307: end() checks only the reader's own this.done, which the 'done' from cancelQuery never sets. It then routes the HY008 as 'error'.
- asCancelled() (:317) already checks notify.getCancelSent(), so the reader does know a cancel was sent at this point.
lib/driver.js:48-58: forwardCancel only treats err && err.length > 0 as an error.
lib/driver.js:137-155: these are the cannot cancel query… errors that still lead to 'done'.
cpp/include/js/workers/cancel_worker.h:34-37: OnOK calls back (null, success_), so a native refusal arrives as success.
cpp/src/odbc/odbc_statement_legacy.cpp:
- :341-377: Cancel(). The refusal branch writes _errors from the cancel thread (:376).
- :656-671: return_odbc_error() sets STATEMENT_ERROR for any diagnostic.
- :1184-1185: the only transition to READING.
- :1217-1220: SQL_SUCCESS_WITH_INFO at execute.

Suggested fix

These are only ideas; you know the lifecycle better than I do.

Emit 'done' only for a cancel that worked. In connection.cancelQuery, emit it only when e is null.
Report native refusals.
- Either CancelWorker::OnOK passes an Error when success_ is false, or forwardCancel honours the boolean (and accepts a non-array error).
- Separately, should a running statement in STATEMENT_ERROR because of info-only diagnostics still be cancellable? One option is to have return_odbc_error() set STATEMENT_ERROR only when a record is not 01xxx.
Keep 'done' on cancel, but let it be last.
- node-mssql asked for 'done' on cancel in done event not fired on cancelled queries #120, so we'd rather keep it.
- For a query the reader will end itself (anything other than the paused-and-freed branch in DriverMgr.drop), the 'done' could come from reader.end() after the HY008 is routed, when notify.getCancelSent() is true.
- A once-only guard at the notifier level would stop a query ever emitting two 'done's.
- If you'd rather keep the current order, documenting "on cancel, 'error' may follow 'done'; 'free' is always last" would also work for us.
Add an event-order test for cancel, alongside the events() helper in batch-errors.test.js. Cover a running query, a streaming query, a paused query and a late cancel. Assert:
- at most one 'done';
- only 'free' after 'done';
- no 'done' when the cancel callback gets an error.

Docs (small, related)

lib/index.d.ts is unchanged from 5.5.0, so it predates both this change and fix: prints, errors and results after a failed statement in a batch were lost #475:
- Query.on still says 'error' is a "critical error that caused the query to end" (:863) and that 'done' means the "query is complete" (:869).
- ErrorEventCb is (err: Error) => void (:1005), with no more argument.
The wiki's "Cancel A Query" section still says only polling queries can be cancelled.

The release notes and the api-tour already describe the new behaviour well. If the typings described it as well, TypeScript users would see it too.

I found two more things on the cancel path while reviewing tediousjs/node-mssql#1917. Both involve a paused query and the isExecuting() check that #470 added to DriverMgr.drop(). Same caveat as before: this comes from reading the 5.6.0 source, which is unchanged on master at 1353ab0, and I couldn't run any of it. The numbering carries on from the issue.

4. A paused polling query that is cancelled mid-stream never ends, and blocks the connection

isExecuting() counts STATEMENT_POLLING as executing (lib/driver.js:21, :103-106).
try_execute_direct takes a polling statement from SUBMITTED to READING to POLLING (odbc_statement_legacy.cpp:1175, :1184-1185, :1189-1190). A polling statement that succeeds stays in POLLING after poll_check returns. Only a diagnostic (STATEMENT_ERROR, :669) or Close() moves it on.
- So a polling query that is paused while it streams rows still counts as executing.
drop() therefore un-parks the work item and cancels it as the active query, without freeing it (driver.js:111-117).
Cancel() on a polling statement only sets _cancelRequested (odbc_statement_legacy.cpp:368-370). Nothing on the fetch path reads that flag: fetch_read (:226-262) only checks _cancelHandleIssued.
The reader is never told. Query.paused (reader.js:476) stays true, so dispatch() returns early (reader.js:392-395).

What an event consumer sees:

The query gets done and a cancel callback with no error, and then nothing else: no error and no free.
The QUERY item is now the head of the work queue and is no longer parked, so nothing queued after it runs (queue.js:31-37). A follow-up conn.query() or conn.close() never calls back.
If the caller resumes, the remaining rows stream after the done, and a second done follows them.

On 5.5.x the same pause and cancel took the paused branch and freed the statement, so I think this is new in 5.6.0.

A non-polling query can reach this branch too, in a narrow window. lastStateChange arrives through a thread-safe function, so JS can still see SUBMITTED after SQLExecDirect has returned. SQLCancelHandle on the idle statement then only closes the cursor (as the comment at odbc_statement_legacy.cpp:675-679 says), and the parked reader never makes the fetch that would report it.

// uses trace() from the repro above
const q = conn.query(sql.PollingQuery(big))
let rows = 0
q.on('row', () => {
  if (++rows === 100) {
    q.pauseQuery()
    q.cancelQuery(e => {
      console.log('cancel cb', e ? e.message : 'no error')
      conn.query('select 1 as n', (err, res) => console.log('follow-up', err ? err.message : res))
    })
  }
})
trace(q, console.log)

Expected: the query ends with free, and the follow-up prints [ { n: 1 } ].

What I expect from the code on 5.6.0: done, then cancel cb no error, and nothing after that. The follow-up never calls back.

To check the branch, turn on sql.logger.setLogLevel('DEBUG') and sql.logger.setConsoleLogging(true). You should then see drop called for query id …, paused: true, executing: true (driver.js:110), followed by un-park executing query id … to cancel (driver.js:115). This assumes the polling fetches return no diagnostics. One would move the state to STATEMENT_ERROR, and the old free branch would run instead.

5. A cancel requested before the handle is known isn't caught by `isExecuting()`

A cancel requested before there is a statement handle is held as cancelToken (notifier.js:173-178). The state-change callback fires it (:209-212) before it runs this.lastStateChange = stateChange (:213).
So on the first state change, isExecuting() sees lastStateChange === null and returns false (driver.js:104-105). A paused query then takes the branch that cancels and then calls freeStatement (driver.js:118-126). That is the case isExecuting() was added to prevent (see the comment at notifier.js:63-65).
Swapping :209-212 and :213 would not be enough on its own:
- The first change JS sees is CREATED → BINDING. The constructor sets CREATED (odbc_statement_legacy.cpp:203) before the notifier is attached (odbc_connection.cpp:351-353), and BINDING (:1136) is not in executingStates.
- By the time JS handles that change, the worker is usually already inside SQLExecDirect.
- If Cancel() runs while the native state is still BINDING, the cancel is refused but reported as success (point 3).
What an event consumer sees:
- done, the cancel callback (with no error) and free can all arrive, in that order, before the statement has stopped executing.
- The statement can be released (ConnectionHandles::checkin frees the handle) while the worker is still using it. fix: cancel leaving the connection busy, pause ending queries early, nvarchar output params #470's description shows what that looks like in the native log: SafeHandle freed while still referenced.
- What follows depends on where the native cancel landed. If it landed in SUBMITTED, then as far as I can tell the reader's execute-error path runs when the worker returns. That would put the HY008 error after free, followed by a second free.

This happens when the query is paused and cancelled before its first state change reaches JS, for example in the same tick as query(). The reader parks itself in setQueryWorker() before begin() (notifier.js:154-160, driver.js:283-287).

const q = conn.query("waitfor delay '00:00:05'; select 1 as n")
q.pauseQuery()
q.cancelQuery(e => console.log('cancel cb', e ? e.message : 'no error'))
trace(q, console.log)

With DEBUG logging on, the drop called for query id … line (driver.js:110) should show paused: true, executing: false. 5.5.x also freed the statement on this path, so this isn't a regression; the new check just doesn't cover it.

Suggested fix (ideas only)

For 4:
- Move a polling statement to READING once poll_check returns (odbc_statement_legacy.cpp:1189-1191). POLLING would then mean "still executing", as SUBMITTED does, and a paused polling query that is streaming would take the free branch again, as on 5.5.x.
- Separately, in drop()'s executing branch, have the reader end rather than park once notify.getCancelSent() is true: route HY008 and free the statement.
  - Just clearing the pause would not be enough for a polling query. Its cursor is still open and the fetch never reads _cancelRequested, so it would stream the remaining rows after the early done.
  - In the non-polling window the cursor is closed, so the next fetch fails with HY010 or 24000, which asCancelled() already reports as HY008.
For 5: set lastStateChange before firing the held cancel, and count every state up to the end of execution as executing: null, BINDING, SUBMITTED and POLLING. That sends more cancels down point 4's branch, so the reader fix there matters for both.
The event-order test suggested in the issue would catch both if it also covered a query paused before it executes and a paused polling query.

Merge pull request #487 from TimelordUK/fix/done-every-query

Merge pull request #488 from TimelordUK/fix/cancel-after-print

Fixed on master in #487, #488 and #490. It will ship in v6, which is not released yet. That is a major version because the event contract below changes behaviour that workarounds may depend on.

The contract in v6

done means no more results. It is raised on every query, exactly once, whatever happened: success, error, bind failure, cancel or refusal.
free means the native statement has been released. It always comes after done.
Nothing comes after done except free.

Cancel

An accepted cancel always ends the query with cancel-cb ok → error HY008 → done → free.
A cancel that is not accepted gets only an error in its own callback, and the query's events are untouched. That covers a cancel that is too late, one the driver refuses, and a repeat: there is one cancel per query, and a second is refused with "cancel already requested". Every cancel's callback is called.

What was wrong

done came from cancelQuery itself: before the HY008, and again on a late cancel (A, B).
Info-only diagnostics such as a PRINT put a still-reading statement into an error state natively, so the cancel was silently refused and the query read on (C). The same happened after a failed statement mid-batch.
A cancel that landed between native calls ended the query without HY008.
A second cancel's callback was never called.
Error paths never raised done at all.

Your repro is test/contract/scripts/issue-479.js. Its transcript, corpus/scripts/issue-479.txt, and all cancel/* scenarios are now agreed and checked on every PR.

There is one Linux caveat, unrelated to cancel: #483. After an error at execute, the rest of the batch is lost on Linux, so a cancel issued after such an error finds nothing to cancel there.

Navigation Menu

Repository navigation

5.6.0: on cancel, 'done' is emitted before the query has ended, and also when nothing was cancelled #479

1. 'done' arrives before the query ends, and the HY008 'error' comes after it (seen in CI)

2. 'done' is emitted when the cancel callback gets an error (from code; plain JS, so it should be deterministic)

3. A cancel that the native statement refuses is reported as success (from code, not run; please check)

Repro (msnodesqlv8 only)

Sequences

Cause

Suggested fix

Docs (small, related)

4. A paused polling query that is cancelled mid-stream never ends, and blocks the connection

5. A cancel requested before the handle is known isn't caught by `isExecuting()`

Suggested fix (ideas only)

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

5.6.0: on cancel, 'done' is emitted before the query has ended, and also when nothing was cancelled #479

Description

1. 'done' arrives before the query ends, and the HY008 'error' comes after it (seen in CI)

2. 'done' is emitted when the cancel callback gets an error (from code; plain JS, so it should be deterministic)

3. A cancel that the native statement refuses is reported as success (from code, not run; please check)

Repro (msnodesqlv8 only)

Sequences

Cause

Suggested fix

Docs (small, related)

Activity

dhensby commented on Oct 13, 2026

4. A paused polling query that is cancelled mid-stream never ends, and blocks the connection

5. A cancel requested before the handle is known isn't caught by isExecuting()

Suggested fix (ideas only)

TimelordUK commented on Oct 18, 2026

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions

5. A cancel requested before the handle is known isn't caught by `isExecuting()`