Commit cc381bd4 authored by Lee Howes's avatar Lee Howes Committed by Facebook Github Bot

Deprecate Future::onError

Summary:
onError is to be phased out because:
 * It is weakly typed
 * (most seriously) It loses the executor and so terminates a via chain silently causing subsequent work to run in the wrong place

onError is replaced with thenError which fixes both problems.

This diff merely deprecates it to allow for a gentle phase out.

Reviewed By: yfeldblum

Differential Revision: D13418290

fbshipit-source-id: a0c5e65a97ed41de18d85ceab258417355a43139
parent 32543b61
...@@ -1095,11 +1095,15 @@ Future<T> Future<T>::thenError(F&& func) && { ...@@ -1095,11 +1095,15 @@ Future<T> Future<T>::thenError(F&& func) && {
// possible. // possible.
auto* ePtr = this->getExecutor(); auto* ePtr = this->getExecutor();
auto e = folly::getKeepAliveToken(ePtr ? *ePtr : InlineExecutor::instance()); auto e = folly::getKeepAliveToken(ePtr ? *ePtr : InlineExecutor::instance());
FOLLY_PUSH_WARNING
FOLLY_GNU_DISABLE_WARNING("-Wdeprecated-declarations")
return std::move(*this) return std::move(*this)
.onError([func = std::forward<F>(func)](ExceptionType& ex) mutable { .onError([func = std::forward<F>(func)](ExceptionType& ex) mutable {
return std::forward<F>(func)(ex); return std::forward<F>(func)(ex);
}) })
.via(std::move(e)); .via(std::move(e));
FOLLY_POP_WARNING
} }
template <class T> template <class T>
...@@ -1110,12 +1114,16 @@ Future<T> Future<T>::thenError(F&& func) && { ...@@ -1110,12 +1114,16 @@ Future<T> Future<T>::thenError(F&& func) && {
// possible. // possible.
auto* ePtr = this->getExecutor(); auto* ePtr = this->getExecutor();
auto e = folly::getKeepAliveToken(ePtr ? *ePtr : InlineExecutor::instance()); auto e = folly::getKeepAliveToken(ePtr ? *ePtr : InlineExecutor::instance());
FOLLY_PUSH_WARNING
FOLLY_GNU_DISABLE_WARNING("-Wdeprecated-declarations")
return std::move(*this) return std::move(*this)
.onError([func = std::forward<F>(func)]( .onError([func = std::forward<F>(func)](
folly::exception_wrapper&& ex) mutable { folly::exception_wrapper&& ex) mutable {
return std::forward<F>(func)(std::move(ex)); return std::forward<F>(func)(std::move(ex));
}) })
.via(std::move(e)); .via(std::move(e));
FOLLY_POP_WARNING
} }
template <class T> template <class T>
......
...@@ -1457,6 +1457,8 @@ class Future : private futures::detail::FutureBase<T> { ...@@ -1457,6 +1457,8 @@ class Future : private futures::detail::FutureBase<T> {
/// i.e., as if `*this` was moved into RESULT. /// i.e., as if `*this` was moved into RESULT.
/// - `RESULT.valid() == true` /// - `RESULT.valid() == true`
template <class F> template <class F>
[[deprecated(
"onError loses the attached executor and is weakly typed. Please move to thenError instead.")]]
typename std::enable_if< typename std::enable_if<
!is_invocable<F, exception_wrapper>::value && !is_invocable<F, exception_wrapper>::value &&
!futures::detail::Extract<F>::ReturnsFuture::value, !futures::detail::Extract<F>::ReturnsFuture::value,
...@@ -1475,6 +1477,8 @@ class Future : private futures::detail::FutureBase<T> { ...@@ -1475,6 +1477,8 @@ class Future : private futures::detail::FutureBase<T> {
/// i.e., as if `*this` was moved into RESULT. /// i.e., as if `*this` was moved into RESULT.
/// - `RESULT.valid() == true` /// - `RESULT.valid() == true`
template <class F> template <class F>
[[deprecated(
"onError loses the attached executor and is weakly typed. Please move to thenError instead.")]]
typename std::enable_if< typename std::enable_if<
!is_invocable<F, exception_wrapper>::value && !is_invocable<F, exception_wrapper>::value &&
futures::detail::Extract<F>::ReturnsFuture::value, futures::detail::Extract<F>::ReturnsFuture::value,
...@@ -1493,6 +1497,8 @@ class Future : private futures::detail::FutureBase<T> { ...@@ -1493,6 +1497,8 @@ class Future : private futures::detail::FutureBase<T> {
/// i.e., as if `*this` was moved into RESULT. /// i.e., as if `*this` was moved into RESULT.
/// - `RESULT.valid() == true` /// - `RESULT.valid() == true`
template <class F> template <class F>
[[deprecated(
"onError loses the attached executor and is weakly typed. Please move to thenError instead.")]]
typename std::enable_if< typename std::enable_if<
is_invocable<F, exception_wrapper>::value && is_invocable<F, exception_wrapper>::value &&
futures::detail::Extract<F>::ReturnsFuture::value, futures::detail::Extract<F>::ReturnsFuture::value,
...@@ -1511,6 +1517,8 @@ class Future : private futures::detail::FutureBase<T> { ...@@ -1511,6 +1517,8 @@ class Future : private futures::detail::FutureBase<T> {
/// i.e., as if `*this` was moved into RESULT. /// i.e., as if `*this` was moved into RESULT.
/// - `RESULT.valid() == true` /// - `RESULT.valid() == true`
template <class F> template <class F>
[[deprecated(
"onError loses the attached executor and is weakly typed. Please move to thenError instead.")]]
typename std::enable_if< typename std::enable_if<
is_invocable<F, exception_wrapper>::value && is_invocable<F, exception_wrapper>::value &&
!futures::detail::Extract<F>::ReturnsFuture::value, !futures::detail::Extract<F>::ReturnsFuture::value,
...@@ -1519,7 +1527,7 @@ class Future : private futures::detail::FutureBase<T> { ...@@ -1519,7 +1527,7 @@ class Future : private futures::detail::FutureBase<T> {
// clang-format off // clang-format off
template <class F> template <class F>
[[deprecated("use rvalue-qualified fn, eg, std::move(future).onError(...)")]] [[deprecated("ERROR: use rvalue-qualified fn, eg, std::move(future).onError(...)")]]
Future<T> onError(F&& func) & = delete; Future<T> onError(F&& func) & = delete;
/// func is like std::function<void()> and is executed unconditionally, and /// func is like std::function<void()> and is executed unconditionally, and
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment