fix(download-clients): one circuit breaker per client, not one for all of them

[m4bard]

Summary

AddDownloadClientHttpClients built one circuit breaker and passed the same instance to five HttpClient registrations. Polly's circuit breaker is stateful, so that is one global breaker rather than one per client, and three transient failures against any single download client open the circuit for all of them.

Full write-up in #867.

Changes

Fixed

• Each download-client HttpClient builds its own circuit breaker through a CreateCircuitBreakerPolicy() factory, rather than sharing one instance.

Deliberately unchanged

• The retry policy is still a single shared instance. WaitAndRetryAsync is stateless, so sharing it is correct and is how Polly is intended to be used. Splitting it too would suggest the sharing itself was the mistake rather than the sharing of state. There is a comment at the registration saying which of the two is stateful, so the difference does not read as an oversight later.
• The thresholds are untouched. Three failures and a thirty second break may or may not be right, but that is tuning and it is separable from whether the clients share a breaker.

Why it outlasts its trigger

Polly's simple breaker returns straight to Open when one half-open trial fails, rather than requiring another run of three. With the breaker shared, that trial can be consumed by whichever client polls next, so a client that is still intermittently failing keeps re-tripping it and the others never get a successful trial through. A brief blip on one client becomes an open-ended outage for all of them.

Testing

DownloadClientCircuitBreakerIsolationTests asserts the factory returns distinct instances, opens the first with three transient failures, confirms it then refuses calls, and confirms the second still executes.

It drives the policies directly rather than through HttpClient on purpose. Going through the named clients also passes through the retry policy, whose backoff is 2, 4 and 8 seconds, so opening a breaker that way costs about 45 seconds to observe a property that is visible in milliseconds otherwise.

Verified as a real guard: with the factory changed to return one cached instance, which is the original defect, the test fails on Assert.NotSame.

Full suite: 3,030 passed, 0 failed, 125 skipped, against a 3,029 baseline on 03958c15.

Note

Not a regression from recent work. git diff on this file between f27c7989 and current canary shows no changes.

I have not reproduced the outage. What I verified is the registration, that one object reference reaches all five AddPolicyHandler calls, and Polly's documented behaviour for a stateful policy. The runtime evidence in the issue is a reporter's rather than mine, and the issue says so.