[dhensby]

Problem

In lib/msnodesqlv8/transaction.js, _commit and _rollback wrap the server's error in a TransactionError and then call callback(null), so both always resolve. This has been the case since v4.0.0 (5e26731).

• Commit after the server has ended the transaction. With XACT_ABORT ON plus a failing statement, the server rolls the transaction back. commit() then resolves and emits 'commit', although nothing was committed: the server answered COMMIT TRANSACTION with error 3902. tedious rejects with EABORT here.
• Rollback after the server has ended the transaction. rollback() resolves (and emits rollback(false)) even though the server answered with error 3903.
• Rollback while a request is in progress. _rollback calls super._commit, so it rejects with "Can't commit transaction. There is a request in progress." (tedious says "Can't rollback…"). It also never sets _rollbackRequested; only the tedious driver reads that flag, so this one is cosmetic on msnodesqlv8.

Fix

• Pass the wrapped error to the callback in _commit and _rollback.
• Call super._rollback from _rollback.

The base class needs no change:

• The connection is still released before the callback runs.
• The 'commit'/'rollback' events, and the matching mssql:transaction:commit/rollback diagnostics channel messages, are only emitted on success. They are no longer emitted for a failed commit or rollback on msnodesqlv8.
• After a failed commit or rollback, a later rollback() gets ENOTBEGUN.

The error is a TransactionError with:

• code EREQUEST
• the server's message
• the RequestError as originalError (number 3902/3903 in the cases above)

On tedious the same situation gives EABORT, because tedious sees the server's rollback (ENVCHANGE) and marks the transaction aborted. The class matches across drivers; the code does not.

Docs

README, "Aborted transactions": that section is written around abortTransactionOnError, which is a tedious-only option. It now has one paragraph for msnodesqlv8: the driver can't tell when the server has rolled a transaction back, so when XACT_ABORT is on, or the server rolls the transaction back for another reason (such as a deadlock), the rollback event isn't emitted and the example's rollback() call fails with a TransactionError.

Changelog caveat

On msnodesqlv8, rollback() now rejects with a TransactionError (code EREQUEST, "The ROLLBACK TRANSACTION request has no corresponding BEGIN TRANSACTION", originalError.number 3903) whenever the server has already rolled the transaction back. It used to resolve silently and emit rollback(false). This affects:

• users who turn XACT_ABORT ON themselves and follow the README's "Aborted transactions" pattern;
• transactions the server picks as a deadlock victim (error 1205), whatever XACT_ABORT is set to. A catch (e) { await tran.rollback(); throw e } block will now throw the rollback's TransactionError instead of the deadlock error, as it already does on tedious (EABORT);
• code that ended the transaction itself (for example by running rollback tran in a request) and then called commit(). commit() now rejects with 3902.

The new behaviour is correct and close to tedious's EABORT, but users will see it.

Tests

Two new shared tests in test/common/tests.js, registered as three tests in both suites:

• commit rejects when the server has already ended the transaction and rollback rejects when the server has already ended the transaction (one shared test, run with each method)
• rollback while a request is in progress reports a rollback error

Design notes:

• The first two run exec sp_executesql N'set xact_abort on; select 1 / 0'. Setting the option inside sp_executesql scopes it to that batch, so the result doesn't depend on the suite's abortTransactionOnError. It also leaves the pooled connection's own setting untouched, which matters for msnodesqlv8, where a parameterless query is a plain batch.
• They assert only instanceof sql.TransactionError, since that is all both drivers agree on.
• The in-progress test cleans up the transaction before asserting, so the no-fix failure leaks no connection.

Verification:

• npm run lint passes; npm run test-unit: 139 passing.
• tedious integration on 19.2.2 and 20.3.4: 150 passing. The one failure is user defined types (no CLR on the local Azure SQL Edge).
• msnodesqlv8 was verified by emulation only. node-mssql's real lib/msnodesqlv8 code ran over a tedious-backed stand-in for the native module. Without the lib fix, all three new tests fail; with it, they pass. The other emulated failures are emulator or local-database limits (bulk/bcp, timeouts, cancel, streaming, CLR) and appear identically with and without the fix.
• transaction with synchronous error, which the msnodesqlv8 parity change registers, passes with this fix on the emulator, run both before and after the XACT_ABORT test.
• A raw-session probe confirmed that the nested SET XACT_ABORT ON rolls the transaction back and leaves the session's own setting as it was.
• Windows CI (msnodesqlv8 v2/v3/v4/5.2.1/^5) is the real check. The emulator can't reproduce msnodesqlv8's native binding or ODBC message prefixes. The assertions don't depend on either: the /rollback/ match is against node-mssql's own message.

Found while bringing the two driver suites in line (#1908, #1910).

🤖 Generated with Claude Code

[Copilot]

🟢 Approval recommended

The focused implementation correctly propagates errors, preserves cleanup, and includes appropriate cross-driver coverage.

0 open findings

What changed in this PR

Fixes msnodesqlv8 transaction finalization so commit and rollback server errors propagate correctly.

Changes:

• Propagates wrapped transaction errors and uses the correct rollback guard.
• Adds shared cross-driver integration coverage.
• Documents msnodesqlv8 behavior after server-side rollback.

File	Description
lib/​msnodesqlv8/​transaction.js	Returns commit/rollback errors and calls the rollback base hook.
test/​common/​tests.js	Adds shared transaction failure tests.
test/​tedious/​tedious.js	Registers the new tests for tedious.
test/​msnodesqlv8/​msnodesqlv8.js	Registers the new tests for msnodesqlv8.
README.md	Documents msnodesqlv8 server-side rollback behavior.

🧠 Review effort: Balanced

---

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

[github-actions]

🎉 This PR is included in version 12.7.5 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