Merge branch 'main' into tests/flaky-tests

mdaigle · web-flow · commit b4583508bd19 · 2026-05-22T16:06:30.000-07:00
diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md
@@ -50,6 +50,7 @@ This project includes several key products and libraries that facilitate SQL Ser
 - **Data Encryption**: Supports data encryption for secure data transmission.
 - **Logging and Diagnostics**: Provides event source tracing diagnostic capabilities for troubleshooting.
 - **Failover Support**: Handles automatic failover scenarios for high availability.
+  - Compatibility switch: `Switch.Microsoft.Data.SqlClient.UseLegacyFailoverAlternationOnLoginSqlErrors` (default `false`) can restore legacy alternation behavior in `LoginWithFailover` for login-phase SQL errors.
 - **Cross-Platform Support**: Compatible with both .NET Framework and .NET Core, allowing applications to run on Windows, Linux, and macOS.
 - **Column Encryption AKV Provider**: Supports Azure Key Vault (AKV) provider for acquiring keys from Azure Key Vault to be used for encryption and decryption.
 
diff --git a/.github/instructions/features.instructions.md b/.github/instructions/features.instructions.md
@@ -246,6 +246,7 @@ AppContext switches allow runtime behavior changes without modifying connection
 | `Switch.Microsoft.Data.SqlClient.EnableMultiSubnetFailoverByDefault` | `false` | Sets `MultiSubnetFailover=true` as the default for all connections |
 | `Switch.Microsoft.Data.SqlClient.EnableUserAgent` | varies | Controls sending user agent information to SQL Server |
 | `Switch.Microsoft.Data.SqlClient.IgnoreServerProvidedFailoverPartner` | `false` | Ignores failover partner information sent by the server |
+| `Switch.Microsoft.Data.SqlClient.UseLegacyFailoverAlternationOnLoginSqlErrors` | `false` | Restores legacy `LoginWithFailover` alternation for login-phase SQL errors when parser state is not `Closed` |
 | `Switch.Microsoft.Data.SqlClient.LegacyRowVersionNullBehavior` | `false` | Restores legacy null handling for rowversion columns |
 | `Switch.Microsoft.Data.SqlClient.LegacyVarTimeZeroScaleBehaviour` | `false` | Restores legacy zero-scale behavior for time/datetime2/datetimeoffset |
 | `Switch.Microsoft.Data.SqlClient.MakeReadAsyncBlocking` | `false` | Makes ReadAsync behave synchronously (legacy compat) |
diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml
@@ -72,6 +72,10 @@ jobs:
       uses: actions/setup-dotnet@v5.2.0
       with:
         global-json-file: global.json
+      
+    - name: Restore dotnet tools
+      shell: bash
+      run: dotnet tool restore
 
     # Initializes the CodeQL tools for scanning.
     - name: Initialize CodeQL
diff --git a/.vscode/mcp.json b/.vscode/mcp.json
diff --git a/AGENTS.md b/AGENTS.md
@@ -79,10 +79,11 @@ Do **not** create branches directly under `main`, `dev/`, or any other top-level
 ### Bug Fix Workflow
 1. Understand the issue from the bug report
 2. Locate relevant code in `src/Microsoft.Data.SqlClient/src/` (do NOT modify legacy `netcore/src/` or `netfx/src/`)
-3. Write a failing test that reproduces the issue
-4. Implement the fix
-5. Ensure all tests pass
-6. Update documentation if behavior changes
+3. Check `.github/instructions/features.instructions.md` for existing AppContext switches (including failover compatibility switches) before introducing behavior changes
+4. Write a failing test that reproduces the issue
+5. Implement the fix
+6. Ensure all tests pass
+7. Update documentation if behavior changes
 
 ### Feature Implementation
 1. Review the feature specification
diff --git a/BUILDGUIDE.md b/BUILDGUIDE.md
@@ -19,15 +19,22 @@ on operating systems that do not support .NET Framework. As such, it is not nece
 
 ### Miscellaneous
 
