Fix broken promise error in folly::retrying

Summary: This diff shows an issue in `folly::retrying`. When the future generation function throws an exception and `folly::retrying` is nested in another functor that returns Future, it will throw `broken promise` instead of the actual exception message, which can be very generic and confusing. Fix is to capture the exception so that exact error message can be propagated up.

Reviewed By: yfeldblum

Differential Revision: D5050690

fbshipit-source-id: 5b9b24977788f60aa778bb8e9cdf4281ea9a0023

diff --git a/folly/futures/Future-inl.h b/folly/futures/Future-inl.h
index 0e6c7e31c40f8e32f1205530844429a84044f203..c4f9ca2f307ab6b49e6c1286f1947346d72867f4 100644 (file)
--- a/folly/futures/Future-inl.h
+++ b/folly/futures/Future-inl.h
@@ -1233,7 +1233,7 @@ template <class Policy, class FF, class Prom>
 void retryingImpl(size_t k, Policy&& p, FF&& ff, Prom prom) {
   using F = typename std::result_of<FF(size_t)>::type;
   using T = typename F::value_type;
 void retryingImpl(size_t k, Policy&& p, FF&& ff, Prom prom) {
   using F = typename std::result_of<FF(size_t)>::type;
   using T = typename F::value_type;

-  auto f = ff(k++);
+  auto f = makeFutureWith([&] { return ff(k++); });

   f.then([
     k,
     prom = std::move(prom),
   f.then([
     k,
     prom = std::move(prom),

diff --git a/folly/futures/test/RetryingTest.cpp b/folly/futures/test/RetryingTest.cpp
index 2ec569fa2169826f08fd87eb0bb63750fe607766..59eb3e0156a6110686c26a572ebd48a3ebf0dca8 100644 (file)
--- a/folly/futures/test/RetryingTest.cpp
+++ b/folly/futures/test/RetryingTest.cpp
@@ -75,6 +75,27 @@ TEST(RetryingTest, basic) {
   EXPECT_EQ(2, r.value());
 }
 
   EXPECT_EQ(2, r.value());
 }
 

+TEST(RetryingTest, future_factory_throws) {
+  struct ReturnedException : exception {};
+  struct ThrownException : exception {};
+  auto result = futures::retrying(
+                    [](size_t n, const exception_wrapper&) { return n < 2; },
+                    [](size_t n) {
+                      switch (n) {
+                        case 0:
+                          return makeFuture<size_t>(
+                              make_exception_wrapper<ReturnedException>());
+                        case 1:
+                          throw ThrownException();
+                        default:
+                          return makeFuture(n);
+                      }
+                    })
+                    .wait()
+                    .getTry();
+  EXPECT_THROW(result.throwIfFailed(), ThrownException);
+}
+

 TEST(RetryingTest, policy_future) {
   atomic<size_t> sleeps {0};
   auto r = futures::retrying(
 TEST(RetryingTest, policy_future) {
   atomic<size_t> sleeps {0};
   auto r = futures::retrying(