Skip to content

AOT 3: Move authentication registration to an AOT-safe Abstractions registry - #4821

Open
paulmedynski wants to merge 10 commits into
mainfrom
dev/paul/aot/phase-1.3
Open

paulmedynski wants to merge 10 commits into
mainfrom
dev/paul/aot/phase-1.3

Conversation

@paulmedynski

@paulmedynski paulmedynski commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Description

Move authentication registration and bootstrap into Microsoft.Data.SqlClient.Extensions.Abstractions, enabling direct GetProvider/SetProvider calls without loading the driver through reflection. Explicit provider registration is available in trimmed and Native AOT applications.

New capabilities and API ownership

  • Add a shared Abstractions registry used directly by applications and driver authentication.
  • Move SqlAuthenticationInitializer into Abstractions with a type forward from SqlClient. Assembly-qualified reflection observes the new defining assembly.
  • Add an AOT-analyzed net10.0 Abstractions asset.
  • Add Switch.Microsoft.Data.SqlClient.EnableAzureExtensionDiscovery, a default-enabled, first-use-cached switch independent of authentication configuration loading.
  • Guard reflective authentication configuration and Azure discovery so trimmed/Native AOT .NET 10 publishing removes those paths.
  • Move authentication-specific Azure loading into Abstractions. Attestation uses an independent driver loader and does not trigger authentication bootstrap.
  • Preserve relaxed configuration parsing and apply configuration best-effort: trace recoverable initializer/provider failures, attempt remaining entries, and continue enabled Azure discovery. Retain valid option overrides and successful registrations.

Package and ownership relationships

Solid arrows show compile-time references or direct calls; dashed arrows show type forwarding or optional reflective loading.

flowchart TB
    App["Application / custom authentication provider"]
    subgraph Driver["Microsoft.Data.SqlClient"]
        Forwarders["Authentication type forwarders"]
        Connection["SqlConnectionInternal<br/>provider-based token acquisition"]
        Attestation["Independent Azure attestation loader"]
    end
    subgraph Abstractions["Microsoft.Data.SqlClient.Extensions.Abstractions"]
        API["SqlAuthenticationProvider<br/>GetProvider / SetProvider"]
        Types["Authentication contracts<br/>including SqlAuthenticationInitializer"]
        Registry["SqlAuthenticationProviderRegistry<br/>shared providers + registration tiers"]
        Config["SqlAuthenticationConfiguration<br/>load and validate settings"]
        Bootstrap["Guarded configuration activation<br/>and Azure discovery"]
        AuthLoader["Authentication Azure loader<br/>owned by Abstractions"]
    end
    subgraph Azure["Microsoft.Data.SqlClient.Extensions.Azure (optional)"]
        AzureProvider["ActiveDirectoryAuthenticationProvider"]
        Validator["Azure attestation validator"]
    end
    App -->|"GetProvider / SetProvider"| API
    App -->|"implements / uses"| Types
    Forwarders -.->|"preserve existing type references"| API
    Forwarders -.-> Types
    Connection -->|"GetProvider"| API
    API --> Registry
    Registry --> Bootstrap
    Bootstrap --> Config
    Bootstrap --> AuthLoader
    AuthLoader -.->|"discover and construct defaults"| AzureProvider
    Attestation -.->|"load for attestation only"| Validator
    AzureProvider -->|"implements contracts; references Abstractions"| Types
    Connection -->|"AcquireTokenAsync on selected provider"| Types
Loading

Provider lookup and connection failures

sequenceDiagram
    participant Caller as Application or SqlConnectionInternal
    participant API as SqlAuthenticationProvider (Abstractions)
    participant Registry as Shared registry
    participant Provider as Selected provider
    Caller->>API: GetProvider(method)
    API->>Registry: GetProvider(method)
    Note over Registry: Hold registry lock through bootstrap and lookup
    Registry->>Registry: EnsureBootstrapped()
    alt Unexpected recoverable bootstrap failure (including cached failure)
        Registry->>Registry: Trace failure without retrying bootstrap
    else Bootstrap completes, was already completed, or same-thread reentry
        Note over Registry: Continue with available registrations
    end
    Registry->>Registry: Look up provider by method
    Registry-->>API: Provider, or null if unregistered
    API-->>Caller: Provider or null
    opt Driver provider-based token acquisition
        Note over Caller: Missing provider with no AccessTokenCallback produces CannotFindAuthProvider
        Caller->>Provider: AcquireTokenAsync(parameters), if selected
        Provider-->>Caller: SqlAuthenticationToken
    end
Loading

GetProvider now shares the bootstrap/replacement lock. Slow callbacks can delay lookups and connection logins; synchronously waiting inside a callback for another thread that accesses the registry can deadlock.

Recoverable configuration reading and activation failures are traced during first-access bootstrap rather than preventing later configured providers or enabled Azure discovery. Successful registrations remain accessible and application registration can supply a missing provider. Configuration and optional discovery are attempted once; failed entries are not retried on later lookups. Unexpected recoverable bootstrap failures still use the cached-failure path.

If provider-based authentication has neither a provider nor an AccessTokenCallback, Open and OpenAsync report a SqlException with registration, package/discovery and tracing guidance instead of the missing-provider ArgumentException. Detailed configuration and discovery causes are reported through Abstractions EventSource traces rather than attached to the connection exception.

Registration precedence and callbacks

sequenceDiagram
    participant App as Application / configured initializer
    participant API as SqlAuthenticationProvider (Abstractions)
    participant Registry as Shared registry
    participant Old as Existing provider
    participant New as Incoming provider
    App->>API: SetProvider(method, provider)
    API->>Registry: SetProvider(method, provider)
    Registry->>Registry: EnsureBootstrapped()
    opt Unexpected recoverable bootstrap failure for application registration
        Registry->>Registry: Trace failure and continue without retrying bootstrap
    end
    Registry->>Registry: Reject null provider
    Note over Registry: Application tier normally, Config tier inside configured initializer
    Registry->>New: IsSupported(method)
    New-->>Registry: true required
    Note over Registry: After bootstrap, support validation is outside the lock
    critical Registry lock: precedence, callbacks, publication
        alt Existing registration has higher precedence
            Registry->>Registry: Trace refusal and leave registration unchanged
        else Registration allowed
            opt Replacing an existing provider
                Registry->>Old: BeforeUnload(method)
                Registry->>Registry: Recheck entry identity and retry if changed
                Registry->>New: BeforeLoad(method)
                Registry->>Registry: Recheck entry identity and retry if changed
            end
            Registry->>Registry: Publish provider and tier only after callbacks succeed
        end
    end
    Registry-->>API: true if registered, false if refused
    API-->>App: Registration result
Loading

Registrations made during a configured initializer's Initialize() callback now receive Config precedence, so application registrations cannot replace them. Explicit configuration entries run afterward and can replace initializer registrations. Constructor-time registrations do not receive this promotion.

Providers listed in app.config's <providers> collection now use the same replacement callbacks as SetProvider, including calls made by an initializer, instead of direct dictionary assignment. Replacing an existing provider for the same authentication method invokes its BeforeUnload and the replacement's BeforeLoad before publication. Registering for a method with no existing provider invokes neither callback.

Recoverable registration errors raised during Initialize() stop that initializer rather than being reduced to false. Configuration application traces the failure and continues with the explicit provider list; each failed provider entry is traced and skipped without blocking later entries or enabled Azure discovery. Successful registrations and callback side effects are not rolled back. Ordinary application SetProvider calls retain the existing public false result for null/unsupported providers and recoverable callback failures; the previous reflection wrapper already translated those invocation failures to false.

First-access bootstrap and trimming/AOT boundaries

flowchart TD
    Entry["EnsureBootstrapped<br/>under registry lock"] --> Failure{"Unexpected cached<br/>bootstrap failure?"}
    Failure -->|"Yes"| Rethrow["Rethrow cached failure internally<br/>public Get/Set trace and continue lookup/registration"]
    Failure -->|"No"| State{"Completed or reentrant<br/>bootstrap in progress?"}
    State -->|"Yes"| Return["Return to registry operation"]
    State -->|"No"| Start["Mark bootstrap in progress"]
    Start --> ConfigGate{"IsAppConfigSupported?"}
    ConfigGate -->|"Yes"| Load["Read configuration using relaxed parsing"]
    Load --> Initializer["Invoke configured initializer, if any<br/>reentrant SetProvider uses Config tier"]
    Load -.->|"ConfigurationErrorsException"| Empty["Trace and use empty settings<br/>do not fall back after a reading/schema error"]
    Empty --> Initializer
    Initializer --> Entries["Attempt each explicit configured provider in order<br/>Config tier; may replace initializer choices"]
    ConfigGate -->|"No"| AzureGate{"IsAzureExtensionDiscoverySupported?"}
    Entries --> AzureGate
    AzureGate -->|"Yes"| Discover["Discover Azure extension and apply option overrides<br/>register defaults only for unregistered methods"]
    AzureGate -->|"No"| Done["Mark bootstrap completed"]
    Discover --> Done
    Discover -.->|"Optional discovery failure"| DiscoveryError["Trace; do not poison bootstrap<br/>explicit registration remains available"]
    DiscoveryError --> Done
    Load -.->|"Other recoverable reading error"| ReadError["Trace; continue without configuration"]
    ReadError --> AzureGate
    Initializer -.->|"Recoverable error"| InitializerError["Trace; retain successful registrations"]
    InitializerError --> Entries
    Entries -.->|"Recoverable entry error"| EntryError["Trace and skip failed entry<br/>continue remaining configured entries"]
    EntryError --> Entries
    Done --> Return
Loading

The configuration loader retains its existing relaxed ConfigurationErrorsException policy. Other recoverable configuration failures are handled without suppressing later work. Fatal activation and discovery errors still propagate. Azure discovery remains independently switch-controlled and unavailable when its trimming/AOT capability guard removes the reflective path.

Public API and behavioral compatibility

This PR introduces behavioral compatibility breaks, but no intended source/binary API signature breaks.

Public API

  • No public methods or signatures are removed or changed.
  • SqlAuthenticationInitializer moves to Abstractions with a type forward. Existing compiled references remain supported, but reflection inspecting its defining assembly observes a change.

Potentially breaking behavior

Change Applications potentially affected Comments
Initializer registrations gain Config precedence App code can no longer replace providers registered during configured Initialize(). Corrects inconsistent precedence; explicit configuration entries were already protected. Documented as a behavior change.
Missing-provider login failures become SqlException, rather than ArgumentException Exception handlers depending on the previous type. Aligns Open/OpenAsync with their documented exception contract; still a behavior change.
Configured-provider replacements invoke callbacks Providers whose BeforeUnload/BeforeLoad callbacks throw or have side effects. Entries in <providers> now use the same replacement path as SetProvider. No callback is invoked when the authentication method has no existing registration.
Configuration failures become trace-based diagnostics, with best-effort continuation Apps relying on initialization exceptions or their inner exceptions to detect configuration failures. Failed configured entries may now leave a method available for Azure defaults. A failed initializer does not block the provider list; a failed provider entry does not block later entries. Enabled Azure discovery runs afterward using valid options and fills only unregistered methods. Successful configuration registrations keep precedence. No new public diagnostic API is introduced.
Azure discovery requires the options-based contract Older Azure extensions relying on parameterless/string-constructor discovery no longer load automatically. Upgrade to a compatible Extensions.Azure package with the options contract, first shipped in 7.0.2, and keep SqlClient family versions compatible. Public older constructors are not removed.
EnableAppConfig=false now applies to authentication on .NET Framework Apps setting that switch while still expecting authentication configuration to load. Not a problem - EnableAppConfig has never been released.
GetProvider now takes the registry lock Slow callbacks can stall logins; cross-thread synchronous waits involving the registry can deadlock. Documented in the package README; keep callbacks short and avoid synchronous cross-thread registry waits.
Wrapped fatal failures propagate Code previously relying on those failures being suppressed will see exceptions escape. Fatal exceptions should always escape our public API.

Trimmed/Native AOT applications must explicitly register providers because reflective configuration and automatic discovery are removed.

This is not a behavior-neutral relocation. The compatibility changes are documented in the package readmes.

Issues

Related follow-up: #4817 covers the executable current/legacy app.config compatibility harness, which is not included in this layer.

Testing

Pipeline runs for the latest PR head:

Copilot AI balanced review requested due to automatic review settings October 9, 2026 18:24
@paulmedynski
paulmedynski added this pull request to stack #4822 October 9, 2026 18:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Fatal constructor failures can be swallowed, trimming guidance is inaccurate, and the new switch lacks default-value coverage.

3 open findings
What changed in this PR

Relocates authentication registration and initialization into Abstractions, removing the driver reflection bridge while preserving compatibility through type forwarding.

Changes:

  • Adds an AOT-aware provider registry, feature switches, diagnostics, and localized resources.
  • Forwards SqlAuthenticationInitializer and routes driver lookups through the public registry.
  • Expands authentication tests and documentation.
