Assume exception when Executor::add throws

Summary:
Rather than crashing spectacularly, if `Executor::add` throws (e.g. because the queue is full), then discard the result we got and assume the exception the executor threw instead.

Alternatively, we could pass this exceptional Try to the callback (without an executor, as it is here), but not perturb `result_`. This would mean two different world views in these two code snippets:

auto f1 = makeFuture(42).via(&crappyExecutor);
f1.value(); // 42 (no callback happened)
f1.then(...); // would see the executor's exception. Would also be ill-advised to do this after value()

auto f2 = makeFuture(42).via(&crappyExecutor)
.then([](int x) { return x * 2; }); // skipped
f2.value(); // throws executor's exception

It feels rude to throw away the result, but it feels too potentially dangerous to allow this split view of the world.

Test Plan: modified unit

Reviewed By: jsedgwick@fb.com

Subscribers: exa, folly-diffs@, jsedgwick, yfeldblum, chalfant

FB internal diff: D2007729

Tasks: 5306911

Signature: t1:2007729:1429627114:b627ce758ce9231298f1b28e203ccc1ee415ed9a

folly/futures/detail/Core.h

@@ -278,18 +278,21 @@ class Core {
   }
 
   void doCallback() {
-    // TODO(5306911) we should probably try/catch around the callback
-
     RequestContext::setContext(context_);
 
     // TODO(6115514) semantic race on reading executor_ and setExecutor()
     Executor* x = executor_;
     if (x) {
       ++attached_; // keep Core alive until executor did its thing
-      x->add([this]() mutable {
-        SCOPE_EXIT { detachOne(); };
+      try {
+        x->add([this]() mutable {
+          SCOPE_EXIT { detachOne(); };
+          callback_(std::move(*result_));
+        });
+      } catch (...) {
+        result_ = Try<T>(exception_wrapper(std::current_exception()));
         callback_(std::move(*result_));
-      });
+      }
     } else {
       callback_(std::move(*result_));
     }

folly/futures/test/ExecutorTest.cpp

@@ -178,14 +178,10 @@ class CrappyExecutor : public Executor {
 
 TEST(Executor, CrappyExecutor) {
   CrappyExecutor x;
-  try {
-    auto f = Future<void>().via(&x).then([](){
-      return;
-    });
-    f.value();
-    EXPECT_TRUE(false);
-  } catch(...) {
-    // via() should throw
-    return;
-  }
+  bool flag = false;
+  auto f = folly::via(&x).onError([&](std::runtime_error& e) {
+    EXPECT_STREQ("bad", e.what());
+    flag = true;
+  });
+  EXPECT_TRUE(flag);
 }