onError callbacks

Summary:
We've discussed these a bunch and here they are. I stole a bunch of Hannes' magic from https://www.facebook.com/groups/715931878455430/permalink/772854686096482/ to make this easier

Test Plan: added lots of unit tests

Reviewed By: hans@fb.com

Subscribers: fugalh, folly-diffs@, hannesr

FB internal diff: D1748418

Signature: t1:1748418:1418945606:e14c7d6a31245e222bc6a0d259d0e2b9ddd5a830

diff --git a/folly/wangle/futures/Future-inl.h b/folly/wangle/futures/Future-inl.h
index 074571dbe2f1becf2f7035084fb9f0f587ae87a7..f2816b487e694035d215c654b36c3aef9d93d77b 100644 (file)
--- a/folly/wangle/futures/Future-inl.h
+++ b/folly/wangle/futures/Future-inl.h
@@ -305,6 +305,77 @@ Future<void> Future<T>::then() {
   return then([] (Try<T>&& t) {});
 }
 
+// onError where the callback returns T
+template <class T>
+template <class F>
+typename std::enable_if<
+  !detail::Extract<F>::ReturnsFuture::value,
+  Future<T>>::type
+Future<T>::onError(F&& func) {
+  typedef typename detail::Extract<F>::FirstArg Exn;
+  static_assert(
+      std::is_same<typename detail::Extract<F>::RawReturn, T>::value,
+      "Return type of onError callback must be T or Future<T>");
+
+  Promise<T> p;
+  auto f = p.getFuture();
+  auto pm = folly::makeMoveWrapper(std::move(p));
+  auto funcm = folly::makeMoveWrapper(std::move(func));
+  setCallback_([pm, funcm](Try<T>&& t) mutable {
+    try {
+      t.throwIfFailed();
+    } catch (Exn& e) {
+      pm->fulfil([&]{
+        return (*funcm)(e);
+      });
+      return;
+    } catch (...) {
+      // fall through
+    }
+    pm->fulfilTry(std::move(t));
+  });
+
+  return f;
+}
+
+// onError where the callback returns Future<T>
+template <class T>
+template <class F>
+typename std::enable_if<
+  detail::Extract<F>::ReturnsFuture::value,
+  Future<T>>::type
+Future<T>::onError(F&& func) {
+  static_assert(
+      std::is_same<typename detail::Extract<F>::Return, Future<T>>::value,
+      "Return type of onError callback must be T or Future<T>");
+  typedef typename detail::Extract<F>::FirstArg Exn;
+
+  Promise<T> p;
+  auto f = p.getFuture();
+  auto pm = folly::makeMoveWrapper(std::move(p));
+  auto funcm = folly::makeMoveWrapper(std::move(func));
+  setCallback_([pm, funcm](Try<T>&& t) mutable {
+    try {
+      t.throwIfFailed();
+    } catch (Exn& e) {
+      try {
+        auto f2 = (*funcm)(e);
+        f2.setCallback_([pm](Try<T>&& t2) mutable {
+          pm->fulfilTry(std::move(t2));
+        });
+      } catch (...) {
+        pm->setException(std::current_exception());
+      }
+      return;
+    } catch (...) {
+      // fall through
+    }
+    pm->fulfilTry(std::move(t));
+  });
+
+  return f;
+}
+
 template <class T>
 typename std::add_lvalue_reference<T>::type Future<T>::value() {
   throwIfInvalid();

diff --git a/folly/wangle/futures/Future.h b/folly/wangle/futures/Future.h
index cdea93df95f1242125d787cabca06d6570a9108c..15349e395158d2d7d0b2455236f671bb2e783c0c 100644 (file)
--- a/folly/wangle/futures/Future.h
+++ b/folly/wangle/futures/Future.h
@@ -30,17 +30,62 @@
 namespace folly { namespace wangle {
 
 namespace detail {
-  template <class> struct Core;
-  template <class...> struct VariadicContext;
-
-  template <class T>
-  struct AliasIfVoid {
-    typedef typename std::conditional<
-      std::is_same<T, void>::value,
-      int,
-      T>::type type;
-  };
-}
+
+template <class> struct Core;
+template <class...> struct VariadicContext;
+
+template <class T>
+struct AliasIfVoid {
+  typedef typename std::conditional<
+    std::is_same<T, void>::value,
+    int,
+    T>::type type;
+};
+
+
+template <typename T>
+struct IsFuture : std::integral_constant<bool, false> {
+    typedef T Inner;
+};
+
+template <template <typename T> class Future, typename T>
+struct IsFuture<Future<T>> : std::integral_constant<bool, true> {
+    typedef T Inner;
+};
+
+template <typename...>
+struct ArgType;
+
+template <typename Arg, typename... Args>
+struct ArgType<Arg, Args...> {
+  typedef Arg FirstArg;
+};
+
+template <>
+struct ArgType<> {
+  typedef void FirstArg;
+};
+
+template <typename L>
+struct Extract : Extract<decltype(&L::operator())> { };
+
+template <typename Class, typename R, typename... Args>
+struct Extract<R(Class::*)(Args...) const> {
+  typedef IsFuture<R> ReturnsFuture;
+  typedef Future<typename ReturnsFuture::Inner> Return;
+  typedef typename ReturnsFuture::Inner RawReturn;
+  typedef typename ArgType<Args...>::FirstArg FirstArg;
+};
+
+template <typename Class, typename R, typename... Args>
+struct Extract<R(Class::*)(Args...)> {
+  typedef IsFuture<R> ReturnsFuture;
+  typedef Future<typename ReturnsFuture::Inner> Return;
+  typedef typename ReturnsFuture::Inner RawReturn;
+  typedef typename ArgType<Args...>::FirstArg FirstArg;
+};
+
+} // detail
 
 template <class> struct Promise;
 
@@ -302,6 +347,33 @@ class Future {
   /// Exceptions still propagate.
   Future<void> then();
 
+  /// Set an error callback for this Future. The callback should take a single
+  /// argument of the type that you want to catch, and should return a value of
+  /// the same type as this Future, or a Future of that type (see overload
+  /// below). For instance,
+  ///
+  /// makeFuture()
+  ///   .then([] {
+  ///     throw std::runtime_error("oh no!");
+  ///     return 42;
+  ///   })
+  ///   .onError([] (std::runtime_error& e) {
+  ///     LOG(INFO) << "std::runtime_error: " << e.what();
+  ///     return -1; // or makeFuture<int>(-1)
+  ///   });
+  template <class F>
+  typename std::enable_if<
+    !detail::Extract<F>::ReturnsFuture::value,
+    Future<T>>::type
+  onError(F&& func);
+
+  /// Overload of onError where the error callback returns a Future<T>
+  template <class F>
+  typename std::enable_if<
+    detail::Extract<F>::ReturnsFuture::value,
+    Future<T>>::type
+  onError(F&& func);
+
   /// This is not the method you're looking for.
   ///
   /// This needs to be public because it's used by make* and when*, and it's

diff --git a/folly/wangle/futures/test/FutureTest.cpp b/folly/wangle/futures/test/FutureTest.cpp
index fedd7a3564d332a26eacdee7b6dc807ae8a70967..3e8fc97ab142b53b42892c1a990acdfb80d93301 100644 (file)
--- a/folly/wangle/futures/test/FutureTest.cpp
+++ b/folly/wangle/futures/test/FutureTest.cpp
@@ -60,7 +60,7 @@ class ThreadExecutor : public Executor {
   }
 
  public:
-  ThreadExecutor(size_t n = 1024)
+  explicit ThreadExecutor(size_t n = 1024)
     : funcs(n), worker(std::bind(&ThreadExecutor::work, this)) {}
 
   ~ThreadExecutor() {
@@ -83,6 +83,172 @@ static eggs_t eggs("eggs");
 
 // Future
 
+TEST(Future, onError) {
+  bool theFlag = false;
+  auto flag = [&]{ theFlag = true; };
+#define EXPECT_FLAG() \
+  do { \
+    EXPECT_TRUE(theFlag); \
+    theFlag = false; \
+  } while(0);
+
+#define EXPECT_NO_FLAG() \
+  do { \
+    EXPECT_FALSE(theFlag); \
+    theFlag = false; \
+  } while(0);
+
+  // By reference
+  {
+    auto f = makeFuture()
+      .then([] { throw eggs; })
+      .onError([&] (eggs_t& e) { flag(); });
+    EXPECT_FLAG();
+    EXPECT_NO_THROW(f.value());
+  }
+
+  {
+    auto f = makeFuture()
+      .then([] { throw eggs; })
+      .onError([&] (eggs_t& e) { flag(); return makeFuture(); });
+    EXPECT_FLAG();
+    EXPECT_NO_THROW(f.value());
+  }
+
+  // By value
+  {
+    auto f = makeFuture()
+      .then([] { throw eggs; })
+      .onError([&] (eggs_t e) { flag(); });
+    EXPECT_FLAG();
+    EXPECT_NO_THROW(f.value());
+  }
+
+  {
+    auto f = makeFuture()
+      .then([] { throw eggs; })
+      .onError([&] (eggs_t e) { flag(); return makeFuture(); });
+    EXPECT_FLAG();
+    EXPECT_NO_THROW(f.value());
+  }
+
+  // Polymorphic
+  {
+    auto f = makeFuture()
+      .then([] { throw eggs; })
+      .onError([&] (std::exception& e) { flag(); });
+    EXPECT_FLAG();
+    EXPECT_NO_THROW(f.value());
+  }
+
+  {
+    auto f = makeFuture()
+      .then([] { throw eggs; })
+      .onError([&] (std::exception& e) { flag(); return makeFuture(); });
+    EXPECT_FLAG();
+    EXPECT_NO_THROW(f.value());
+  }
+
+  // Non-exceptions
+  {
+    auto f = makeFuture()
+      .then([] { throw -1; })
+      .onError([&] (int e) { flag(); });
+    EXPECT_FLAG();
+    EXPECT_NO_THROW(f.value());
+  }
+
+  {
+    auto f = makeFuture()
+      .then([] { throw -1; })
+      .onError([&] (int e) { flag(); return makeFuture(); });
+    EXPECT_FLAG();
+    EXPECT_NO_THROW(f.value());
+  }
+
+  // Mutable lambda
+  {
+    auto f = makeFuture()
+      .then([] { throw eggs; })
+      .onError([&] (eggs_t& e) mutable { flag(); });
+    EXPECT_FLAG();
+    EXPECT_NO_THROW(f.value());
+  }
+
+  {
+    auto f = makeFuture()
+      .then([] { throw eggs; })
+      .onError([&] (eggs_t& e) mutable { flag(); return makeFuture(); });
+    EXPECT_FLAG();
+    EXPECT_NO_THROW(f.value());
+  }
+
+  // No throw
+  {
+    auto f = makeFuture()
+      .then([] { return 42; })
+      .onError([&] (eggs_t& e) { flag(); return -1; });
+    EXPECT_NO_FLAG();
+    EXPECT_EQ(42, f.value());
+  }
+
+  {
+    auto f = makeFuture()
+      .then([] { return 42; })
+      .onError([&] (eggs_t& e) { flag(); return makeFuture<int>(-1); });
+    EXPECT_NO_FLAG();
+    EXPECT_EQ(42, f.value());
+  }
+
+  // Catch different exception
+  {
+    auto f = makeFuture()
+      .then([] { throw eggs; })
+      .onError([&] (std::runtime_error& e) { flag(); });
+    EXPECT_NO_FLAG();
+    EXPECT_THROW(f.value(), eggs_t);
+  }
+
+  {
+    auto f = makeFuture()
+      .then([] { throw eggs; })
+      .onError([&] (std::runtime_error& e) { flag(); return makeFuture(); });
+    EXPECT_NO_FLAG();
+    EXPECT_THROW(f.value(), eggs_t);
+  }
+
+  // Returned value propagates
+  {
+    auto f = makeFuture()
+      .then([] { throw eggs; return 0; })
+      .onError([&] (eggs_t& e) { return 42; });
+    EXPECT_EQ(42, f.value());
+  }
+
+  // Returned future propagates
+  {
+    auto f = makeFuture()
+      .then([] { throw eggs; return 0; })
+      .onError([&] (eggs_t& e) { return makeFuture<int>(42); });
+    EXPECT_EQ(42, f.value());
+  }
+
+  // Throw in callback
+  {
+    auto f = makeFuture()
+      .then([] { throw eggs; return 0; })
+      .onError([&] (eggs_t& e) { throw e; return -1; });
+    EXPECT_THROW(f.value(), eggs_t);
+  }
+
+  {
+    auto f = makeFuture()
+      .then([] { throw eggs; return 0; })
+      .onError([&] (eggs_t& e) { throw e; return makeFuture<int>(-1); });
+    EXPECT_THROW(f.value(), eggs_t);
+  }
+}
+
 TEST(Future, try) {
   class A {
    public: