Async Generic Helpers by benrr101 · Pull Request #4334 · dotnet/SqlClient

benrr101 · 2026-06-03T03:52:38Z

Description

This is a rebuild of #3705. Very small changes this time. The goal is to make the state object(s) used by the async helpers be generic types, so that the callbacks don't need to unwrap the state object(s) to their type. This provides better type safety for these methods. Additionally, the new versions of the helpers are more consistently written, which will be easier to maintain going forward.

In the previous PR, many of the async helper calls were updated to better utilize the generic state object(s). This made the PR very large and prone to failures. In an effort to get this reviewed more quickly, I've just taken out the most important changes from that PR (the helpers themselves) and made the associated changes smaller (fix calls that were not compiling).

This PR maintains the unobserved exception changes, and should resolve #2104 and #3720

Testing

An entire suite of unit tests for the helpers were added, and the old reflection-based tests were removed.
The PR validation tests have been passing before moving from draft to ready state.

# Conflicts: # src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj # src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.netcore.cs # src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs # src/Microsoft.Data.SqlClient/netfx/src/Microsoft.Data.SqlClient.csproj # src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.netfx.cs # src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs # src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlBulkCopy.cs # src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.NonQuery.cs # src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Reader.cs # src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Xml.cs # src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnection.cs # src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParser.cs # src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs

Copilot

Pull request overview

This PR refactors the internal async continuation/timeout helpers by introducing a new Microsoft.Data.SqlClient.Utilities.AsyncHelper with generic state overloads, updates several product call sites to use the new APIs, and adds new UnitTests coverage (plus a Moq dependency) to validate the helper behaviors.

Changes:

Added a new internal Utilities/AsyncHelper implementation with generic-state continuation helpers and timeout helpers, and removed the legacy AsyncHelper previously embedded in SqlUtil.cs.
Updated multiple async call sites (e.g., SqlCommand, SqlBulkCopy, TdsParser*) to use the new stateful continuation APIs.
Added Moq-based UnitTests for AsyncHelper behaviors plus small Moq helper extensions.

Reviewed changes

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

Show a summary per file

File	Description
src/Microsoft.Data.SqlClient/tests/UnitTests/Utilities/MockExtensions.cs	Adds Moq extension helpers to reduce boilerplate in new unit tests.
src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/Utilities/AsyncHelperTest.cs	Adds extensive unit coverage for continuation helpers and timeout/unobserved-exception behavior.
src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft.Data.SqlClient.UnitTests.csproj	Introduces Moq PackageReference for unit test projects.
src/Microsoft.Data.SqlClient/tests/Directory.Packages.props	Adds central package version for Moq in tests.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Utilities/AsyncHelper.cs	New generic-state async continuation/timeout helper implementation.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs	Converts a continuation callback to use typed state via new helper.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParser.cs	Updates continuation usage to new `CreateContinuationTaskWithState` overloads.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlUtil.cs	Removes legacy internal AsyncHelper previously defined here.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnection.cs	Imports new helper namespace for existing WaitForCompletion usage.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Xml.cs	Updates async continuations to use new helper parameter naming and typed state patterns.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Reader.cs	Updates continuations/timeouts to new helper overloads and typed state.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.NonQuery.cs	Updates continuations to new helper overloads and typed state patterns.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Encryption.cs	Updates continuation usage to new typed-state helper overloads.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlBulkCopy.cs	Updates multiple continuations/timeouts to new helper APIs and typed-state patterns.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs	Imports new helper namespace and adjusts usings/conditionals.

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

codecov · 2026-06-04T19:33:56Z

Codecov Report

❌ Patch coverage is 78.30375% with 110 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.74%. Comparing base (bfbdd30) to head (e516e3b).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
.../Microsoft/Data/SqlClient/Utilities/AsyncHelper.cs	89.47%	44 Missing ⚠️
...Client/src/Microsoft/Data/SqlClient/SqlBulkCopy.cs	12.12%	29 Missing ⚠️
.../src/Microsoft/Data/SqlClient/SqlCommand.Reader.cs	22.22%	14 Missing ⚠️
.../Microsoft/Data/SqlClient/SqlCommand.Encryption.cs	0.00%	10 Missing ⚠️
...c/Microsoft/Data/SqlClient/TdsParserStateObject.cs	44.44%	5 Missing ⚠️
...qlClient/src/Microsoft/Data/SqlClient/TdsParser.cs	50.00%	4 Missing ⚠️
...ent/src/Microsoft/Data/SqlClient/SqlCommand.Xml.cs	25.00%	3 Missing ⚠️
...rc/Microsoft/Data/SqlClient/SqlCommand.NonQuery.cs	85.71%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4334      +/-   ##
==========================================
- Coverage   66.69%   64.74%   -1.95%     
==========================================
  Files         284      280       -4     
  Lines       43238    66300   +23062     
==========================================
+ Hits        28836    42927   +14091     
- Misses      14402    23373    +8971

