Fix #3736 | Propagate Errors from ExecuteScalar by samsharma2700 · Pull Request #3912 · dotnet/SqlClient

samsharma2700 · 2026-01-23T21:10:40Z

Description

Fix ExecuteScalar to propagate errors when server sends data followed by error token

Fixes #3736

This PR fixes an issue where ExecuteScalar() swallows server errors that occur after the first result row is returned. When SQL Server sends data followed by an error token (e.g., a conversion error during WHERE clause evaluation), the error was being silently consumed during reader.Close() instead of being thrown to the caller.

Problem

When executing a query like SELECT Id FROM tab1 WHERE Val = 12345 against a table containing values that can't be converted (e.g., '42-43'), SQL Server:

Returns matching rows (where conversion succeeds)
Sends an error token for rows where conversion fails

The original code only read the first result and immediately closed the reader. The Close() method consumed the remaining TDS stream (including the error token) but the error was never propagated to the caller.

This caused serious issues in transactional scenarios:

The transaction would be silently "zombied" by the server
Subsequent commands appeared to succeed but ran outside the transaction
Data integrity issues from partial commits

Solution

In CompleteExecuteScalar(), drain all remaining result sets by calling NextResult() until it returns false before closing the reader. This ensures any pending error tokens are processed and thrown as exceptions.

Changes

SqlCommand.Scalar.cs: Added result draining loop in CompleteExecuteScalar() for the sync path
SqlCommandExecuteScalarTest.cs: Added 4 regression tests

Copilot

Pull request overview

This PR addresses GitHub issue #3736, which reports that ExecuteScalar() swallows server errors that occur after the first result row is returned. This causes transaction "zombification" where the transaction appears to succeed but is actually rolled back by the server, leading to data integrity issues.

Changes:

Adds result draining logic to CompleteExecuteScalar() in the synchronous path to ensure error tokens are processed before returning
Adds comprehensive regression tests for the synchronous ExecuteScalar method, including transaction scenarios
Updates the project file to include the new test file

Reviewed changes

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

File	Description
SqlCommand.Scalar.cs	Adds a while loop to drain remaining result sets in the non-batch sync path, ensuring error tokens are processed
SqlCommandExecuteScalarTest.cs	Adds 4 regression tests covering conversion errors with and without transactions, plus sanity tests for normal functionality
Microsoft.Data.SqlClient.ManualTesting.Tests.csproj	Adds reference to the new test file

src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandExecuteScalarTest.cs

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Scalar.cs

src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandExecuteScalarTest.cs

benrr101

I think this change needs some work to ensure we're fixing it in sync and async, as well as ensure we're testing what we actually needs to be tested.

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Scalar.cs

src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandExecuteScalarTest.cs

Copilot

Pull request overview

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

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Scalar.cs

…amsharma2700/issue3736fix

Copilot

Pull request overview

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

codecov · 2026-01-30T17:57:53Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0.00%. Comparing base (88de2b9) to head (47fb5e5).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #3912       +/-   ##
==========================================
- Coverage   74.93%       0   -74.94%     
==========================================
  Files         269       0      -269     
  Lines       43247       0    -43247     
==========================================
- Hits        32407       0    -32407     
+ Misses      10840       0    -10840

Flag	Coverage Δ
addons	`?`
netcore	`?`
netfx	`?`

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

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

Backport of PR #3912 (Fix #3736 | Propagate Errors from ExecuteScalar) to release/6.1 branch. Changes: - netcore/netfx SqlCommand.cs: Drain remaining results in CompleteExecuteScalar sync path - netcore/netfx SqlCommand.cs: Remove _batchRPCMode guard in ExecuteScalarUntilEndAsync - MultipleResultsTest.cs: Updated to expect SqlException from ExecuteScalar draining - SqlCommandExecuteScalarTest.cs: New regression tests (sync/async, transactions, multi-result) - ManualTesting.Tests.csproj: Include new test file Fixes #3736

Co-authored-by: paulmedynski <[email protected]> Co-authored-by: copilot-swe-agent[bot] <[email protected]>

