diff --git a/folly/futures/detail/Core.h b/folly/futures/detail/Core.h index 037ad13..f9d1867 100644 --- a/folly/futures/detail/Core.h +++ b/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(exception_wrapper(std::current_exception())); callback_(std::move(*result_)); - }); + } } else { callback_(std::move(*result_)); } diff --git a/folly/futures/test/ExecutorTest.cpp b/folly/futures/test/ExecutorTest.cpp index 832e135..e477f5c 100644 --- a/folly/futures/test/ExecutorTest.cpp +++ b/folly/futures/test/ExecutorTest.cpp @@ -178,14 +178,10 @@ class CrappyExecutor : public Executor { TEST(Executor, CrappyExecutor) { CrappyExecutor x; - try { - auto f = Future().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); }