Flag	Coverage Δ
CI-SqlClient	`?`
PR-SqlClient-Project	`64.74% <78.30%> (?)`

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

☔ View full report in Codecov by Harness.
📢 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.

Wraith2 · 2026-06-04T23:54:34Z

-                            onCancellation: static state => ((StrongBox<CancellationTokenRegistration>)state).Value.Dispose(),
-                            exceptionConverter: ex => SQL.BulkLoadInvalidDestinationTable(_destinationTableName, ex)
-                        );
+                            onFailure: (regReconnectCancel2, exception) =>


Can this be static or is it now capturing instance state? if it's capturing that's a slight memory performance regression.

Wraith2 · 2026-06-05T00:03:05Z

-                        completion: localCompletion,
+                        taskToContinue: writeTask,
+                        taskCompletionSource: localCompletion,
                        state: Tuple.Create(this, localCompletion),


Can this one use the two state parameter form used earlier to avoid the tuple creation again?

Wraith2 · 2026-06-05T00:07:55Z

@@ -1272,13 +1269,12 @@ internal Task ExecuteFlush()
                    else
                    {
                        return AsyncHelper.CreateContinuationTaskWithState(


Unless state is constained to class this pattern could be used with a struct and lead to a hard to find bug. If we don't want the constraint it might be worth an assertion of reference type.

Wraith2 · 2026-06-05T00:12:55Z

    -->
    <PackageReference Include="Microsoft.Extensions.TimeProvider.Testing" ExcludeAssets="build;buildTransitive" />
    <PackageReference Include="Microsoft.NET.Test.Sdk" />
+    <PackageReference Include="Moq" />


I remember that there was some controversy around Moq that caused a lot of people to not want to use it anymore. Is it worth considering other newer alternatives like https://github.com/guinhx/NimbleMock (MIT)

benrr101 added 6 commits June 2, 2026 13:10

Implement AsyncHelpers with generic state, add unit tests

e20fc9b

Change existing async helper calls in SqlBulkCopy.cs

ab424fa

Change existing async helper calls in TdsParser.cs

1608dfd

Change existing async helper calls in TdsParser.cs

80beeff

Change existing async helper calls in SqlCommand

734fa77

Copilot AI review requested due to automatic review settings June 3, 2026 03:52

benrr101 added the DO NOT MERGE PRs that are created for test reasons, should not be merged. label Jun 3, 2026

github-project-automation Bot added this to SqlClient Board Jun 3, 2026

github-project-automation Bot moved this to To triage in SqlClient Board Jun 3, 2026

Copilot started reviewing on behalf of benrr101 June 3, 2026 03:52 View session

Copilot AI reviewed Jun 3, 2026

View reviewed changes

benrr101 added 2 commits June 3, 2026 12:19

Remove old tests

c0127ec

Comments from copilot round 1

e516e3b

Copilot AI review requested due to automatic review settings June 4, 2026 17:57

Copilot started reviewing on behalf of benrr101 June 4, 2026 17:57 View session

Copilot AI reviewed Jun 4, 2026

View reviewed changes

Copilot stopped reviewing on behalf of benrr101 due to an error June 4, 2026 18:56
Copilot had to stop work due to a timeout.

benrr101 added Code Health 💊 Issues/PRs that are targeted to source code quality improvements. and removed DO NOT MERGE PRs that are created for test reasons, should not be merged. labels Jun 4, 2026

benrr101 marked this pull request as ready for review June 4, 2026 22:50

benrr101 requested a review from a team as a code owner June 4, 2026 22:50

benrr101 added this to the 7.1.0-preview2 milestone Jun 4, 2026

benrr101 mentioned this pull request Jun 4, 2026

Cleanup | AsyncHelpers Generic State #3705

Closed

benrr101 changed the title ~~[DRAFT] Async Generic Helpers~~ Async Generic Helpers Jun 4, 2026

Wraith2 reviewed Jun 4, 2026

View reviewed changes

Wraith2 reviewed Jun 5, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Async Generic Helpers#4334

Async Generic Helpers#4334
benrr101 wants to merge 8 commits into
mainfrom
dev/russellben/async-helpers-generic

benrr101 commented Jun 3, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

codecov Bot commented Jun 4, 2026 •

edited

Loading

Uh oh!

Wraith2 Jun 4, 2026

Uh oh!

Wraith2 Jun 5, 2026

Uh oh!

Wraith2 Jun 5, 2026

Uh oh!

Wraith2 Jun 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

benrr101 commented Jun 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing

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!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Wraith2 Jun 4, 2026

Choose a reason for hiding this comment

Uh oh!

Wraith2 Jun 5, 2026

Choose a reason for hiding this comment

Uh oh!

Wraith2 Jun 5, 2026

Choose a reason for hiding this comment

Uh oh!

Wraith2 Jun 5, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

benrr101 commented Jun 3, 2026 •

edited

Loading

codecov Bot commented Jun 4, 2026 •

edited

Loading