File Description
src/​Microsoft.Data.SqlClient/​tests/​UnitTests/​TypeForwardTests.cs Tests initializer forwarding.
src/​Microsoft.Data.SqlClient/​tests/​UnitTests/​Microsoft/​Data/​SqlClient/​EnableAppConfigSwitchTest.cs Removes relocated configuration test.
src/​Microsoft.Data.SqlClient/​tests/​FunctionalTests/​AADAuthenticationTests.cs Updates registry documentation.
src/​Microsoft.Data.SqlClient/​tests/​AuthenticationTests/​ConfigurationTest/​Program.cs Verifies relocated initialization.
src/​Microsoft.Data.SqlClient/​src/​TypeForwards.Abstractions.cs Forwards initializer type.
src/​Microsoft.Data.SqlClient/​src/​Resources/​Strings.zh-Hant.resx Removes moved authentication strings.
src/​Microsoft.Data.SqlClient/​src/​Resources/​Strings.zh-Hans.resx Removes moved authentication strings.
src/​Microsoft.Data.SqlClient/​src/​Resources/​Strings.tr.resx Removes moved authentication strings.
src/​Microsoft.Data.SqlClient/​src/​Resources/​Strings.ru.resx Removes moved authentication strings.
src/​Microsoft.Data.SqlClient/​src/​Resources/​Strings.resx Removes moved authentication strings.
src/​Microsoft.Data.SqlClient/​src/​Resources/​Strings.pt-BR.resx Removes moved authentication strings.
src/​Microsoft.Data.SqlClient/​src/​Resources/​Strings.pl.resx Removes moved authentication strings.
src/​Microsoft.Data.SqlClient/​src/​Resources/​Strings.ko.resx Removes moved authentication strings.
src/​Microsoft.Data.SqlClient/​src/​Resources/​Strings.ja.resx Removes moved authentication strings.
src/​Microsoft.Data.SqlClient/​src/​Resources/​Strings.it.resx Removes moved authentication strings.
src/​Microsoft.Data.SqlClient/​src/​Resources/​Strings.fr.resx Removes moved authentication strings.
src/​Microsoft.Data.SqlClient/​src/​Resources/​Strings.es.resx Removes moved authentication strings.
src/​Microsoft.Data.SqlClient/​src/​Resources/​Strings.Designer.cs Removes moved resource accessors.
src/​Microsoft.Data.SqlClient/​src/​Resources/​Strings.de.resx Removes moved authentication strings.
src/​Microsoft.Data.SqlClient/​src/​Resources/​Strings.cs.resx Removes moved authentication strings.
src/​Microsoft.Data.SqlClient/​src/​PackageReadme.md Documents registry relocation and AOT behavior.
src/​Microsoft.Data.SqlClient/​src/​Microsoft/​Data/​SqlClient/​SqlUtil.cs Removes relocated exception factories.
src/​Microsoft.Data.SqlClient/​src/​Microsoft/​Data/​SqlClient/​SqlAuthenticationProviderManager.cs Removes driver-owned registry.
src/​Microsoft.Data.SqlClient/​src/​Microsoft/​Data/​SqlClient/​Connection/​SqlConnectionInternal.cs Uses the public provider registry.
src/​Microsoft.Data.SqlClient/​ref/​Microsoft.Data.SqlClient.cs Removes relocated initializer declaration.
src/​Microsoft.Data.SqlClient.Extensions/​Azure/​test/​WamBrokerTests.cs Updates registry references.
src/​Microsoft.Data.SqlClient.Extensions/​Azure/​test/​DefaultAuthProviderTests.cs Updates bootstrap test context.
src/​Microsoft.Data.SqlClient.Extensions/​Abstractions/​test/​SqlAuthenticationProviderTest.cs Tests direct registration without the driver.
src/​Microsoft.Data.SqlClient.Extensions/​Abstractions/​test/​ExceptionHelpersTest.cs Tests fatal-exception classification.
src/​Microsoft.Data.SqlClient.Extensions/​Abstractions/​test/​AzureProviderConstructionTest.cs Moves Azure construction tests.
src/​Microsoft.Data.SqlClient.Extensions/​Abstractions/​test/​AuthenticationStringsTest.cs Tests resilient message formatting.
src/​Microsoft.Data.SqlClient.Extensions/​Abstractions/​test/​AuthenticationRegistryTest.cs Tests registry callbacks and failures.
src/​Microsoft.Data.SqlClient.Extensions/​Abstractions/​test/​AuthenticationFeatureSwitchesTest.cs Tests switch caching.
src/​Microsoft.Data.SqlClient.Extensions/​Abstractions/​src/​TrimmingAttributes.cs Adds netstandard annotation polyfills.
src/​Microsoft.Data.SqlClient.Extensions/​Abstractions/​src/​SqlAuthenticationProviderRegistry.cs Implements registry and bootstrap.
src/​Microsoft.Data.SqlClient.Extensions/​Abstractions/​src/​SqlAuthenticationProvider.Internal.cs Removes the reflection bridge.
src/​Microsoft.Data.SqlClient.Extensions/​Abstractions/​src/​SqlAuthenticationProvider.cs Delegates directly to the registry.
src/​Microsoft.Data.SqlClient.Extensions/​Abstractions/​src/​SqlAuthenticationInitializer.cs Defines the relocated public type.
src/​Microsoft.Data.SqlClient.Extensions/​Abstractions/​src/​Resources/​Strings.zh-Hant.resx Adds localized authentication strings.
src/​Microsoft.Data.SqlClient.Extensions/​Abstractions/​src/​Resources/​Strings.zh-Hans.resx Adds localized authentication strings.
src/​Microsoft.Data.SqlClient.Extensions/​Abstractions/​src/​Resources/​Strings.tr.resx Adds localized authentication strings.
src/​Microsoft.Data.SqlClient.Extensions/​Abstractions/​src/​Resources/​Strings.ru.resx Adds localized authentication strings.
src/​Microsoft.Data.SqlClient.Extensions/​Abstractions/​src/​Resources/​Strings.resx Adds neutral authentication strings.
src/​Microsoft.Data.SqlClient.Extensions/​Abstractions/​src/​Resources/​Strings.pt-BR.resx Adds localized authentication strings.
src/​Microsoft.Data.SqlClient.Extensions/​Abstractions/​src/​Resources/​Strings.pl.resx Adds localized authentication strings.
src/​Microsoft.Data.SqlClient.Extensions/​Abstractions/​src/​Resources/​Strings.ko.resx Adds localized authentication strings.
src/​Microsoft.Data.SqlClient.Extensions/​Abstractions/​src/​Resources/​Strings.ja.resx Adds localized authentication strings.
src/​Microsoft.Data.SqlClient.Extensions/​Abstractions/​src/​Resources/​Strings.it.resx Adds localized authentication strings.
src/​Microsoft.Data.SqlClient.Extensions/​Abstractions/​src/​Resources/​Strings.fr.resx Adds localized authentication strings.
src/​Microsoft.Data.SqlClient.Extensions/​Abstractions/​src/​Resources/​Strings.es.resx Adds localized authentication strings.
src/​Microsoft.Data.SqlClient.Extensions/​Abstractions/​src/​Resources/​Strings.de.resx Adds localized authentication strings.
src/​Microsoft.Data.SqlClient.Extensions/​Abstractions/​src/​Resources/​Strings.cs.resx Adds localized authentication strings.
src/​Microsoft.Data.SqlClient.Extensions/​Abstractions/​src/​PackageReadme.md Documents registration and trimming.
src/​Microsoft.Data.SqlClient.Extensions/​Abstractions/​src/​ExceptionHelpers.cs Classifies recoverable exceptions.
src/​Microsoft.Data.SqlClient.Extensions/​Abstractions/​src/​AuthenticationStrings.cs Implements localized error formatting.
src/​Microsoft.Data.SqlClient.Extensions/​Abstractions/​src/​AuthenticationFeatureSwitches.cs Adds cached authentication switches.
src/​Microsoft.Data.SqlClient.Extensions/​Abstractions/​src/​Abstractions.csproj Adds net10 and AOT analysis.
src/​Microsoft.Data.SqlClient.Extensions/​Abstractions/​doc/​SqlAuthenticationProvider.xml Documents new registry behavior.
BUILDGUIDE.md Documents targets and tests.
.github/​instructions/​features.instructions.md Documents authentication switches.
.github/​instructions/​architecture.instructions.md Records the new ownership model.
Files not reviewed (1)
  • src/Microsoft.Data.SqlClient/src/Resources/Strings.Designer.cs: Generated file

🧠 Review effort: Balanced


💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@codecov

codecov Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.12%. Comparing base (c76d081) to head (af66afd).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4821      +/-   ##
==========================================
- Coverage   67.68%   67.12%   -0.57%     
==========================================
  Files         311      317       +6     
  Lines       45249    70236   +24987     
==========================================
+ Hits        30629    47148   +16519     
- Misses      14620    23088    +8468     
Flag Coverage Δ
CI-SqlClient 67.79% <100.00%> (+0.10%) ⬆️
PR-SqlClient-Project 66.18% <100.00%> (?)

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.

@paulmedynski paulmedynski added the Area\Azure Connectivity Use this to tag issues that are related to Azure connectivity. label Oct 13, 2026
@paulmedynski paulmedynski added this to the 8.0.0-preview1 milestone Oct 13, 2026
@paulmedynski paulmedynski changed the title Move authentication registration to an AOT-safe Abstractions registry AOT 3: Move authentication registration to an AOT-safe Abstractions registry Oct 13, 2026
Copilot AI balanced review requested due to automatic review settings October 13, 2026 17:41
@paulmedynski
paulmedynski removed this pull request from stack #4822 October 13, 2026 17:44
@paulmedynski
paulmedynski added this pull request to stack #4839 October 13, 2026 17:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The AOT guards do not automatically disable reflection, and wrapped fatal bootstrap errors can be suppressed.

4 open findings
Files not reviewed (1)
  • src/Microsoft.Data.SqlClient/src/Resources/Strings.Designer.cs: Generated file

🧠 Review effort: Balanced

@paulmedynski paulmedynski changed the title AOT 3: Move authentication registration to an AOT-safe Abstractions registry AOT 4: Move authentication registration to an AOT-safe Abstractions registry Oct 13, 2026
Copilot AI balanced review requested due to automatic review settings October 14, 2026 10:49
@paulmedynski
paulmedynski force-pushed the dev/paul/aot/phase-1.3 branch from e3b1eb7 to 92c8049 Compare October 14, 2026 10:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The package documentation incorrectly claims trimming and NativeAOT automatically disable reflective bootstrap paths.

4 open findings
Files not reviewed (1)
  • src/Microsoft.Data.SqlClient/src/Resources/Strings.Designer.cs: Generated file

🧠 Review effort: Balanced

@cheenamalhotra cheenamalhotra left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Registry design and locking look fine. Main concern is how failures reach customers.

GetProvider now returns null in several new cases: cached bootstrap failure, Azure extension without the options type, discovery switch off, or trimmed publish. In all of them the connection throws "Install the 'Microsoft.Data.SqlClient.Extensions.Azure' NuGet package", usually when it's already installed. The real cause is only in EventSource traces. On main the cause surfaced inside the exception.

Requested changes:

  1. Surface the cause in the connection error (inner exception + a matching message).
  2. Don't let a config failure block application-level SetProvider, or still run Azure discovery after it. Today there's no code workaround short of a restart.
  3. Give a clear error for an incompatible (1.0.x) Azure extension instead of a trace only.

Please also call out in release notes: EnableAppConfig=false now affects auth config on .NET Framework; initializer-registered providers can't be overridden by app code; GetProvider now takes a lock, so slow BeforeLoad/BeforeUnload callbacks stall logins.

Missing tests: connection-level error after a cached bootstrap failure; Azure assembly without the options type; SetProvider after a cached config failure; net462 driver test for EnableAppConfig=false. Also SqlAuthenticationProviderTest should join the non-parallel AuthenticationFeatureSwitchTestCollection, since other tests mutate the shared Instance.

SqlAuthenticationProvider? authProvider = SqlAuthenticationProvider.GetProvider(ConnectionOptions.Authentication);
if (authProvider == null && _accessTokenCallback == null)
{
throw SQL.CannotFindAuthProvider(ConnectionOptions.Authentication);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This now runs after config failures, version/extension mismatches, disabled discovery and trimming, and always says "install the Azure package". Please pass the registry's cached failure (or a reason) here and attach it as InnerException with a message matching the cause.

@paulmedynski paulmedynski Oct 23, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partially addressed in 4f2a2d2f7. The connection error no longer assumes the Azure package is absent: it points to explicit SetProvider registration, a compatible Azure package with discovery enabled, trimmed/AOT restrictions, and Abstractions EventSource traces.

There is no API on Abstractions to ask for the cached bootstrap exception, so I don't see a way to get that info into SqlConnectionInternal as an InnerException. Unless we want to add a public API just for this?

// Cache partial-bootstrap failures without losing their original stack.
SqlClientEventSource.Log.TryTraceEvent(
"Authentication bootstrap failed: {0}", e);
_bootstrapFailure = ExceptionDispatchInfo.Capture(e);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once cached, every later SetProvider also returns false (it calls EnsureBootstrapped first), so the app can't register a provider in code as a workaround. Azure discovery is skipped too. Suggest allowing application-tier registration after a config failure, or running discovery regardless.

@paulmedynski paulmedynski Oct 23, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 4f2a2d2f7 by allowing application registration after a recoverable bootstrap failure. GetProvider still returns successfully registered providers after that failure, so the registration is usable rather than merely returning true. The original failure remains cached and traced; the failed initializer is not retried, successful configuration registrations retain precedence, and fatal failures still propagate.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extended the recovery behavior in 5d9001ea2. Recoverable initializer failures are traced and the configured provider list is still attempted; a failed provider entry is traced and skipped without blocking later entries. Enabled Azure discovery then runs using the valid option overrides and fills only unregistered methods. Successful configuration registrations retain precedence, and application registration remains available. Fatal activation errors still propagate. The configuration, registry, driver and real-Azure test selections passed on .NET 10 (90 cases total).

// Require the modern options contract rather than probing older constructors or types.
const string optionsTypeName =
"Microsoft.Data.SqlClient.ActiveDirectoryAuthenticationProviderOptions";
Type optionsType = assembly.GetType(optionsTypeName, throwOnError: true)!;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With Extensions.Azure 1.0.x this throws and is caught as a trace only, so the default provider silently disappears. NuGet doesn't block that combination. Please give users a clear "upgrade Extensions.Azure to >= X" error (e.g. via the connection error above).

@paulmedynski paulmedynski Oct 23, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added explicit upgrade guidance in 4f2a2d2f7. When the installed assembly lacks ActiveDirectoryAuthenticationProviderOptions, Abstractions traces the missing contract and tells users to upgrade Extensions.Azure to version 1.1.0 or later with the options-based contract and keep SqlClient family package versions compatible. Discovery failures now trace the full exception rather than just its message.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected the upgrade guidance in 5d9001ea2: the required options contract first shipped in Extensions.Azure 7.0.2, not 1.1.0. The discovery trace, package README, useWamBroker resource message and regression assertion now consistently name 7.0.2. The legacy-contract diagnostic regression test passes.

benrr101
benrr101 previously approved these changes Oct 21, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Authentication bootstrap, callback reentrancy, and cross-framework compatibility require final human validation, with the loader exception-handling issue still unresolved.

1 open issue

Review effort: Balanced

Comment thread src/Microsoft.Data.SqlClient.Extensions/Abstractions/src/AzureExtensionLoader.cs Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The authentication ownership and compatibility changes warrant human review of signed-package, .NET Framework, and trimmed/Native AOT validation.

2 open issues

Review effort: Balanced

benrr101
benrr101 previously approved these changes Oct 23, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The new TestCommon Abstractions dependency conflicts with the released-package performance baseline’s explicit version requirement.

2 open issues

Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The acknowledged production-loader follow-up and incomplete latest-head publishing, .NET Framework runtime, and signed-build validation require human review.

2 open issues

Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Cross-assembly authentication and callback changes need human sign-off with current-head Windows runtime, signed-build, and trimmed/NativeAOT validation.

No open issues 🎉

2 resolved since last review

Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new loader consistency test uses an ambiguous reflection lookup and fails before comparing the loaders.

1 open issue

Review effort: Balanced

Comment thread src/Microsoft.Data.SqlClient.Extensions/Azure/test/DefaultAuthProviderTests.cs Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Authentication compatibility and synchronization changes require maintainer review, especially because the registry implementation diff is absent from the supplied changes.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

benrr101
benrr101 previously approved these changes Oct 26, 2026

@cheenamalhotra cheenamalhotra left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed at d425a1e. Thanks for the rebase, the SqlException change and the compatibility table.

Still open:

  • A config failure still skips Azure discovery (inline).
  • The version text says 1.1.0 in 3 places; it should be 7.0.2 (inline).
  • The SetProvider throw → false change isn't documented (inline).
  • Minor: EnableAppConfig now applies on .NET Framework, but features.instructions.md and the switch docs still say it has no effect there. Please update them.

OK as design decisions: dropping Azure extensions without the options type, and no diagnostics API.

// Load configuration first so Azure discovery receives its option overrides.
if (AuthenticationFeatureSwitches.IsAppConfigSupported)
{
ApplyConfiguration(SqlAuthenticationConfiguration.Load(getSection));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Config apply and Azure discovery are still in one try. One bad initializer or provider entry skips discovery, so every Entra ID method fails with the generic "cannot find provider" error. On main this failed loudly with the real cause.

Please use a separate try for each step so discovery always runs. Also add a test for config failure + discovery. If skipping discovery is intended, add it to the readme compatibility table.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 5d9001ea2. Configuration failures no longer prevent enabled Azure discovery. Recoverable initializer failures are traced and the provider list is still attempted; each recoverable provider failure is traced and skipped so later entries can succeed. Valid Azure options and successful configuration registrations are retained, and discovered defaults cannot replace existing registrations. Fatal activation errors still propagate. Added coverage for configuration-read, initializer and provider failures with discovery enabled/disabled, plus an integration test using the real Azure package. The focused .NET 10 selections passed (90 cases total); the Abstractions and Azure test projects also cross-build for net462 without warnings. Windows runtime validation remains for CI. The README compatibility table now documents this best-effort policy.

Type optionsType = assembly.GetType(optionsTypeName)
?? throw new TypeLoadException(
$"Azure authentication discovery requires '{optionsTypeName}'. Upgrade the " +
"'Microsoft.Data.SqlClient.Extensions.Azure' package to version 1.1.0 or " +

@cheenamalhotra cheenamalhotra Oct 26, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The options type first shipped in Extensions.Azure 7.0.2 (since we version-aligned in that milestone), so "1.1.0" is wrong. Same fix needed in Abstractions PackageReadme.md and Strings.resx (useWamBroker message).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 5d9001ea2. The discovery trace, PackageReadme.md and useWamBroker resource message now require 7.0.2 rather than 1.1.0. Updated the legacy-contract regression assertion to expect 7.0.2; it passes.

Automatic Azure discovery requires the `ActiveDirectoryAuthenticationProviderOptions` constructor
from a compatible `Microsoft.Data.SqlClient.Extensions.Azure` package, even when no overrides are
configured. Discovery does not fall back to parameterless or string-based provider constructors.
An extension missing the options contract is traced with upgrade guidance: use version 1.1.0 or

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be 7.0.2, not 1.1.0.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected the README guidance to 7.0.2 in 5d9001ea2, together with the discovery trace, resource message and regression assertion. The version-guidance regression test passes.

<value>Cannot find an authentication provider for '{0}'. Register a provider with SqlAuthenticationProvider.SetProvider, or use a compatible 'Microsoft.Data.SqlClient.Extensions.Azure' NuGet package with automatic discovery enabled. Discovery is unavailable in trimmed and NativeAOT applications. Check Microsoft.Data.SqlClient.EventSource traces (Informational level, Trace keyword 0x2) for configuration or Azure extension loading failures.</value>
</data>
<data name="SQL_UseWamBrokerRequiresAzureExtensionUpgrade" xml:space="preserve">
<value>Setting 'useWamBroker' requires the 'Microsoft.Data.SqlClient.Extensions.Azure' package to expose 'Microsoft.Data.SqlClient.ActiveDirectoryAuthenticationProviderOptions'. Upgrade the 'Microsoft.Data.SqlClient.Extensions.Azure' package to at least v1.1.0 that includes this type.</value>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be 7.0.2, not v1.1.0.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected this resource message to v7.0.2 in 5d9001ea2. The discovery trace and README now name the same minimum version. Resource XML validation and the updated version-guidance regression test pass.

"non-null provider that supports this method and fix any throwing IsSupported, " +
"BeforeLoad or BeforeUnload callbacks. Error: {1}",
authenticationMethod, e);
return false;

@cheenamalhotra cheenamalhotra Oct 26, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On main, SetProvider threw for a null provider, for an unsupported method, and when a callback threw. It now returns false. This is the one behavior change missing from the readme compatibility table. Please keep throwing for argument errors (null or unsupported) as before.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked this against the current PR base, 185a7ba. The internal manager threw for these cases, but the public API invoked it through the Abstractions reflection wrapper, which caught TargetInvocationException and returned false. Ordinary application calls therefore already returned false for null/unsupported providers and recoverable callback failures. The direct registry preserves that public result, so I have not changed it to throw or added a compatibility-table row claiming a new throw-to-false behavior. The README describes the false-return behavior, and the null, unsupported-provider and callback-failure regression cases pass in 5d9001ea2.

{
try
{
Instance.EnsureBootstrapped();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: after a bootstrap failure, every GetProvider call (once per physical connection open) rethrows the cached exception and traces it. Checking _bootstrapFailure directly avoids that cost.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The configuration-failure case is now handled without repeated throws in 5d9001ea2: recoverable reading/activation failures are traced when handled, later configuration and enabled Azure discovery are attempted, and bootstrap completes. Subsequent lookups do not rethrow those configuration failures or retry failed entries. Tests verify that later accesses do not rerun configuration or discovery. The separate unexpected cached-bootstrap-failure path and lookup synchronization remain unchanged; this is not a general cached-exception optimization.

paulmedynski and others added 8 commits October 27, 2026 15:52
Move provider registration, lookup and initialization into Extensions.Abstractions while preserving the driver's public API through type forwarding. Keep configuration and initializer precedence, replacement callbacks, and diagnostic failure handling.

Guard reflective configuration and optional Azure discovery for trimming and Native AOT, and cover registry behavior, bootstrap, discovery, feature switches and type forwarding.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Allow explicit provider registration and lookup after recoverable bootstrap
failure while preserving configuration precedence. Improve Azure contract
upgrade traces and missing-provider guidance without adding a diagnostics API.

Add recovery, connection-error, switch and test-isolation coverage, and
document compatibility considerations in package readmes and API guidance.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Preserve connection metadata and trace-only bootstrap diagnostics, cover Open and OpenAsync with simulated-server regressions, and document authentication compatibility changes in package readmes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove obsolete string-schema failure cases now covered by the parent's relaxed parsing tests. Retain provider-collection validation coverage.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove obsolete failure expectations superseded by the parent's provider-schema recovery coverage.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Filter optional authentication assembly binding failures through the existing nested-fatal recoverability check. Exercise production Assembly.Load resolution for recoverable and fatal FileNotFoundException, FileLoadException, and BadImageFormatException failures.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Signed Abstractions test invocations default to a reference mode without the required friend-assembly grant, preventing compilation.

1 open issue

Review effort: Balanced

Comment thread build.proj

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Authentication bootstrap, callback synchronization, and cross-runtime compatibility changes warrant final human validation before approval.

1 open issue

Review effort: Balanced

Trace and skip recoverable initializer and provider failures while continuing
later configuration entries and enabled Azure discovery. Preserve valid
options and configuration precedence, and retain fatal activation errors.

Correct Azure options-contract upgrade guidance to 7.0.2, document the
compatibility behavior, and add unit and real-Azure discovery coverage.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Authentication ownership, callback synchronization, and behavioral compatibility changes warrant final human validation beyond the targeted documentation checks.

3 open issues

Review effort: Balanced

@github-actions

Copy link
Copy Markdown

@paulmedynski This pull request has been marked as Author attention needed.

When you have addressed the reviewer feedback and are ready for another review, please post a comment with /ready to remove the label and re-engage reviewers.

Forward Abstractions test reference mode and version arguments, prepare
Logging in Package mode, and cover orchestration with regression tests.
Clarify diagnostic timing and recovery behavior in authentication XML docs.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Human assessment is needed because a missing authentication provider under legacy failover settings can trigger retries that exhaust the connection timeout or continue indefinitely when the timeout is disabled.

No open issues 🎉

3 resolved since last review

Review effort: Balanced

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area\Azure Connectivity Use this to tag issues that are related to Azure connectivity. Breaking Change 🔨 Issues/PRs that are related with breaking API changes in the driver.

Projects

Status: In review

Development

Successfully merging this pull request may close these issues.

6 participants