Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 44 additions & 1 deletion lib/notifier.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ const notifyModule = ((() => {
this.operation = null
this.paused = null
this.canelSent = false
// a cancel sent whose own callback has not run yet, and what waits for it to run
this.cancelPending = false
this.afterCancelSteps = []
this.prepared = null
this.handle = null
this.cancelToken = null
Expand Down Expand Up @@ -181,7 +184,26 @@ const notifyModule = ((() => {
logger.debugLazy(() => `${this.queryId} issue cancel request`)
this.cancelToken = null
this.canelSent = true
this.theConnection.cancelQuery(this, cb)
this.cancelPending = true
// the cancel's callback and the HY008 from the native call in flight complete on
// different threads: the reader holds what that call brings back until this has run
const settled = (e, res) => {
try {
if (cb) {
cb(e, res)
} else if (e) {
this.theConnection.defaultCallback(e)
}
} finally {
this.settleCancel()
}
}
try {
this.theConnection.cancelQuery(this, settled)
} catch (e) {
this.settleCancel()
throw e
}
} else {
logger.debugLazy(() => `${this.queryId} cancel request but no statement handle`)
this.cancelToken = {
Expand All @@ -195,6 +217,27 @@ const notifyModule = ((() => {
}
}

// run a step now, or once the cancel in flight has called back
whenCancelSettled (step) {
if (this.cancelPending) {
this.afterCancelSteps.push(step)
} else {
step()
}
}

// a tick on, so that work the cancel's callback queued (e.g. the refusal of a second
// cancel) still comes first
settleCancel () {
if (!this.cancelPending) return
this.cancelPending = false
const steps = this.afterCancelSteps
this.afterCancelSteps = []
if (steps.length > 0) {
setImmediate(() => steps.forEach(step => step()))
}
}

pauseQuery () {
this.paused = true
if (this.queryWorker) {
Expand Down
28 changes: 18 additions & 10 deletions lib/reader.js
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,12 @@ class Query extends BasePromises {
})
}

// what a native call brings back once a cancel has been sent waits for the cancel's own
// callback: an accepted cancel is always cancel-cb, error HY008, done, free
afterCancel (step) {
this.notify.whenCancelSettled(step)
}

// park a step to run on resume, if paused. true when parked - or, for a paused query
// that has been cancelled, when the query has ended instead.
park (step) {
Expand Down Expand Up @@ -468,15 +474,15 @@ class Query extends BasePromises {
if (this.park(() => this.dispatch())) return

logger.traceLazy(() => `dispatch fetching rows for queryId ${this.queryId}, rowBatchSize=${this.rowBatchSize}`, this.context)
this.nativeGetRows(this.queryId, this.rowBatchSize).then(d => {
this.nativeGetRows(this.queryId, this.rowBatchSize).then(d => this.afterCancel(() => {
logger.traceLazy(() => `dispatch received ${d?.data?.length || 0} rows for queryId ${this.queryId}, endOfRows=${d?.endOfRows} endOfResults=${d?.endOfResults}`, this.context)
this.batchRowIndex = 0
this.batchData = d
this.continueBatch()
}).catch(err => {
})).catch(err => this.afterCancel(() => {
logger.debugLazy(() => `dispatch error for queryId ${this.queryId}: ${err}`, 'DriverRead.dispatch', this.context)
this.endOrPark(err)
})
}))
}

// emit what is left of the current batch, then fetch more, or move to the next result.
Expand Down Expand Up @@ -507,11 +513,11 @@ class Query extends BasePromises {
if (this.park(() => this.nextResult())) return
this.infoFromNextResult = false
this.nativeNextResult(this.queryId)
.then(nextResultSetInfo => {
.then(nextResultSetInfo => this.afterCancel(() => {
this.moveToNextResult(nextResultSetInfo)
}).catch(err => {
})).catch(err => this.afterCancel(() => {
this.endOrPark(err)
})
}))
}

// the query has executed: report what came with it, then read on
Expand Down Expand Up @@ -546,9 +552,11 @@ class Query extends BasePromises {
// the handle at once, so a cancel can reach the statement; what is emitted waits
// for a resume, if the query was paused while it executed
this.notify.setHandle(res.queryResult.handle)
const begun = () => this.begun(res)
if (!this.park(begun)) begun()
}).catch(err => {
this.afterCancel(() => {
const begun = () => this.begun(res)
if (!this.park(begun)) begun()
})
}).catch(err => this.afterCancel(() => {
logger.debugLazy(() => `dispatch error for queryId ${this.queryId}: ${err}`, this.context)
// Don't call end() here - it's already handled in beginQuery for binding errors
try {
Expand All @@ -567,7 +575,7 @@ class Query extends BasePromises {
this.close()
}
}
})
}))
this.notify.emit('submitted', this.query, this.params)
}

Expand Down
21 changes: 10 additions & 11 deletions test/contract/HANDOVER.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,8 @@ ODBC and has the same blind spot, so it is not a reference).
- `QueryWorker` (direct execute) still calls back `(null, meta)` on success; info at execute
reaches JS because `try_execute_direct` returns false on `SQL_SUCCESS_WITH_INFO`. Works, but it
is not the same shape as the other workers.
- `cancel/after-error-mid-batch` shows the cancel-cb / HY008 race (about 1 in 10, on master's
native code too): it has an `--- or` alternative now.
- `cancel/after-error-mid-batch` showed the cancel-cb / HY008 race (about 1 in 10, on master's
native code too). Fixed since: see the cancel race under Smaller items.

**To review in the `info/` transcripts** (all `unreviewed`):
- each PRINT, and each statement with no result (a DDL), is followed by `rowcount 0`; an insert
Expand Down Expand Up @@ -354,15 +354,14 @@ that replay covers them.
- #483 (Linux, native) on WSL. When it is fixed, the `known-bug` Linux pins show as diffs:
`errors/{raiserror-then-result,dup-key-d}.*.linux.txt`, `scripts/issue-478.linux.txt`,
`cancel/after-error-mid-batch.events.linux.txt`.
- A race in accepted cancels: the cancel's own callback (`cancel-cb ok`) and the HY008 from the
fetch in flight are two native completions on different threads, so either can reach JS
first. CI showed it on `cancel/twice-streaming-at-100`, which now has an `--- or`
alternative (the first in the corpus). `cancel/again-after-confirm-at-100` failed once
locally, probably the same race, but its transcript was not kept. Other accepted cancels
of a statement in flight can show it too: add the alternative when one does. Or tighten the
contract: hold the HY008 in JS until the cancel callback has run, so that `cancel-cb ok`
always comes first. That is the owner's call. Note that `capture.js --update` rewrites a file
from what it saw, so it drops an alternative it did not see.
- ~~A race in accepted cancels~~, fixed (2026-10-22). The cancel's own callback (`cancel-cb ok`)
and the HY008 from the fetch in flight are two native completions on different threads, so
either could reach JS first. CI showed it on `cancel/twice-streaming-at-100`,
`cancel/again-after-confirm-at-100` and `scripts/issue-479`. Now, once a cancel is sent,
`StreamEvents` holds what a native call brings back to the reader (`afterCancel` in
`begin`, `dispatch`, `nextResult`) until the cancel's callback has run, then releases it a
tick on, after anything that callback queued. An accepted cancel is always `cancel-cb ok`,
`error HY008`, `done`, `free`, and the corpus no longer has `--- or` alternatives for it.
- The 36 `unreviewed` transcripts (listed under Where we are): read them against the
contract and agree them. `read/print-*` belong with the `info/` work above.

Expand Down
9 changes: 0 additions & 9 deletions test/contract/corpus/cancel/after-error-mid-batch.events.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,3 @@ error HY008 0 Operation canceled more=false
done
free
check ok
--- or
submitted
error 42000 50000 boom more=true
action cancel
error HY008 0 Operation canceled more=false
cancel-cb ok
done
free
check ok
11 changes: 0 additions & 11 deletions test/contract/corpus/cancel/twice-streaming-at-100.events.txt
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,3 @@ error HY008 0 Operation canceled more=false
done
free
check ok
--- or
submitted
meta name
row x100
action cancel x2
cancel-cb error cannot cancel query (cancel already requested) id 0
error HY008 0 Operation canceled more=false
cancel-cb ok
done
free
check ok
Loading