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

TResult Fallback is incorrectly null [when non-generic Fallback policy is used with generic Execute<TResult> overloads] #294

Closed
jonnovaretti opened this issue Aug 13, 2017 · 4 comments
Milestone

Comments

@jonnovaretti
Copy link

jonnovaretti commented Aug 13, 2017

I'm creating a PolicyWrap with CircuitBreak and Fallback. When all occurs right, PolicyResult returns with my result, but when ocurrs a exception and the Fallback executes the PolicyResult bring result null or default value. I created a little test just with Fallback and the result is the same.

[TestMethod]
        public void Verificar_Funcionamento_Fallback()
        {
            FallbackPolicy policy;

            policy = Policy.Handle<DivideByZeroException>().Fallback(() => DivideWithoutZero(3, 0));

            var result = policy.ExecuteAndCapture(() => Divide(3, 0));
        }

        private int Divide(int x, int y)
        {
            Thread.Sleep(2000);
            var z = x / y;

            return z;
        }

        private int DivideWithoutZero(int v1, int v2)
        {
            int result = 99;

            if (v2 > 0)
                result = v1 / v2;

            return result;
        }

What did I do wrong?

@reisenberger
Copy link
Member

reisenberger commented Aug 13, 2017

Hi. Declaring the policy explicitly as FallbackPolicy policy has declared a non-generic policy. The compiler thus picks up this configuration overload: .Fallback(Action fallbackAction)

That this is not jumping out as a problem is in part caused by the fact that the compiler will silently accept assigning a Func<int> to an Action. It's not an error - it's just as you could execute DivideWithoutZero(3, 0); as a statement, without doing anything with its return value - but there isn't any warning to you from the compiler that this is happening.

Instead, you need to declare that the policy is explicitly for delegates returning int:

FallbackPolicy<int> policy = Policy<int>.Handle<DivideByZeroException>().Fallback(() => DivideWithoutZero(3, 0));

@reisenberger reisenberger changed the title Result Fallback is null Result Fallback is null [if Fallback is not configured for TResult] Aug 14, 2017
@reisenberger reisenberger changed the title Result Fallback is null [if Fallback is not configured for TResult] Result Fallback is null [if non-generic Fallback policy is used with generic Execute<TResult> overloads] Aug 18, 2017
@reisenberger
Copy link
Member

@jonnovaretti Thanks for raising this. Did the suggestion fix for you?

I think we should fix this in Polly so that it's not possible to fall into the situation you encountered. The combination is:

(1) A plain (non-generic) FallbackPolicy configures an Action fallbackAction to run in case of fallback
(2) All Polly non-Generic policies let you use a generic execute method .Execute<TResult>(...). But a generic-TResult execution for a plain FallbackPolicy doesn't make sense, because the Action fallbackAction configured can only ever return void, so can never provide a substitute TResult value.

As a result, I think we should treat FallbackPolicy (non-generic) .Execute<TResult>(...) as an InvalidOperationException.

@reisenberger
Copy link
Member

Marking 'up-for-grabs'. Needed:

  • override the generic method overloads .Execute<TResult>(...), on non-generic FallbackPolicy, so that they throw InvalidOperationException (for the reasons described in prev. comment).
  • unit tests

@reisenberger reisenberger changed the title Result Fallback is null [if non-generic Fallback policy is used with generic Execute<TResult> overloads] TResult Fallback is incorrectly null [when non-generic Fallback policy is used with generic Execute<TResult> overloads] Oct 8, 2017
@reisenberger reisenberger added this to the v5.6.0 milestone Nov 23, 2017
reisenberger added a commit to reisenberger/Polly that referenced this issue Nov 23, 2017
@reisenberger
Copy link
Member

Fixed in Polly v5.6.0 (will be released via nuget in next day or so). Now throws an exception as warning, when non-generic FallbackPolicy is used with generic methods .Execute<TResult>(...), with explanatory message:

You have executed the generic .Execute<TResult> method on a non-generic FallbackPolicy.  
A non-generic FallbackPolicy only defines a fallback action which returns void; it can never return a substitute TResult value.  
To use FallbackPolicy to provide fallback TResult values you must define a generic fallback policy FallbackPolicy<TResult>.  
For example, define the policy as Policy<TResult>.Handle<Whatever>.Fallback<TResult>(/* some TResult value or Func<..., TResult> */);

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

2 participants