Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adopt Nullable annotations in Polly.dll #2376

Open
Barsonax opened this issue Nov 12, 2024 · 2 comments
Open

Adopt Nullable annotations in Polly.dll #2376

Barsonax opened this issue Nov 12, 2024 · 2 comments

Comments

@Barsonax
Copy link

Barsonax commented Nov 12, 2024

Describe the bug

The xml docs imply that Result and Exception are nullable in https://github.com/App-vNext/Polly/blob/main/src/Polly/DelegateResult.cs but this is not clear from the typing.

Expected behavior

TResult should be TResult? and Exception should be Exception?

Actual behavior

TResult and Exception are non nullable

Steps to reproduce

No response

Exception(s) (if any)

We got a null ref on .Result since we thought it was non nullable

Polly version

8.4.2

.NET Version

8.0.403

Anything else?

No response

@Barsonax Barsonax added the bug label Nov 12, 2024
@peter-csala
Copy link
Contributor

The DelegateResult class is part of the V7 API which is currently under maintenance. Your proposed solution regarding nullability is used in its successor class inside the V8's Outcome.

@martincostello I think this would cause breaking change. What do you think?

@martincostello martincostello changed the title [Bug]: Inconsistent nullability in DelegateResult Adopt Nullable annotations in Polly.dll Nov 12, 2024
@martincostello
Copy link
Member

martincostello commented Nov 12, 2024

The Polly assembly has not opted into nullable annotations at all (the project file has no Nullable property, compared to Polly.Core which has).

This issue isn't a bug - Polly pre-dates the concept of nullable reference types. This issue is instead a feature request to add them to Polly.

As Polly is effectively now legacy and exists for backwards compatibility and minimal future evolution, it's unlikely we'd do the work to add nullable reference types to it due to the effort involved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants