Skip to content

[cDAC] Adding TryGetContract on ContractRegistry to allow more custom behavior if contract is not found#127019

Merged
rcj1 merged 3 commits intodotnet:mainfrom
rcj1:contract-notimpl-optional
Apr 17, 2026
Merged

[cDAC] Adding TryGetContract on ContractRegistry to allow more custom behavior if contract is not found#127019
rcj1 merged 3 commits intodotnet:mainfrom
rcj1:contract-notimpl-optional

Conversation

@rcj1
Copy link
Copy Markdown
Contributor

@rcj1 rcj1 commented Apr 16, 2026

Currently, we access the contracts through GetContract, which throws NotImplementedException if the contract is not found. In the vast majority of cases, this fits neatly into the desired behavior - either we use this contract as part of an API that is supposed to return E_NOTIMPL when the feature is not enabled, or it is a catastrophic failure if the contract is not found, or both. However, a small subset of APIs have different behavior - we want to, say, return S_OK or branch and try another behavior. Currently, doing this requires our control flow to rely on these exceptions. This seeks to eliminate this pattern; in the rare case when a caller wants a different behavior, it can call TryGetContract and handle the true and false cases in a custom way.

More context: #126963 (comment)

@rcj1 rcj1 changed the title Adding TryGetContract on ContractRegistry to allow more custom behavior if contract is not found [cDAC] Adding TryGetContract on ContractRegistry to allow more custom behavior if contract is not found Apr 16, 2026
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag
See info in area-owners.md if you want to be subscribed.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a TryGetContract API to the cDAC ContractRegistry so callers can handle “contract not available” without relying on NotImplementedException control flow.

Changes:

  • Introduces ContractRegistry.TryGetContract<TContract>(out TContract, out string? failureReason) and routes existing property access through a throwing helper.
  • Updates CachingContractRegistry and the test TestContractRegistry to implement TryGetContract.
  • Updates SOSDacImpl.GetTieredVersions to use TryGetContract<IReJIT> to return S_OK when ReJIT isn’t supported.

Reviewed changes

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

File Description
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/ContractRegistry.cs Adds TryGetContract and changes how GetContract is exposed/used.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader/CachingContractRegistry.cs Implements TryGetContract for descriptor-backed resolution and caching.
src/native/managed/cdac/tests/TestPlaceholderTarget.cs Updates test contract registry to the new TryGetContract pattern.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs Switches GetTieredVersions feature-gating from exceptions to TryGetContract.

@rcj1 rcj1 force-pushed the contract-notimpl-optional branch from 6fcb482 to 1beed55 Compare April 16, 2026 19:59
@rcj1 rcj1 marked this pull request as ready for review April 16, 2026 19:59
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

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

…er.Abstractions/ContractRegistry.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 16, 2026 20:18
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

@rcj1 rcj1 merged commit 30125a7 into dotnet:main Apr 17, 2026
61 checks passed
@rcj1 rcj1 deleted the contract-notimpl-optional branch April 17, 2026 05:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants