Fix retrying to correctly deal with a SemiFuture-returning policy

Summary: Retrying was modified to support SemiFuture returning policy and function, but incorrectly handled the return type propagation. Fix the problem and add a test to verify.

Reviewed By: yfeldblum

Differential Revision: D20462982

fbshipit-source-id: 97b4dad6d83dbbfe7f1846b457bf915c14b9f31a

folly/futures/Retrying.h

@@ -67,7 +67,7 @@ namespace futures {
  *  overflow due to the recursive nature of the retry implementation
  */
 template <class Policy, class FF>
-invoke_result_t<FF, size_t> retrying(Policy&& p, FF&& ff);
+auto retrying(Policy&& p, FF&& ff);
 
 namespace detail {
 
@@ -79,11 +79,14 @@ struct retrying_policy_traits {
   using result = invoke_result_t<Policy, size_t, const exception_wrapper&>;
   using is_raw = std::is_same<result, bool>;
   using is_fut = std::is_same<result, Future<bool>>;
+  using is_semi_fut = std::is_same<result, SemiFuture<bool>>;
   using tag = typename std::conditional<
       is_raw::value,
       retrying_policy_raw_tag,
-      typename std::conditional<is_fut::value, retrying_policy_fut_tag, void>::
-          type>::type;
+      typename std::conditional<
+          is_fut::value || is_semi_fut::value,
+          retrying_policy_fut_tag,
+          void>::type>::type;
 };
 
 template <class Policy, class FF, class Prom>
@@ -136,7 +139,8 @@ template <class Policy, class FF>
 typename std::enable_if<
     isSemiFuture<invoke_result_t<FF, size_t>>::value ||
         isSemiFuture<invoke_result_t<Policy, size_t, exception_wrapper>>::value,
-    invoke_result_t<FF, size_t>>::type
+    SemiFuture<typename isFutureOrSemiFuture<
+        invoke_result_t<FF, size_t>>::Inner>>::type
 retrying(size_t k, Policy&& p, FF&& ff) {
   auto sf = folly::makeSemiFuture().deferExValue(
       [k, p = std::forward<Policy>(p), ff = std::forward<FF>(ff)](
@@ -162,8 +166,7 @@ retrying(Policy&& p, FF&& ff, retrying_policy_raw_tag) {
 }
 
 template <class Policy, class FF>
-invoke_result_t<FF, size_t>
-retrying(Policy&& p, FF&& ff, retrying_policy_fut_tag) {
+auto retrying(Policy&& p, FF&& ff, retrying_policy_fut_tag) {
   return retrying(0, std::forward<Policy>(p), std::forward<FF>(ff));
 }
 
@@ -263,7 +266,7 @@ retryingPolicyCappedJitteredExponentialBackoff(
 } // namespace detail
 
 template <class Policy, class FF>
-invoke_result_t<FF, size_t> retrying(Policy&& p, FF&& ff) {
+auto retrying(Policy&& p, FF&& ff) {
   using tag = typename detail::retrying_policy_traits<Policy>::tag;
   return detail::retrying(std::forward<Policy>(p), std::forward<FF>(ff), tag());
 }

folly/futures/test/RetryingTest.cpp

@@ -59,9 +59,14 @@ TEST(RetryingTest, has_op_call) {
   using ew = exception_wrapper;
   auto policy_raw = [](size_t n, const ew&) { return n < 3; };
   auto policy_fut = [](size_t n, const ew&) { return makeFuture(n < 3); };
+  auto policy_semi_fut = [](size_t n, const ew&) {
+    return makeSemiFuture(n < 3);
+  };
   using namespace futures::detail;
   EXPECT_TRUE(retrying_policy_traits<decltype(policy_raw)>::is_raw::value);
   EXPECT_TRUE(retrying_policy_traits<decltype(policy_fut)>::is_fut::value);
+  EXPECT_TRUE(
+      retrying_policy_traits<decltype(policy_semi_fut)>::is_semi_fut::value);
 }
 
 TEST(RetryingTest, basic) {
@@ -122,6 +127,23 @@ TEST(RetryingTest, policy_future) {
   EXPECT_EQ(2, sleeps);
 }
 
+TEST(RetryingTest, policy_semi_future) {
+  atomic<size_t> sleeps{0};
+  auto r = futures::retrying(
+               [&](size_t n, const exception_wrapper&) {
+                 return n < 3 ? makeSemiFuture(++sleeps).deferValue(
+                                    [](auto&&) { return true; })
+                              : makeSemiFuture(false);
+               },
+               [](size_t n) {
+                 return n < 2 ? makeFuture<size_t>(runtime_error("ha"))
+                              : makeFuture(n);
+               })
+               .wait();
+  EXPECT_EQ(2, r.value());
+  EXPECT_EQ(2, sleeps);
+}
+
 TEST(RetryingTest, policy_basic) {
   auto r = futures::retrying(
                futures::retryingPolicyBasic(3),