-**PowerShell** is required to run several miscellaneous tasks as part of building and packaging. On
-Windows systems, either the built-in `powershell.exe` will be used, or if installed, the modern
-`pwsh` will be used. On Linux and macOS systems, the `pwsh` command is required to be in the `$PATH`
-environment variable. For specific instructions see: [Install
-PowerShell](https://learn.microsoft.com/en-us/powershell/scripting/install/install-powershell)
+**PowerShell** is included as a .NET local tool in this repository. Running `dotnet tool restore`
+(see below) will make it available via `dotnet tool run pwsh -- <args>`. Note that `pwsh` is not
+added to PATH — it must be invoked through `dotnet tool run`. Build targets handle this
+automatically; manual invocation is only needed for ad-hoc scripting.
 
 The **NuGet** binary is optional for inspection and feed-management workflows, but build and packaging flows in this
 repository are run through `dotnet build` against `build.proj`.
 
+### .NET Tools
+
+This repository uses .NET local tools (e.g. PowerShell) that must be restored before building. Run the following from the repository root:
+
+```bash
+dotnet tool restore
+```
+
 ## Developer Workflow
 
 Once you've cloned the repository and made your changes to the codebase, it is time to build, test, and optionally
diff --git a/dotnet-tools.json b/dotnet-tools.json
@@ -15,6 +15,13 @@
         "apicompat"
       ],
       "rollForward": false
+    },
+    "powershell": {
+      "version": "7.6.0",
+      "commands": [
+        "pwsh"
+      ],
+      "rollForward": false
     }
   }
 }
diff --git a/eng/pipelines/common/templates/jobs/ci-build-nugets-job.yml b/eng/pipelines/common/templates/jobs/ci-build-nugets-job.yml
@@ -134,6 +134,9 @@ jobs:
   # Install the .NET SDK.
   - template: /eng/pipelines/steps/install-dotnet.yml@self
 
+  # Restore dotnet CLI tools (e.g. pwsh, apicompat) before building.
+  - template: /eng/pipelines/steps/restore-dotnet-tools.yml@self
+
   # When we're performing a Debug build, we still want to try _compiling_ the
   # code in Release mode to ensure downstream pipelines don't encounter
   # compilation errors.  We won't use the Release artifacts for anything else
diff --git a/eng/pipelines/common/templates/jobs/ci-run-tests-job.yml b/eng/pipelines/common/templates/jobs/ci-run-tests-job.yml
@@ -222,6 +222,9 @@ jobs:
       ${{ else }}:
         runtimes: [8.x, 9.x]
 
+  # Restore dotnet CLI tools (e.g. pwsh, apicompat) before building.
+  - template: /eng/pipelines/steps/restore-dotnet-tools.yml@self
+
   - ${{ if ne(parameters.prebuildSteps, '') }}:
     - ${{ parameters.prebuildSteps }} # extra steps to run before the build like downloading sni and the required configuration
 
diff --git a/eng/pipelines/steps/restore-dotnet-tools.yml b/eng/pipelines/steps/restore-dotnet-tools.yml
@@ -0,0 +1,14 @@
+#################################################################################
+# Licensed to the .NET Foundation under one or more agreements.                 #
+# The .NET Foundation licenses this file to you under the MIT license.          #
+# See the LICENSE file in the project root for more information.                #
+#################################################################################
+
+# Restores dotnet CLI tools defined in dotnet-tools.json.
+# This step should be invoked after install-dotnet.yml and before any build
+# steps that depend on the restored tools (e.g. pwsh, apicompat).
+
+steps:
+  - script: dotnet tool restore
+    displayName: Restore .NET Tools
+    workingDirectory: $(Build.SourcesDirectory)
diff --git a/src/Microsoft.Data.SqlClient/ref/Microsoft.Data.SqlClient.csproj b/src/Microsoft.Data.SqlClient/ref/Microsoft.Data.SqlClient.csproj
@@ -44,23 +44,35 @@
     <OutputPath>$(ArtifactPath)$(AssemblyName).ref/$(ReferenceType)-$(Configuration)/</OutputPath>
   </PropertyGroup>
 
+
   <!-- Trim Docs for IntelliSense ====================================== -->
   <!--
     It has been determined that including the remarks section in IntelliSense docs looks bad in
     Visual Studio. So, before we generate the ref binaries, we trim off the remarks tags. We leave
     them in the xml files, though, because those are the source for the docs site, which needs to
     have the full documentation.
   -->