Fix github issue 3736

ee91dbc

samsharma2700 requested a review from a team as a code owner January 23, 2026 21:10

Copilot AI review requested due to automatic review settings January 23, 2026 21:10

github-project-automation bot added this to SqlClient Board Jan 23, 2026

github-project-automation bot moved this to To triage in SqlClient Board Jan 23, 2026

Copilot started reviewing on behalf of samsharma2700 January 23, 2026 21:10 View session

Copilot AI reviewed Jan 23, 2026

View reviewed changes

cheenamalhotra reviewed Jan 24, 2026

View reviewed changes

src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandExecuteScalarTest.cs Outdated Show resolved Hide resolved

benrr101 moved this from To triage to In review in SqlClient Board Jan 26, 2026

benrr101 changed the title ~~Fix github issue 3736~~ Fix #3736 | Propagate Errors from ExecuteScalar Jan 26, 2026

benrr101 requested changes Jan 26, 2026

View reviewed changes

github-project-automation bot moved this from In review to In progress in SqlClient Board Jan 26, 2026

samsharma2700 added 2 commits January 28, 2026 13:19

Add async path fix and tests

da9bc62

Consistent exception handling in tests

86b33ba

Copilot AI review requested due to automatic review settings January 28, 2026 22:40

Copilot started reviewing on behalf of samsharma2700 January 28, 2026 22:40 View session

Copilot AI reviewed Jan 28, 2026

View reviewed changes

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Scalar.cs Outdated Show resolved Hide resolved

samsharma2700 requested review from benrr101 and cheenamalhotra January 29, 2026 17:59

samsharma2700 added 2 commits January 29, 2026 12:17

Merge branch 'main' of https://github.com/dotnet/SqlClient into dev/s…

5956481

…amsharma2700/issue3736fix

Update MultipleResultTests

8380594

Copilot AI review requested due to automatic review settings January 29, 2026 22:58

Copilot started reviewing on behalf of samsharma2700 January 29, 2026 22:58 View session

Copilot AI reviewed Jan 29, 2026

View reviewed changes

Fix async path

47fb5e5

priyankatiwari08 self-assigned this Jan 30, 2026

cheenamalhotra approved these changes Feb 2, 2026

View reviewed changes

cheenamalhotra added this to the 7.0.0-preview4 milestone Feb 2, 2026

cheenamalhotra requested a review from mdaigle February 2, 2026 19:57

cheenamalhotra requested a review from paulmedynski February 2, 2026 19:57

cheenamalhotra moved this from In progress to In review in SqlClient Board Feb 2, 2026

benrr101 approved these changes Feb 2, 2026

View reviewed changes

samsharma2700 merged commit a0357b2 into main Feb 3, 2026
272 checks passed

github-project-automation bot moved this from In review to Done in SqlClient Board Feb 3, 2026

samsharma2700 deleted the dev/samsharma2700/issue3736fix branch February 3, 2026 00:18

edwardneal mentioned this pull request Feb 3, 2026

Failed transaction unexpectedly leaves data in table #3736

Closed

samsharma2700 mentioned this pull request Feb 3, 2026

ExecuteScalar does not always raise an exception when selected as deadlock victim #458

Open

samsharma2700 added a commit that referenced this pull request Feb 10, 2026

Port #3912 to release/6.1

10d2b2a

samsharma2700 mentioned this pull request Feb 10, 2026

[6.1] Port #3912 to release/6.1 #3947

Open

paulmedynski added a commit that referenced this pull request Apr 2, 2026

[6.1] Port #3912 to release/6.1 (#4133)

113aab3

Co-authored-by: paulmedynski <[email protected]> Co-authored-by: copilot-swe-agent[bot] <[email protected]>

Conversation

samsharma2700 commented Jan 23, 2026

Description

Problem

Solution

Changes

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!

Uh oh!

Uh oh!

benrr101 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

codecov bot commented Jan 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov bot commented Jan 30, 2026 •

edited

Loading