[dhensby]

Fixes #1924.

What goes wrong

With msnodesqlv8, a stream paused before its query reaches a result set has the driver paused at once. toReadableStream() leaves a stream like this until it is read, and so does request.pause() before query(). When the stream is resumed, the adapter resumes the driver straight away. msnodesqlv8's Query.resume() then fetches at once, whatever the query is doing. If begin() (its SQLExecute) or a nextResult() is still running, that fetch lands on a statement with nothing to fetch yet. Depending on timing, one of these happens:

• ODBC fails it with Function sequence error (HY010), which the stream reports as an extra error.
• The query's own error is lost, so a failing query looks like it succeeded.
• node crashes.

The fix

The early pause stays, because a cancel needs it to reach a stream paused before it runs. Only the matching resume is held back, and only once msnodesqlv8 has started the query, which it signals with 'submitted':

• If the query reaches a result set: the driver is resumed a tick after its first 'meta'. By then begin() or nextResult() has handed over, and dispatch() has returned because the query is paused.
• If the query ends without a result set: the driver is never resumed. The query needs no resume to finish, and fix: stop msnodesqlv8 streams resumed after they end from hanging the connection #1921 already stops pause and resume reaching a query that has ended.
• If the stream is resumed before msnodesqlv8 starts the query: the driver is resumed at once, as before. There is no native call yet, so the resume only means the query starts unpaused. Holding it back would leave the query paused while it executes. On msnodesqlv8 < 5.6 a cancel would then be accepted, and the driver would free the statement while it is still executing, where today the cancel is refused and the query runs to the end.

Pausing at the end of a 50-row batch is unchanged, because the driver has nothing of its own under way there.

Tests

Unit tests (test/common/unit.js):

• A query held back before its first result set is not resumed while it executes, nor while the driver emits its 'meta', even when a 'recordset' listener resumes the stream. It is resumed a tick later.
• A query whose stream is paused once it is running, by a 'rowsaffected' listener, is not resumed while it moves on through its statements. It is resumed a tick after its first 'meta'.
• A query held back that ends without a result set is never resumed.
• A query held back and resumed before msnodesqlv8 starts it is resumed at once.
• The fix: stop msnodesqlv8 streams resumed after they end from hanging the connection #1921 test for an error that does not end the batch now plays its rows out the way the driver does once it is resumed. It also checks that the resume waits for the next result set.

Each of nine mutants of the fix fails at least one of these tests.

Integration test (resuming a stream held back while its query executes), run against both drivers in two variants, one passing and one failing:

• The stream is paused before it runs a query that starts with waitfor delay '00:00:01', and resumed 300 ms in.
• It should stream either the query's one row with no error, or its one 208 error with no rows.

Live check on Windows

These runs used SQL Server 2022 on Windows CI, on a throwaway fork branch that has since been deleted. For each msnodesqlv8 version, the two new integration tests ran with master's adapter and with this one. The full msnodesqlv8 suite then ran with the fix. All of this was done twice: once on an earlier revision of this fix, and once on the commit in this PR.

msnodesqlv8	master's adapter (6 runs)	this PR (3 runs)	full suite with this PR
^2 (node 18)	3 failed: node crashed twice, and a query that succeeds lost its row once	3 passed	161 passing
^3 (node 20)	2 failed: the query's 208 error was lost both times	3 passed	158 passing
^4 (node 22)	3 failed: the 208 error was lost twice, and replaced by Function sequence error once	3 passed	158 passing
5.2.1 (node 20)	6 failed: an extra Function sequence error four times, node crashed twice	3 passed	158 passing
^5 (5.6.0, node 22)	6 failed: an extra Function sequence error four times, node crashed twice	3 passed	158 passing

What the failures looked like:

• An extra error: the stream got the driver manager's [Microsoft][ODBC Driver Manager] Function sequence error (number 0) as well as the query's own 208.
• A lost error: the failing query completed with no error at all.
• A lost row: the query that succeeds completed with no row and no error.
• The query that succeeds failing: on 5.2.1 and 5.6.0, in one run each, it failed with Statement handle was freed before results could be returned.

Master's adapter failed 20 of the 30 runs. The rest passed because the result depends on timing. The fix passed all 30 runs across both revisions, and the full suite on every version both times.

Local checks

• npm run lint: clean.
• npm run test-unit: 234 passing.
• test-tedious against Azure SQL Edge: 179 passing. The 3 failures are the CLR-dependent tests that always fail on SQL Edge: user defined types, and the spatial and hierarchyid bulk loads.

msnodesqlv8 v6

msnodesqlv8's unreleased upstream master fixes this HY010 itself. It also holds back a paused query's meta until the query is resumed, so on master this held resume never fires and the stream hangs. The gate has to be switched off for v6 before v6 is released. #1940 tracks that.

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

Cancellation after a deferred resume can free a still-executing statement on older msnodesqlv8 versions.

1 open finding

• Cancellation frees executing statement after resume before first meta · New

What changed in this PR

Fixes premature msnodesqlv8 stream resumes by waiting until the driver reaches its first result set.

Changes:

• Defers resumes after query submission while preserving pre-submission behavior.
• Adds unit coverage for resume timing and queries without results.
• Adds shared success and failure integration tests for both drivers.

File	Description
test/​tedious/​tedious.js	Registers shared resume regression tests.
test/​msnodesqlv8/​msnodesqlv8.js	Registers shared resume regression tests.
test/​common/​unit.js	Tests deferred and immediate resume paths.
test/​common/​tests.js	Adds delayed-query streaming regression tests.
lib/​msnodesqlv8/​request.js	Guards early resumes using driver lifecycle events.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[dhensby]

The window is real, and it is wider than this PR. I ran live probes on Windows CI (SQL Server 2022) with msnodesqlv8 2.7, 3.1, 4.5, 5.2.1 and 5.6.0:

• The cancel lands while the query is still in SQLExecDirect: safe on every version, whether the stream was resumed or not. The stream ends with ECANCEL within 10 ms, and the next query on the connection runs.
• The cancel lands after the server has flushed an earlier statement's row count: here msnodesqlv8 is in SQLMoreResults, and cancelling the paused query segfaults the process on every version. The query was set nocount off; insert ...; raiserror(..., 0, 1) with nowait; waitfor ...; select .... It crashes on master too, for a stream that was never resumed.

So this is the existing paused-cancel path, which this PR also sends the resume-then-cancel sequence down. #1938 fixes it for both cases. It defers the cancel of a query held before its first result set until the query reaches that result set, and reports the query as cancelled if it ends first. Across the five versions, an earlier form of that deferral, with this PR's change on top, ran 120 times without a crash. #1938 merges first, and then this PR is rebased onto it, with its tests changed to expect the deferred cancel.