+  <!-- Verifies that dotnet local tools have been restored (pwsh is required for doc trimming). -->
+  <Target Name="_CheckPwshToolRestored">
+    <Exec Command="dotnet tool run pwsh -- -Version"
+          WorkingDirectory="$(RepoRoot)"
+          ConsoleToMsBuild="true"
+          IgnoreExitCode="true">
+      <Output TaskParameter="ExitCode" PropertyName="_PwshExitCode" />
+    </Exec>
+    <Error Condition="'$(_PwshExitCode)' != '0'"
+           Text="The 'pwsh' dotnet local tool is not available. Run 'dotnet tool restore' from the repository root before building." />
+  </Target>
+
   <!-- This target runs after Build, and trims XML documentation generated in the $(OutputPath) of the project where this target is included.-->
   <Target Name="TrimDocsForIntelliSense"
           AfterTargets="Build"
+          DependsOnTargets="_CheckPwshToolRestored"
           Condition="'$(IsCrossTargetingBuild)' != 'true' AND '$(GenerateDocumentationFile)' == 'true'">
 
     <PropertyGroup>
-      <PowerShellCommand Condition="'$(OS)' == 'Windows_NT'">powershell.exe</PowerShellCommand>
-      <PowerShellCommand Condition="'$(OS)' != 'Windows_NT'">pwsh</PowerShellCommand>
       <PowerShellCommand>
-        $(PowerShellCommand)
+        dotnet tool run pwsh --
           -NonInteractive
           -ExecutionPolicy Unrestricted
           -Command "$(RepoRoot)tools\intellisense\TrimDocs.ps1 -inputFile '$(DocumentationFile)' -outputFile '$(DocumentationFile)'"
diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs
@@ -3635,7 +3635,17 @@ private void LoginWithFailover(
                         continue;
                     }
 
-                    if (IsDoNotRetryConnectError(sqlex) || timeout.IsExpired)
+                    bool isLoginPhaseSqlError = _parser?.State is not TdsParserState.Closed;
+
+                    // If state != closed, indicates that the parser encountered an error while
+                    // processing the login response (e.g. an explicit error token). Transient
+                    // network errors that impact connectivity will result in parser state being
+                    // closed. Only network-level errors should trigger failover alternation;
+                    // login-phase errors (like transient errors) should be thrown so they can
+                    // be handled by the outer ConnectRetryCount loop.
+                    if ((isLoginPhaseSqlError && !LocalAppContextSwitches.UseLegacyFailoverAlternationOnLoginSqlErrors) ||
+                        IsDoNotRetryConnectError(sqlex) ||
+                        timeout.IsExpired)
                     {
                         // No more time to try again.
                         // Caller will call LoginFailure()
diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs
@@ -58,6 +58,13 @@ internal static class LocalAppContextSwitches
     private const string IgnoreServerProvidedFailoverPartnerString =
         "Switch.Microsoft.Data.SqlClient.IgnoreServerProvidedFailoverPartner";
 
+    /// <summary>
+    /// The name of the app context switch that controls whether failover
+    /// alternation should use legacy behavior for login-phase SQL errors.
+    /// </summary>
+    private const string UseLegacyFailoverAlternationOnLoginSqlErrorsString =
+        "Switch.Microsoft.Data.SqlClient.UseLegacyFailoverAlternationOnLoginSqlErrors";
+
     /// <summary>
     /// The name of the app context switch that controls whether to preserve
     /// legacy behavior where Timestamp/RowVersion fields return empty byte
@@ -182,6 +189,11 @@ private enum SwitchValue : byte
     /// </summary>
     private static SwitchValue s_ignoreServerProvidedFailoverPartner = SwitchValue.None;
 
+    /// <summary>
+    /// The cached value of the UseLegacyFailoverAlternationOnLoginSqlErrors switch.
+    /// </summary>
+    private static SwitchValue s_useLegacyFailoverAlternationOnLoginSqlErrors = SwitchValue.None;
+
     /// <summary>
     /// The cached value of the LegacyRowVersionNullBehavior switch.
     /// </summary>
@@ -409,6 +421,19 @@ public static bool GlobalizationInvariantMode
             defaultValue: false,
             ref s_ignoreServerProvidedFailoverPartner);
 
+    /// <summary>
+    /// When set to true, LoginWithFailover preserves legacy behavior and may
+    /// alternate to the failover partner on login-phase SQL errors where the
+    /// parser state is not Closed.
+    ///
+    /// The default value of this switch is false.
+    /// </summary>
+    public static bool UseLegacyFailoverAlternationOnLoginSqlErrors =>
+        AcquireAndReturn(
+            UseLegacyFailoverAlternationOnLoginSqlErrorsString,
+            defaultValue: false,
+            ref s_useLegacyFailoverAlternationOnLoginSqlErrors);
+
     /// <summary>
     /// In System.Data.SqlClient and Microsoft.Data.SqlClient prior to 3.0.0 a
     /// field with type Timestamp/RowVersion would return an empty byte array.
diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlDependencyUtils.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlDependencyUtils.cs
@@ -89,7 +89,7 @@ private SqlDependencyPerAppDomainDispatcher()
 #if NETFRAMEWORK
                 _timeoutTimer = new Timer(new TimerCallback(TimeoutTimerCallback), null, Timeout.Infinite, Timeout.Infinite);
 
-                // If rude abort - we'll leak.  This is acceptable for now.  
+                // If rude abort - we'll leak.  This is acceptable for now.
                 AppDomain.CurrentDomain.DomainUnload += new EventHandler(UnloadEventHandler);
 #else
                 _timeoutTimer = ADP.UnsafeCreateTimer(
@@ -131,7 +131,7 @@ private void UnloadEventHandler(object sender, EventArgs e)
             }
         }
 
-        //  When remoted across appdomains, MarshalByRefObject links by default time out if there is no activity 
+        //  When remoted across appdomains, MarshalByRefObject links by default time out if there is no activity
         //  within a few minutes.  Add this override to prevent marshaled links from timing out.
 #if NET
         [Obsolete("InitializeLifetimeService() is not supported after .Net5.0 and throws PlatformNotSupportedException.")]
@@ -216,7 +216,7 @@ internal string AddCommandEntry(string commandHash, SqlDependency dep)
                             dependencyList.Add(dep);
 
                             // map command hash to notification we just created to reuse it for the next client
-                            _commandHashToNotificationId.Add(commandHash, notificationId);
+                            _commandHashToNotificationId.Add(commandHash, notificationId); // CodeQL [SM04207] This value is an opaque query-notification correlation identifier, not a secret or security token. It is used only for uniqueness and exact dictionary lookup after SQL Server round-trips the user data. Guid.NewGuid provides sufficient collision resistance for the expected in-process notification cardinality, and changing the generator would not materially improve security.
                             _notificationIdToDependenciesHash.Add(notificationId, dependencyList);
                         }
 
diff --git a/src/Microsoft.Data.SqlClient/tests/Common/LocalAppContextSwitchesHelper.cs b/src/Microsoft.Data.SqlClient/tests/Common/LocalAppContextSwitchesHelper.cs
@@ -47,6 +47,7 @@ public sealed class LocalAppContextSwitchesHelper : IDisposable
     private readonly bool? _globalizationInvariantModeOriginal;
     #endif
     private readonly bool? _ignoreServerProvidedFailoverPartnerOriginal;
+    private readonly bool? _useLegacyFailoverAlternationOnLoginSqlErrorsOriginal;
     private readonly bool? _legacyRowVersionNullBehaviorOriginal;
     private readonly bool? _legacyVarTimeZeroScaleBehaviourOriginal;
     private readonly bool? _makeReadAsyncBlockingOriginal;
@@ -98,6 +99,8 @@ public LocalAppContextSwitchesHelper()
             #endif
             _ignoreServerProvidedFailoverPartnerOriginal =
                 GetSwitchValue("s_ignoreServerProvidedFailoverPartner");
