fix: detect server-aborted transactions to prevent silent auto-commit (XACT_ABORT) by dlevy-msft-sql · Pull Request #370 · microsoft/go-mssqldb

dlevy-msft-sql · 2026-04-17T20:46:52Z

Problem

When XACT_ABORT ON is set and a query causes a server-side error, SQL Server rolls back the transaction and sends ENVCHANGE(RollbackTran) which sets sess.tranid to 0. Subsequent operations on the connection send tranid=0 in TDS headers, causing them to execute as auto-commit outside any transaction. This leads to silent data corruption where partial data is committed.

PR #361 (already merged) fixed Rows.Close() to surface the original server error from the failing query. This PR fixes the second, independent bug: the driver had no detection of server-side transaction rollback, so even when the error was surfaced, subsequent operations could still silently run as auto-commit if the caller ignored the error.

Reproduction scenario (from #244):

SET XACT_ABORT ON via SessionInitSQL
Begin transaction
INSERT INTO Orders (succeeds)
SELECT ... WHERE code = @code with integer parameter against VARCHAR column containing non-numeric values
SQL Server aborts the transaction due to the conversion error
Bug: Second INSERT INTO Orders runs as auto-commit (tranid=0), gets committed
tx.Commit() fails, but the second INSERT is already committed
Result: 1 row instead of 0 (data corruption)

Root Cause

No detection of server-side transaction rollback. When SQL Server rolls back a transaction due to XACT_ABORT, it sends ENVCHANGE(RollbackTran) that sets sess.tranid = 0. The driver continued sending queries with tranid=0 in the TDS headers, which the server interprets as auto-commit.

Fix

Conn.inTransaction: New bool field tracking whether Begin() was called and Commit()/Rollback() hasn't completed yet.
checkServerAbortedTransaction(): Returns mssql.Error when inTransaction==true but sess.tranid==0. The error type (not net.Error/StreamError/ServerError) ensures checkBadConn passes it through without marking the connection bad or triggering retry.
sendQuery() guard: Shared entry point for both Rows and Rowsq paths, so both standard and sqlexp message-loop users are protected.
Commit() guard: Returns clear error instead of sending commit with tranid=0.
Rows.Close() hardening: First-error-wins semantics, document why doneInProcStruct is intentionally skipped, improve cancel-first comment.

Changes

File	Change
`mssql.go`	Add `inTransaction` field, `checkServerAbortedTransaction()`, guards in `sendQuery()` and `Commit()`, lifecycle tracking in `Begin`/`Commit`/`Rollback`, `Rows.Close()` hardening
`xact_abort_test.go`	Integration tests: `TestXactAbortSurfacesError` (standard path) and `TestXactAbortWithReturnMessage` (sqlexp message-loop path)
`xact_abort_unit_test.go`	13 unit tests for `checkServerAbortedTransaction`, `Rows.Close()` error handling, `Commit()` guard

Testing

TestXactAbortSurfacesError: Reproduces exact Transaction does not return error on next statement after breaking. #244 scenario. Verifies QueryRow().Scan() returns conversion error (8114), subsequent Exec fails with guard error (number 0), 0 rows committed.
TestXactAbortWithReturnMessage: Exercises sqlexp/Rowsq path. Verifies MsgError delivers conversion error, subsequent ExecContext fails with guard error, 0 rows committed.
13 unit tests: Cover checkServerAbortedTransaction (not-in-txn, active, aborted, error type, Commit guard), Rows.Close() (no tokens, normal tokens, doneStruct error, first-error-wins, nextToken error priority, cancel called, doneInProc ignored, no-error-bit).
Regression: TestMessageQueue, TestAdvanceResultSetAfterPartialRead, TestMessageQueueWithErrors, TestIgnoreEmptyResults all pass.

Fixes #244

Community contribution by @mgregur (issue report).

codecov-commenter · 2026-04-18T05:20:50Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.59%. Comparing base (296a83a) to head (57c9217).

Additional details and impacted files

@@             Coverage Diff             @@
##             main     #370       +/-   ##
===========================================
+ Coverage   79.92%   96.59%   +16.67%     
===========================================
  Files          34       91       +57     
  Lines        6656    74106    +67450     
===========================================
+ Hits         5320    71586    +66266     
- Misses       1065     2185     +1120     
- Partials      271      335       +64

Flag	Coverage Δ
unittests	`96.53% <100.00%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
mssql.go	`88.81% <100.00%> (+0.58%)`	⬆️

... and 59 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

shueybubbles · 2026-04-20T15:25:04Z

Does this bug exist in the message loop version of running a query (using the experimental interfaces and sqlexp.ReturnMessage) too? How do we know this fix doesn't change or break any users of sqlexp.ReturnMessage?

Copilot

Pull request overview

This PR addresses a correctness gap in the driver when SQL Server aborts a transaction server-side (notably with XACT_ABORT ON), ensuring errors aren’t silently swallowed and preventing subsequent statements from running outside the intended transaction.

Changes:

Add Conn.inTransaction tracking and a pre-query check to detect server-aborted transactions (sess.tranid == 0) and fail fast with an explicit error.
Update Rows.Close() to capture errors surfaced via trailing doneStruct tokens during token draining.
Add a new integration regression test for the #244 scenario plus unit tests for Rows.Close() drain behavior and checkServerAbortedTransaction().

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
`mssql.go`	Tracks “in transaction” state, detects server-aborted transactions before issuing queries, and changes `Rows.Close()` draining/error handling.
`xact_abort_test.go`	Integration regression test reproducing #244 with `XACT_ABORT ON`, validating errors surface and no partial commit occurs.
`xact_abort_unit_test.go`	Unit tests for `Rows.Close()` drain-time error capture and for `checkServerAbortedTransaction()` behavior/type.

dlevy-msft-sql · 2026-04-20T23:14:21Z

Does this bug exist in the message loop version of running a query (using the experimental interfaces and sqlexp.ReturnMessage) too? How do we know this fix doesn't change or break any users of sqlexp.ReturnMessage?

Yes, the bug existed in the sqlexp path too. Both Rows and Rowsq use Stmt.sendQuery(), which is where the guard lives. Without it, any query after a server-side rollback sends tranid=0 regardless of the row-reading path.

sqlexp users already see the original error via MsgError, but if they ignored it and kept using the transaction, they'd hit the same silent auto-commit.

No breakage for sqlexp users. The guard fires in sendQuery() before anything hits the wire -- it doesn't touch the message queue, token channel, or Rowsq logic. The only behavior change: using a dead transaction now returns mssql.Error{Number: 0} instead of silently auto-committing.

Test coverage:

TestXactAbortWithReturnMessage: full Rowsq path with XACT_ABORT, verifies MsgError delivers the conversion error, guard blocks subsequent queries, 0 rows committed
TestMessageQueue, TestAdvanceResultSetAfterPartialRead, TestMessageQueueWithErrors, TestIgnoreEmptyResults: all pass unchanged

Rows.Close() changes only affect the standard Rows path. Rowsq.Close() is untouched.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

shueybubbles

… (XACT_ABORT)

dlevy-msft-sql added this to the v1.11.0 milestone Apr 17, 2026

dlevy-msft-sql force-pushed the fix/xact-abort-error-surface branch 2 times, most recently from 0cb82c2 to 76487e0 Compare April 18, 2026 04:53

dlevy-msft-sql force-pushed the fix/xact-abort-error-surface branch from 76487e0 to a2d2e40 Compare April 20, 2026 14:42

dlevy-msft-sql force-pushed the fix/xact-abort-error-surface branch from a2d2e40 to 062d955 Compare April 20, 2026 18:35

shueybubbles requested a review from Copilot April 20, 2026 21:11

Copilot started reviewing on behalf of shueybubbles April 20, 2026 21:11 View session

Copilot AI reviewed Apr 20, 2026

View reviewed changes

Comment thread mssql.go Outdated

Comment thread mssql.go Outdated

Comment thread xact_abort_test.go Outdated

dlevy-msft-sql force-pushed the fix/xact-abort-error-surface branch from 062d955 to f008f08 Compare April 20, 2026 22:01

dlevy-msft-sql changed the title ~~fix: surface errors from server-aborted transactions (XACT_ABORT)~~ fix: detect server-aborted transactions to prevent silent auto-commit (XACT_ABORT) Apr 20, 2026

dlevy-msft-sql requested a review from Copilot April 20, 2026 23:14

Copilot started reviewing on behalf of dlevy-msft-sql April 20, 2026 23:15 View session

Copilot AI reviewed Apr 20, 2026

View reviewed changes

dlevy-msft-sql force-pushed the fix/xact-abort-error-surface branch 2 times, most recently from 0242063 to d4dbb49 Compare April 21, 2026 02:41

shueybubbles approved these changes Apr 21, 2026

View reviewed changes

dlevy-msft-sql force-pushed the fix/xact-abort-error-surface branch from d4dbb49 to 57c9217 Compare April 21, 2026 14:24

fix: detect server-aborted transactions to prevent silent auto-commit…

57c9217

… (XACT_ABORT)

shueybubbles approved these changes Apr 21, 2026

View reviewed changes

dlevy-msft-sql merged commit 586ea53 into microsoft:main Apr 21, 2026
7 checks passed

dlevy-msft-sql deleted the fix/xact-abort-error-surface branch April 21, 2026 15:07

dlevy-msft-sql mentioned this pull request Apr 21, 2026

chore(main): release 1.10.0 #351

Merged

dlevy-msft-sql modified the milestones: v1.11.0, v1.10.0 Apr 22, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: detect server-aborted transactions to prevent silent auto-commit (XACT_ABORT)#370

fix: detect server-aborted transactions to prevent silent auto-commit (XACT_ABORT)#370
dlevy-msft-sql merged 1 commit into
microsoft:mainfrom
dlevy-msft-sql:fix/xact-abort-error-surface

dlevy-msft-sql commented Apr 17, 2026 •

edited

Loading

Uh oh!

codecov-commenter commented Apr 18, 2026 •

edited

Loading

Uh oh!

shueybubbles commented Apr 20, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dlevy-msft-sql commented Apr 20, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

shueybubbles left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

dlevy-msft-sql commented Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Reproduction scenario (from #244):

Root Cause

Fix

Changes

Testing

Uh oh!

codecov-commenter commented Apr 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

shueybubbles commented Apr 20, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dlevy-msft-sql commented Apr 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

shueybubbles left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dlevy-msft-sql commented Apr 17, 2026 •

edited

Loading

codecov-commenter commented Apr 18, 2026 •

edited

Loading

dlevy-msft-sql commented Apr 20, 2026 •

edited

Loading