Expected diamond-depndency conflict if multi-versions referenced and third-party component does not update - does not affect HttpClientFactory [ System.TypeLoadException when referencing both Polly 7 and transitive Polly 6 ]

[altso]

Assuming a solution with the following project/code structure:

PollySeven.MyHttpClient depends on Microsoft.Extensions.Http.Polly (which depends on Polly v6).
PollySeven.MyHttpClient uses RetryAsync() extension method to register MyService.
PollySeven depends on PollySeven.MyHttpClient and Polly v7.
PollySeven uses MyService.

PollySeven

public static void Main(string[] args)
{
    var services = new ServiceCollection();
    services.AddMyServices();

    ServiceProvider provider = services.BuildServiceProvider(true);

    var service = provider.GetRequiredService<MyService>();

    // System.TypeLoadException:
    // 'Could not load type 'Polly.RetrySyntaxAsync' from assembly
    // 'Polly, Version=7.0.0.0, Culture=neutral, PublicKeyToken=c8a3ffc3f8f825cc'.'
}

PollySeven.MyHttpClient

public static IServiceCollection AddMyServices(this IServiceCollection services)
{
    services.AddHttpClient<MyService>()
        .AddTransientHttpErrorPolicy(p => p.RetryAsync());
    return services;
}

When the application runs, I get System.TypeLoadException.

The root cause of this issue might be similar to #611.

Expected behavior:
Code executes successfully.

Actual behaviour:

System.TypeLoadException is thrown:

Could not load type 'Polly.RetrySyntaxAsync' from assembly
'Polly, Version=7.0.0.0, Culture=neutral, PublicKeyToken=c8a3ffc3f8f825cc'.

Steps / Code to reproduce the problem:

Example repo: https://github.com/altso/PollySeven

[reisenberger]

Hi @altso , thanks for the exemplary repo.

You can solve this by forcing the project indirectly referencing Polly 6.x (the project PollySeven.MyHttpClient in this case) to reference Polly 7.x instead. Add a top-level nuget dependency, within that project, to Polly v7.x. This forces the indirect dependencies also to build against Polly v7.x. The result should look like this:

The yellow-highlighted nuget reference to Polly v7.0.3 is the one you have to add. The indirect reference to Polly via Microsoft.Extensions.Http.Polly (marked red) then falls into line and also references Polly v7.0.3. The app then builds and starts successfully.

There is nothing in the Microsoft extensions to Polly which rely specifically on Polly v6 features, so this is a safe procedure.

Here is a zip showing the fix: Issue615fix.zip

---

For completeness for all coming to this issue in future

Polly v7.0 clarified the sync/async split internally in Polly policies, but no actual change in functionality on that.

The only other breaking change in Polly v7 is changes to cache provider interfaces. For cache providers which the Polly team maintains (Polly.Caching.Memory, Polly.Caching.Distributed and Polly.Caching.Serialization.Json), we have already released upgraded cache providers. Thus, the only cases in which following the above procedure - forcing a Polly v7.x on an external dependency which currently references v6 - could cause a problem, , would be if the third-party component was an implementation of a custom Polly cache provider for CachePolicy, where that third-party custom cache provider had not yet updated for the v7 changes.

[altso]

@reisenberger, thanks for the prompt reply. Adding an explicit package reference to Polly v7.0.3 in PollySeven.MyHttpClient project solves the issue. Hopefully, in my case I have an access to the source code and can make this change. However, do you have any suggestions for the scenario when PollySeven.MyHttpClient is a third party library?

[reisenberger]

Do you have any suggestions for the scenario when PollySeven.MyHttpClient is a third party library?

Hi @altso . I want to be sure I haven't misunderstood the question, but I think the case discussed does illustrate a third-party library. In the above example, Microsoft.Extensions.Http.Polly, which is what was referencing Polly >= v6.0.1, is a third-party library. In all cases, there must ultimately be some project-of-your-own which is referencing that third-party library, and in that project-of-your-own, you add a top-level reference to Polly v7.

Does that make sense, or is there a case you are thinking of which I am not seeing?

---

Sidenote: Where this would not work would be if a third-party library had restricted their library from inter-operating with versions of Polly higher than v6, eg with a dependency directive such as:

<PackageReference Include="Polly" Version="[6.0,7.0)" />

But it would be against common/recommended practice for a library to have done that, as per eg the Microsoft diamond dependencies page ("Avoid NuGet package references with a version upper limit"). And there would be no reason for a library referencing Polly to have done that - unless they are a cache provider release only compatible with v6.

[altso]

@reisenberger apologies for the confusion. I'll try to explain better.

Let's assume PollySeven.MyHttpClient is a third party library. It has only Microsoft.Extensions.Http.Polly package reference and also uses RetryAsync(). Since there is no package references to Polly v7, it compiles against Polly v6 and now has a runtime dependency on Polly.RetrySyntaxAsync.

PollySeven references PollySeven.MyHttpClient and Polly v7, so Polly v7 is deployed along with PollySeven binaries. As a result, application would fail at runtime as Polly.RetrySyntaxAsync is missing in v7.

Even though it's similar to how Microsoft.Extensions.Http.Polly is built, I believe there is a major difference between two that you have outlined in your first comment:

There is nothing in the Microsoft extensions to Polly which rely specifically on Polly v6 features, so this is a safe procedure.

In case of PollySeven.MyHttpClient it relies on Polly.RetrySyntaxAsync which is no longer available in v7.

Hope this clarifies my questions. I can create a complete example by publishing PollySeven.MyHttpClient to nuget if needed.

[reisenberger]

Hi @altso. Ok, let's try it. No need to publish the nuget package to public nuget: you can also drag the .nupkg file into this conversation; I can work with a local copy.

[altso]

@reisenberger I updated the example. Please see this commit altso/PollySeven@7a94985.

I used this command to make a package:

dotnet pack -c Release -o ../LocalPackageSource

You can download the binary here: https://github.com/altso/PollySeven/blob/master/LocalPackageSource/PollySeven.MyHttpClient.1.0.0.nupkg?raw=true

[reisenberger]

Thanks @altso, that's a great example. I see how this arises. Agreed that the breaking changes in v7 mean that an app fails with a runtime failure in this diamond-dependencies-conflict case. Note that this is a common concern for OSS libraries, not unique to Polly.

To your question:

Do you have any suggestions for the scenario when PollySeven.MyHttpClient is a third party library?

If this arises (... I am assuming this is a hypothetical now; do say if not), we could:

(a) Publish a new version of Polly, v8, which undoes the internal rearrangements to Polly code in v7 which are a partial cause; or (EDIT: I should have been clearer that I listed (a) (going backwards) to suggest that it was not really a serious option... )
(b) Ask the third-party library to provide a version of their library built against Polly v7.

You can see the dilemma. I think the sensible position for me as a library maintainer is to recommend (b). ;~) This is a general question for any OSS library. OSS libraries evolve - we cannot (in a general sense) be held back from evolving by third-parties who may choose not to evolve with us. On the other hand, of course we have a responsibility to promote stability in the ecosystem and strive to avoid frequent breaking changes. In general on Polly we have promoted stability for a long time (no breaking changes for 10 months ... then for 18 months before that ... ... tho not sure that is always a good metric: there are significant breaking changes to Polly syntax (edit: also not affecting httpclientfactory) that it would be good to introduce to open up new development paths!).

I hope that makes sense for now? Open to feedback if people think we are getting this wrong.

If a case arises, let's tackle it, but my instinct is that the first approach for your question should be (b).

[altso]

@reisenberger thanks for looking into that. The issue arose in a couple of internal nuget packages and we fixed that by adding an explicit reference to Polly v7 in each of them.

I do not think option (a) is a way to go as you will introduce a breaking change between v7 and v8. I also think option (b) is better but not ideal as it requires additional work from other devs who might not be aware of the issue at all.

A colleague of mine suggested another approach. Let's call it option (c):

(c) Bring back all the missing types and methods with the same exact signatures as they had in v6, but provide a bridge to v7 implementations. Something like this:

using System;
using System.ComponentModel;

namespace Polly
{
    [Obsolete]
    [EditorBrowsable(EditorBrowsableState.Never)]
    public static class RetryTResultSyntaxAsync
    {
        [Obsolete]
        [EditorBrowsable(EditorBrowsableState.Never)]
        public static RetryPolicy<TResult> RetryAsync<TResult>(PolicyBuilder<TResult> policyBuilder)
        {
            // The line below actually does not compile as there is no conversion
            // between AsyncRetryPolicy<TResult> and RetryPolicy<TResult>.
            // Call the new api via extension method
            return policyBuilder.RetryAsync(); 
        }
    }
}

Please note:

• types and methods are marked as Obsolete and EditorBrowsable(EditorBrowsableState.Never) to avoid new consumers of this api;
• there is no this keyword in front of PolicyBuilder to avoid compiler confusion around extension methods with the same name.

If not mistaken, that should bring back the binary compatibility between v6 and v7.

Unfortunately, I do not know how to make the code above compile and work properly at the moment - just an idea, but I can take a deeper look if you think it's worth it. Let me know.

Also, that would be less of an issue if Microsoft.Extensions.Http.Polly is updated to Polly v7. I see the plans to upgrade in v3, but not sure if it's planned for v2.x as well.

[reisenberger]

Thanks @altso for the deep engagement on this issue. Re the fact that (a) would introduce another breaking change: yes, very much so, hence (per semver) my flagging that the replacement would be v8.

I agree that an approach along the lines of (c) might be possible in principle (not tried it, but I see the logic...), but in this case I think we cannot achieve it. We cannot:

bring back all the missing types and methods with the same exact signatures as they had in v6

because, as you observe, the return type for the async policies has (intentionally) changed so that they are now separate AsyncRetryPolicy<TResult> not RetryPolicy<TResult>; so in fact it is not possible to supply a bridge class with methods of the exact same signature. This change is one of the intentional breaking changes of Polly v7 to clarify policy use. (Again, if you/anyone can see something clever I'm missing, fire away.)

---

There is also a fourth approach I forgot yesterday:

(d) ILMerge or ILRepack to hide the dependency on one axis.

I haven't tried this, and don't know if ILMerge and ILRepack are still a "thing", but worth keeping on the radar. It would only be needed in the case that a third-party who had been asked to update their library to v7, per suggestion (b) above, was unresponsive or unwilling. It might be possible in that case to ILRepack/ILMerge-ing their dll-built-against-Polly-v6 to merge Polly v6 in with it, thus hiding the Polly v6 dll.

But (b) is the obvious preferred route.

The HttpClientFactory case is already solved per my earlier comment. I've added guidance on this on both the Polly v7 page and Polly with HttpClientFactory page.

[altso]

Thanks for the clarification @reisenberger. It sounds like there is no way to mitigate this issue in Polly itself. Option (b) worked for me and hopefully will work for others as well.

[reisenberger]

Thanks @altso .

I'm going to close this out as I think we're concluded, but please do re-open if you have further comments/questions; the discussion has surfaced lots of useful detail.

TL;DR It's the result of an (intended) breaking change between Polly v6 and v7; and yes, diamond-dependency conflicts are a fact of life in the ecosystem if third-party libraries do not update.