+            _useLegacyFailoverAlternationOnLoginSqlErrorsOriginal =
+                GetSwitchValue("s_useLegacyFailoverAlternationOnLoginSqlErrors");
             _legacyRowVersionNullBehaviorOriginal =
                 GetSwitchValue("s_legacyRowVersionNullBehavior");
             _legacyVarTimeZeroScaleBehaviourOriginal =
@@ -154,6 +157,9 @@ public void Dispose()
             SetSwitchValue(
                 "s_ignoreServerProvidedFailoverPartner",
                 _ignoreServerProvidedFailoverPartnerOriginal);
+            SetSwitchValue(
+                "s_useLegacyFailoverAlternationOnLoginSqlErrors",
+                _useLegacyFailoverAlternationOnLoginSqlErrorsOriginal);
             SetSwitchValue(
                 "s_legacyRowVersionNullBehavior", 
                 _legacyRowVersionNullBehaviorOriginal);
@@ -242,6 +248,15 @@ public bool? IgnoreServerProvidedFailoverPartner
         set => SetSwitchValue("s_ignoreServerProvidedFailoverPartner", value);
     }
 
+    /// <summary>
+    /// Get or set the UseLegacyFailoverAlternationOnLoginSqlErrors switch value.
+    /// </summary>
+    public bool? UseLegacyFailoverAlternationOnLoginSqlErrors
+    {
+        get => GetSwitchValue("s_useLegacyFailoverAlternationOnLoginSqlErrors");
+        set => SetSwitchValue("s_useLegacyFailoverAlternationOnLoginSqlErrors", value);
+    }
+
     /// <summary>
     /// Get or set the LegacyRowVersionNullBehavior switch value.
     /// </summary>
diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/AdapterTest/AdapterTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/AdapterTest/AdapterTest.cs
@@ -107,6 +107,8 @@ public void SimpleFillTest()
 
         // TODO Synapse: Remove Northwind dependency by creating required tables in setup.
         [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureSynapse))]
+        // https://github.com/dotnet/SqlClient/issues/4135
+        [Trait("Category", "flaky")]
         public void FillShouldAllowRetryLogicProviderToBeInvoked()
         {
             int maxRetries = 3;
diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/LocalAppContextSwitchesTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/LocalAppContextSwitchesTest.cs
@@ -31,6 +31,7 @@ public void TestDefaultAppContextSwitchValues()
         Assert.False(appContextSwitchesHelper.UseConnectionPoolV2);
         Assert.False(appContextSwitchesHelper.TruncateScaledDecimal);
         Assert.False(appContextSwitchesHelper.IgnoreServerProvidedFailoverPartner);
+        Assert.False(LocalAppContextSwitches.UseLegacyFailoverAlternationOnLoginSqlErrors);
         Assert.False(appContextSwitchesHelper.EnableMultiSubnetFailoverByDefault);
         #if NET
         Assert.False(appContextSwitchesHelper.GlobalizationInvariantMode);
diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionFailoverTests.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionFailoverTests.cs
diff --git a/src/Microsoft.Data.SqlClient/tests/tools/TDS/TDS.Servers/TransientTdsErrorTdsServer.cs b/src/Microsoft.Data.SqlClient/tests/tools/TDS/TDS.Servers/TransientTdsErrorTdsServer.cs
diff --git a/src/Microsoft.Data.SqlClient/tests/tools/TDS/TDS.Servers/TransientTdsErrorTdsServerArguments.cs b/src/Microsoft.Data.SqlClient/tests/tools/TDS/TDS.Servers/TransientTdsErrorTdsServerArguments.cs
diff --git a/tools/targets/CompareMdsRefAssemblies.targets b/tools/targets/CompareMdsRefAssemblies.targets

Original file line number	Diff line number	Diff line change
`@@ -15,6 +15,13 @@`
`15`	`15`	`"apicompat"`
`16`	`16`	`],`
`17`	`17`	`"rollForward": false`
	`18`	`+ },`
	`19`	`+ "powershell": {`
	`20`	`+ "version": "7.6.0",`
	`21`	`+ "commands": [`
	`22`	`+ "pwsh"`
	`23`	`+ ],`
	`24`	`+ "rollForward": false`
`18`	`25`	`}`
`19`	`26`	`}`
`20`	`27`	`}`