App-vNext / Polly

Polly is a .NET resilience and transient-fault-handling library that allows developers to express policies such as Retry, Circuit Breaker, Timeout, Bulkhead Isolation, and Fallback in a fluent and thread-safe manner. From version 6.0.1, Polly targets .NET Standard 1.1 and 2.0+.
https://www.thepollyproject.org
BSD 3-Clause "New" or "Revised" License
13.44k stars 1.23k forks source link

How to free HttpResponceMessage in FallBackPolicy and return a new one. #495

Closed elviswatkins closed 6 years ago

elviswatkins commented 6 years ago

I am using a Circuit Breaker with a fallback to catch any exceptions or invalid status codes and return a new HttpResponseMessage to the client that can be interpreted as an error. I would like the Fallback to look as follows:

// fall back for any exception
var fallbackForCircuitBreaker = Policy<HttpResponseMessage>
    .Handle<Exception> ()                                             // if any exception occured or
    .OrResult (response => response.StatusCode != HttpStatusCode.OK)  // if request did not succeed go to fallback
    .Fallback (() => new HttpResponseMessage(HttpStatusCode.InternalServerError)
            {
            Content = new StringContent("fallback")
            },
        onFallback: b =>
            {
            if (b.Exception == null) // got response from service
                {
                var msg = b.Result.Content.ReadAsStringAsync().Result;
                logger.ForContext("Response", msg)
                        .Error("OnFallback Status: {Status}", b.Result.StatusCode);
                }
            else
                logger.ForContext("Exception", b.Exception.Format())
                        .Error("OnFallback Exception: {ExceptionMessage}", b.Exception.Message);
            });

My question is, how to Dispose the current HttpResponseMessage that was created via a upstream call to HttpClient.SendAsync and return a dummy HttpResponseMessage. I believe the onFallback delegate is called before the Fallback delegate. Can I Dispose the HttpResponseMessage there before the actual Fallback delegate? Or am I just missing something?

reisenberger commented 6 years ago

Initial aside: If this is for http calls through HttpClient, you'll want an async policy variant, ie, use FallbackAsync(...).


Timing-wise, the onFallbackAsync runs the statement before the fallbackAction, so it is likely safe to dispose the original HttpResponseMessage in the onFallbackAsync, given your posted code doesn't use it in the subsequent fallbackAction.

You do also have the option of this overload where both the fallbackAction and onFallbackAsync are provided the DelegateResult<HttpResponseMessage> as an input parameter. So (with that overload) you can also dispose the original HttpResponseMessage later, in the fallbackAction, if needed or preferred.

elviswatkins commented 6 years ago

Yes, I want to use async, but this is an older repository and cannot make those changes yet. It is in the backlog for the near future. I have gone with the second option and use the overload that takes the DelegateResult<HttpResponseMessage>. I also found a link that says it is not necessary to dispose the HttpResponseMessage, but my old school brain cannot get around not calling Dispose.

petrroll commented 3 years ago

I also found a link that says it is not necessary to dispose the HttpResponseMessage, but my old school brain cannot get around not calling Dispose.

Just as a caution for others. It is necessary, at least on some versions of .NET FW. We got bit by that on .NET FW 4.7.2 for example.