Fix non-generic Fallback with generic Execute method (#372)

Closes #294.

Author: reisenberger

CHANGELOG.md

@@ -1,9 +1,10 @@
 ## 5.6.0
 - Add ability to handle inner exceptions natively: .HandleInner<TEx>()
 - Allow WaitAndRetry policies to calculate wait based on the handled fault
-- Add GetPolicies() extension methods to IPolicyWrap
-- Allow PolicyWrap to take interfaces as parameters
-- Bug fix: set context keys for generic execute method with PolicyWrap
+- Add the ability to access the policies within an IPolicyWrap
+- Allow PolicyWrap to configure policies expressed as interfaces
+- Bug fix: set context keys for generic execute methods with PolicyWrap
+- Bug fix: generic TResult method with non-generic fallback policy
 - Performance improvements
 - Multiple build speed improvements
 

src/Polly.Net40Async.nuspec

@@ -19,9 +19,10 @@
      ---------------------
      - Add ability to handle inner exceptions natively: .HandleInner<TEx>()
      - Allow WaitAndRetry policies to calculate wait based on the handled fault
-     - Add GetPolicies() extension methods to IPolicyWrap
+     - Add the ability to access the policies within an IPolicyWrap
      - Allow PolicyWrap to take interfaces as parameters
      - Bug fix: set context keys for generic execute method with PolicyWrap
+     - Bug fix: generic TResult method with non-generic fallback policy
      - Performance improvements
 
      5.5.0

src/Polly.Shared/Fallback/FallbackPolicy.cs

@@ -13,6 +13,19 @@ internal FallbackPolicy(Action<Action<Context, CancellationToken>, Context, Canc
             : base(exceptionPolicy, exceptionPredicates)
         {
         }
+
+        /// <summary>
+        /// Executes the specified action within the cache policy and returns the result.
+        /// </summary>
+        /// <typeparam name="TResult">The type of the result.</typeparam>
+        /// <param name="action">The action to perform.</param>
+        /// <param name="context">Execution context that is passed to the exception policy; defines the cache key to use in cache lookup.</param>
+        /// <param name="cancellationToken">The cancellation token.</param>
+        /// <returns>The value returned by the action, or the cache.</returns>
+        public override TResult ExecuteInternal<TResult>(Func<Context, CancellationToken, TResult> action, Context context, CancellationToken cancellationToken)
+        {
+          throw new InvalidOperationException($"You have executed the generic .Execute<{nameof(TResult)}> method on a non-generic {nameof(FallbackPolicy)}.  A non-generic {nameof(FallbackPolicy)} only defines a fallback action which returns void; it can never return a substitute {nameof(TResult)} value.  To use {nameof(FallbackPolicy)} to provide fallback {nameof(TResult)} values you must define a generic fallback policy {nameof(FallbackPolicy)}<{nameof(TResult)}>.  For example, define the policy as Policy<{nameof(TResult)}>.Handle<Whatever>.Fallback<{nameof(TResult)}>(/* some {nameof(TResult)} value or Func<..., {nameof(TResult)}> */);");
+        }
     }
 
     /// <summary>

src/Polly.Shared/Fallback/FallbackPolicyAsync.cs

@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.Threading;
 using System.Threading.Tasks;
 
@@ -11,6 +12,22 @@ internal FallbackPolicy(Func<Func<Context, CancellationToken, Task>, Context, Ca
            : base(asyncExceptionPolicy, exceptionPredicates)
         {
         }
+
+        /// <summary>
+        ///     Executes the specified asynchronous action within the policy and returns the result.
+        /// </summary>
+        /// <typeparam name="TResult">The type of the result.</typeparam>
+        /// <param name="action">The action to perform.</param>
+        /// <param name="context">Context data that is passed to the exception policy.</param>
+        /// <param name="continueOnCapturedContext">Whether to continue on a captured synchronization context.</param>
+        /// <param name="cancellationToken">A cancellation token which can be used to cancel the action.  When a retry policy is in use, also cancels any further retries.</param>
+        /// <returns>The value returned by the action</returns>
+        /// <exception cref="System.InvalidOperationException">Please use asynchronous-defined policies when calling asynchronous ExecuteAsync (and similar) methods.</exception>
+        [DebuggerStepThrough]
+        public override Task<TResult> ExecuteAsyncInternal<TResult>(Func<Context, CancellationToken, Task<TResult>> action, Context context, CancellationToken cancellationToken, bool continueOnCapturedContext)
+        {
+            throw new InvalidOperationException($"You have executed the generic .Execute<{nameof(TResult)}> method on a non-generic {nameof(FallbackPolicy)}.  A non-generic {nameof(FallbackPolicy)} only defines a fallback action which returns void; it can never return a substitute {nameof(TResult)} value.  To use {nameof(FallbackPolicy)} to provide fallback {nameof(TResult)} values you must define a generic fallback policy {nameof(FallbackPolicy)}<{nameof(TResult)}>.  For example, define the policy as Policy<{nameof(TResult)}>.Handle<Whatever>.Fallback<{nameof(TResult)}>(/* some {nameof(TResult)} value or Func<..., {nameof(TResult)}> */);");
+        }
     }
 
     public partial class FallbackPolicy<TResult> : IFallbackPolicy<TResult>

src/Polly.SharedSpecs/Fallback/FallbackAsyncSpecs.cs

@@ -251,6 +251,16 @@ public void Should_not_handle_exception_thrown_by_fallback_delegate_even_if_is_e
             fallbackActionExecuted.Should().BeTrue();
         }
 
+        [Fact]
+        public void Should_throw_for_generic_method_execution_on_non_generic_policy()
+        {
+            FallbackPolicy fallbackPolicy = Policy
+                .Handle<DivideByZeroException>()
+                .FallbackAsync(_ => TaskHelper.EmptyTask);
+
+            fallbackPolicy.Awaiting(p => p.ExecuteAsync<int>(() => TaskHelper.FromResult(0))).ShouldThrow<InvalidOperationException>();
+        }
+
         #endregion
 
         #region onPolicyEvent delegate tests

src/Polly.SharedSpecs/Fallback/FallbackSpecs.cs

@@ -316,6 +316,16 @@ public void Should_not_handle_exception_thrown_by_fallback_delegate_even_if_is_e
             fallbackActionExecuted.Should().BeTrue();
         }
 
+        [Fact]
+        public void Should_throw_for_generic_method_execution_on_non_generic_policy()
+        {
+            FallbackPolicy fallbackPolicy = Policy
+                .Handle<DivideByZeroException>()
+                .Fallback(() => {});
+
+            fallbackPolicy.Invoking(p => p.Execute<int>(() => 0)).ShouldThrow<InvalidOperationException>();
+        }
+
         #endregion
 
         #region HandleInner tests, inner of normal exceptions

src/Polly.SharedSpecs/Helpers/PolicyExtensions.cs

@@ -22,7 +22,7 @@ public static void RaiseException<TException>(this Policy policy, TException ins
                 NumberOfTimesToRaiseException = 1
             };
 
-            policy.RaiseExceptionAndOrCancellation(scenario, new CancellationTokenSource(), () => { }, _ => instance, 0);
+            policy.RaiseExceptionAndOrCancellation(scenario, new CancellationTokenSource(), () => { }, _ => instance);
         }
 
         public static void RaiseException<TException>(this Policy policy, Action<TException, int> configureException = null) where TException : Exception, new()
@@ -46,12 +46,12 @@ public static void RaiseException<TException>(this Policy policy, TException ins
                 return exception;
             };
 
-            policy.RaiseExceptionAndOrCancellation(scenario, new CancellationTokenSource(), () => { }, exceptionFactory, 0);
+            policy.RaiseExceptionAndOrCancellation(scenario, new CancellationTokenSource(), () => { }, exceptionFactory);
         }
 
         public static void RaiseExceptionAndOrCancellation<TException>(this Policy policy, ExceptionAndOrCancellationScenario scenario, CancellationTokenSource cancellationTokenSource, Action onExecute) where TException : Exception, new()
         {
-            policy.RaiseExceptionAndOrCancellation<TException, int>(scenario, cancellationTokenSource, onExecute, 0);
+            policy.RaiseExceptionAndOrCancellation<TException>(scenario, cancellationTokenSource, onExecute, _ => new TException());
         }
 
         public static TResult RaiseExceptionAndOrCancellation<TException, TResult>(this Policy policy, ExceptionAndOrCancellationScenario scenario, CancellationTokenSource cancellationTokenSource, Action onExecute, TResult successResult) where TException : Exception, new()
@@ -60,6 +60,36 @@ public static void RaiseException<TException>(this Policy policy, TException ins
                 _ => new TException(), successResult);
         }
 
+        public static void RaiseExceptionAndOrCancellation<TException>(this Policy policy, ExceptionAndOrCancellationScenario scenario, CancellationTokenSource cancellationTokenSource, Action onExecute, Func<int, TException> exceptionFactory) where TException : Exception
+        {
+            int counter = 0;
+
+            CancellationToken cancellationToken = cancellationTokenSource.Token;
+
+            policy.Execute(ct =>
+            {
+                onExecute();
+
+                counter++;
+
+                if (scenario.AttemptDuringWhichToCancel.HasValue && counter >= scenario.AttemptDuringWhichToCancel.Value)
+                {
+                    cancellationTokenSource.Cancel();
+                }
+
+                if (scenario.ActionObservesCancellation)
+                {
+                    ct.ThrowIfCancellationRequested();
+                }
+
+                if (counter <= scenario.NumberOfTimesToRaiseException)
+                {
+                    throw exceptionFactory(counter);
+                }
+
+            }, cancellationToken);
+        }
+
         public static TResult RaiseExceptionAndOrCancellation<TException, TResult>(this Policy policy, ExceptionAndOrCancellationScenario scenario, CancellationTokenSource cancellationTokenSource, Action onExecute, Func<int, TException> exceptionFactory, TResult successResult) where TException : Exception
         {
             int counter = 0;

src/Polly.SharedSpecs/Helpers/PolicyExtensionsAsync.cs

@@ -25,7 +25,7 @@ public static Task RaiseExceptionAsync<TException>(this Policy policy, TExceptio
                 NumberOfTimesToRaiseException = 1
             };
 
-            return policy.RaiseExceptionAndOrCancellationAsync(scenario, new CancellationTokenSource(), () => { }, _ => instance, 0);
+            return policy.RaiseExceptionAndOrCancellationAsync(scenario, new CancellationTokenSource(), () => { }, _ => instance);
         }
 
         public static Task RaiseExceptionAsync<TException>(this Policy policy, Action<TException, int> configureException = null) where TException : Exception, new()
@@ -49,12 +49,12 @@ public static Task RaiseExceptionAsync<TException>(this Policy policy, TExceptio
                 return exception;
             };
 
-            return policy.RaiseExceptionAndOrCancellationAsync(scenario, new CancellationTokenSource(), () => { }, exceptionFactory, 0);
+            return policy.RaiseExceptionAndOrCancellationAsync(scenario, new CancellationTokenSource(), () => { }, exceptionFactory);
         }
 
         public static Task RaiseExceptionAndOrCancellationAsync<TException>(this Policy policy, ExceptionAndOrCancellationScenario scenario, CancellationTokenSource cancellationTokenSource, Action onExecute) where TException : Exception, new()
         {
-            return policy.RaiseExceptionAndOrCancellationAsync<TException, int>(scenario, cancellationTokenSource, onExecute, 0);
+            return policy.RaiseExceptionAndOrCancellationAsync<TException>(scenario, cancellationTokenSource, onExecute, _ => new TException());
         }
 
         public static Task<TResult> RaiseExceptionAndOrCancellationAsync<TException, TResult>(this Policy policy, ExceptionAndOrCancellationScenario scenario, CancellationTokenSource cancellationTokenSource, Action onExecute, TResult successResult) where TException : Exception, new()
@@ -63,6 +63,37 @@ public static Task RaiseExceptionAsync<TException>(this Policy policy, TExceptio
                 _ => new TException(), successResult);
         }
 
+        public static Task RaiseExceptionAndOrCancellationAsync<TException>(this Policy policy, ExceptionAndOrCancellationScenario scenario, CancellationTokenSource cancellationTokenSource, Action onExecute, Func<int, TException> exceptionFactory) where TException : Exception
+        {
+            int counter = 0;
+
+            CancellationToken cancellationToken = cancellationTokenSource.Token;
+
+            return policy.ExecuteAsync(ct =>
+            {
+                onExecute();
+
+                counter++;
+
+                if (scenario.AttemptDuringWhichToCancel.HasValue && counter >= scenario.AttemptDuringWhichToCancel.Value)
+                {
+                    cancellationTokenSource.Cancel();
+                }
+
+                if (scenario.ActionObservesCancellation)
+                {
+                    ct.ThrowIfCancellationRequested();
+                }
+
+                if (counter <= scenario.NumberOfTimesToRaiseException)
+                {
+                    throw exceptionFactory(counter);
+                }
+
+                return TaskHelper.EmptyTask;
+            }, cancellationToken);
+        }
+
         public static Task<TResult> RaiseExceptionAndOrCancellationAsync<TException, TResult>(this Policy policy, ExceptionAndOrCancellationScenario scenario, CancellationTokenSource cancellationTokenSource, Action onExecute, Func<int, TException> exceptionFactory, TResult successResult) where TException : Exception
         {
             int counter = 0;

src/Polly.SharedSpecs/Wrap/PolicyWrapContextAndKeySpecs.cs

@@ -115,14 +115,13 @@ public void Should_pass_outmost_PolicyWrap_Key_as_PolicyWrapKey_to_innermost_Pol
             var outerWrap = fallback.Wrap(innerWrap).WithPolicyKey(outerWrapKey);
 
             bool doneOnceOny = false;
-            outerWrap.Execute<int>(() =>
+            outerWrap.Execute(() =>
             {
                 if (!doneOnceOny)
                 {
                     doneOnceOny = true;
                     throw new Exception();
                 }
-                return 0;
             });
 
             policyWrapKeySetOnExecutionContext.Should().NotBe(retryKey);

src/Polly.SharedSpecs/Wrap/PolicyWrapContextAndKeySpecsAsync.cs

@@ -117,14 +117,14 @@ public async Task Should_pass_outmost_PolicyWrap_Key_as_PolicyWrapKey_to_innermo
             var outerWrap = fallback.WrapAsync(innerWrap).WithPolicyKey(outerWrapKey);
 
             bool doneOnceOny = false;
-            await outerWrap.ExecuteAsync<int>(() =>
+            await outerWrap.ExecuteAsync(() =>
             {
                 if (!doneOnceOny)
                 {
                     doneOnceOny = true;
                     throw new Exception();
                 }
-                return TaskHelper.FromResult(0);
+                return TaskHelper.EmptyTask;
             });
 
             policyWrapKeySetOnExecutionContext.Should().NotBe(retryKey);