[6.1] Align SqlException Numbers across platforms #3475
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Converted Threads to long-running Tasks The key advantage is that exceptions propagate properly. If a thread throws an exception (as a result of a failed test assertion, or otherwise) then the test host crashes and must be restarted. * Corrected the instantiation of the cancellation task - missing state parameter. * Changes to TestSqlCommandCancel, eliminating timing-specific cancellation behaviour testing. This should also allow the test to run on both netcore and netfx. * Responding to code review. * Removed two unnecessary iterations from DatabaseHelper. * Added explanatory comments to ApiShould. * Switched to using Task.WaitAll rather than waiting for each Task in sequence. * Improve cancellation detection Cancellation can trigger one of several different errors, resulting in a flakier test. Also ensure that the query always takes more than 150ms, ensuring that a quick query execution doesn't cause the test to fail. Finally, make sure that we try to read everything from the SqlDataReader. * Correcting previous merge
* - Align SqlException Numbers across platforms - Better capture error scenarios in TCP managed SNI. - Fix logging bug in SqlClientEventSource. - Change nativeError from uint to int * - Removed duplicate SniErrorDetails object and aligned field names. * - Added localized string for the new connection timed out exception. * - Fixed tests sensitive to OS newlines. --------- Co-authored-by: Paul Medynski <31868385+paulmedynski@users.noreply.github.com>
- Manually fixing some error reporting that wasn't cherr-picked because it was part of larger changes.
There was a problem hiding this comment.
Pull Request Overview
This PR aligns SQL error and SNI error handling across platforms, replaces direct thread usage in manual tests with long-running tasks, and updates resource strings and error code types.
- Switch manual test threading from
ThreadtoTask.Factory.StartNewfor long-running operations. - Introduce a reusable
SniErrorDetailsstruct and unify native error code types fromuinttoint. - Add a new resource string for connection timeouts and update event source method naming.
Reviewed Changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/RandomStressTest/RandomStressTest.cs | Replace Thread with Task for stress test threads |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/ParametersTest.cs | Replace thread list with Task[] and use Task.Factory.StartNew |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/MirroringTest/ConnectionOnMirroringTest.cs | Replace Thread with Task for mirroring connection test |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/ConnectivityTest.cs | Replace _thread with _task and start tasks in connection worker |
| src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/TestFixtures/DatabaseHelper.cs | Simplify IEnumerable<object[]> yields by removing redundant arguments |
| src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/ApiShould.cs | Rewrite cancel tests to use Tasks with ManualResetEventSlim |
| src/Microsoft.Data.SqlClient/src/Resources/Strings.resx | Add new resource <data name="SQL_ConnectTimeout"> |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs | Introduce new SniErrorDetails struct for managed SNI |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsEnums.cs | Change SNI_WAIT_TIMEOUT constant from uint to int |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlError.cs | Update SqlError constructors to accept int win32ErrorCode |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlClientEventSource.cs | Rename SNIScopeEnter event to ScopeEnter |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ManagedSni/SniTcpHandle.netcore.cs | Improve exception capture and logging in Connect |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ManagedSni/SniNpHandle.netcore.cs | Unify ReportErrorAndReleasePacket signature to use int nativeError |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ManagedSni/SniError.netcore.cs | Change nativeError field to int and map exception codes properly |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ManagedSni/SniCommon.netcore.cs | Update ReportSNIError overloads to use int nativeError |
| src/Microsoft.Data.SqlClient/src/Interop/Windows/Sni/SniError.cs | Change interop nativeError field from uint to int |
| src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs | Update SNI error code handling and constructor call to int nativeError |
| src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.netcore.cs | Remove old SNIErrorDetails struct and switch to new one |
| src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs | Refactor ProcessSNIError to use new struct properties |
| src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.Unix.cs | Implement GetSniErrorDetails to return TdsParserStateObject.SniErrorDetails |
Files not reviewed (1)
- src/Microsoft.Data.SqlClient/src/Resources/Strings.Designer.cs: Language not supported
Comments suppressed due to low confidence (4)
src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/ApiShould.cs:3405
- The XML doc
<param name="numberOfTimesToCancel">no longer matches any constructor parameter. Please remove or update this<param>entry to reflect the current signature.
/// <summary>
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs:63
- [nitpick] The field name
Exceptionshadows theSystem.Exceptiontype and can be confusing. Consider renaming it to something likeInnerExceptionorErrorExceptionfor clarity.
public readonly Exception Exception;
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsEnums.cs:607
- Changing this public constant from
uinttointis a breaking API change. Verify that all consumers depending onTdsEnums.SNI_WAIT_TIMEOUThave been updated or consider providing a compatibility shim.
public const int SNI_WAIT_TIMEOUT = 258; // The wait operation timed out.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlClientEventSource.cs:427
- The event method
SNIScopeEnterwas replaced byScopeEnter, which could affect existing event listeners or ETW consumers. Please confirm this change is reflected in any schema or downstream tooling.
return ScopeEnter(sb.ToString());
src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/ApiShould.cs
Show resolved
Hide resolved
…dsParserStateObject.
David-Engel
approved these changes
Jul 15, 2025
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release/6.1 #3475 +/- ##
===============================================
+ Coverage 66.91% 67.44% +0.53%
===============================================
Files 280 280
Lines 62386 62393 +7
===============================================
+ Hits 41745 42084 +339
+ Misses 20641 20309 -332
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This is a cherry-pick of 2 PRs and a touch-up commit:
Issues
#1773