Future::value() should throw when unset

Summary:
Added a test to call Future::value() before the Promise value is set, expecting an exception.
In a dbg build the test failed due on the assertion in Optional::value().
In a opt build the test failed due as no exception was thrown.
There are 2 points where we could throw our exception:
a) Optional::value() - replacing the assertion
b) Future::value()

I'm not sure which location makes the most sense.
With the assertion in Optional it seems that adding the throw here would not be unexpected but this is outside the wangle code.
So as a first pass I've added the throw in Future::value(), and made a new WangleException for this.

Test Plan:
$ fbconfig folly/wangle
$ fbmake runtests

Reviewed By: hans@fb.com

Subscribers: folly@lists, fugalh

FB internal diff: D1340886

diff --git a/folly/wangle/WangleException.h b/folly/wangle/WangleException.h
index b1fd737a8f8e0bfd16210ee44e9fa734523af8c8..aec2297057db362b927edca92853ceeab81be7b6 100644 (file)
--- a/folly/wangle/WangleException.h
+++ b/folly/wangle/WangleException.h
@@ -63,6 +63,12 @@ class PromiseAlreadySatisfied : public WangleException {
       WangleException("Promise already satisfied") { }
 };
 
+class FutureNotReady : public WangleException {
+  public:
+    explicit FutureNotReady() :
+      WangleException("Future not ready") { }
+};
+
 class FutureAlreadyRetrieved : public WangleException {
   public:
     explicit FutureAlreadyRetrieved () :

diff --git a/folly/wangle/detail.h b/folly/wangle/detail.h
index dda83ad40b74fcc565db3c4cbce1d31e2e22dc09..8a89c64139a8b02bcc15393ebfad96ae2bf35d19 100644 (file)
--- a/folly/wangle/detail.h
+++ b/folly/wangle/detail.h
@@ -38,7 +38,7 @@ class FutureObject {
   FutureObject& operator=(FutureObject const&) = delete;
 
   // not movable (see comment in the implementation of Future::then)
-  FutureObject(FutureObject&&) = delete;
+  FutureObject(FutureObject&&) noexcept = delete;
   FutureObject& operator=(FutureObject&&) = delete;
 
   Try<T>& getTry() {
@@ -85,7 +85,11 @@ class FutureObject {
   }
 
   typename std::add_lvalue_reference<T>::type value() {
-    return value_->value();
+    if (ready()) {
+      return value_->value();
+    } else {
+      throw FutureNotReady();
+    }
   }
 
  private:

diff --git a/folly/wangle/test/FutureTest.cpp b/folly/wangle/test/FutureTest.cpp
index 0202470cef045b35a7d23150ba30ed6fa5b504c9..a7a2fa1c92cda0285053234ff82f5dae5ace4efa 100644 (file)
--- a/folly/wangle/test/FutureTest.cpp
+++ b/folly/wangle/test/FutureTest.cpp
@@ -153,6 +153,12 @@ TEST(Future, isReady) {
   EXPECT_TRUE(f.isReady());
   }
 
+TEST(Future, futureNotReady) {
+  Promise<int> p;
+  Future<int> f = p.getFuture();
+  EXPECT_THROW(f.value(), eggs_t);
+}
+
 TEST(Future, hasException) {
   EXPECT_TRUE(makeFuture<int>(eggs).getTry().hasException());
   EXPECT_FALSE(makeFuture(42).getTry().hasException());