Catch exceptions thrown by user callbacks

Summary:
Calls to Inspector::evaluate() and Inspector::executeIfEnabled()
take a user-provided callback which are not try-guarded. This
means that, should the user code throw, the inspector's process
will likely die due to the unhandled exception. This change
makes sure that, if the user provided callback throws, then
the promise attached to those methods will be fulfilled by
an exception.

Change: [internal]

Reviewed By: avp

Differential Revision: D33852073

fbshipit-source-id: 7fbb6662b28d393a5d5b494c004aa9521e23ebb6

ReactCommon/hermes/inspector/Exceptions.h

@@ -47,6 +47,12 @@ class MultipleCommandsPendingException : public std::runtime_error {
             ": a step or resume is already pending") {}
 };
 
+class UserCallbackException : public std::runtime_error {
+ public:
+  UserCallbackException(const std::exception &e)
+      : std::runtime_error(std::string("callback exception: ") + e.what()) {}
+};
+
 } // namespace inspector
 } // namespace hermes
 } // namespace facebook

ReactCommon/hermes/inspector/Inspector.cpp

@@ -589,8 +589,11 @@ void Inspector::executeIfEnabledOnExecutor(
 
   state_->pushPendingFunc(
       [wrappedFunc = std::move(wrappedFunc), promise]() mutable {
-        wrappedFunc();
-        promise->setValue();
+        if (auto userCallbackException = runUserCallback(wrappedFunc)) {
+          promise->setException(*userCallbackException);
+        } else {
+          promise->setValue();
+        }
       });
 }
 

ReactCommon/hermes/inspector/Inspector.h

@@ -15,11 +15,13 @@
 #include <unordered_map>
 
 #include <folly/Executor.h>
+#include <folly/Optional.h>
 #include <folly/Unit.h>
 #include <folly/futures/Future.h>
 #include <hermes/DebuggerAPI.h>
 #include <hermes/hermes.h>
 #include <hermes/inspector/AsyncPauseState.h>
+#include <hermes/inspector/Exceptions.h>
 #include <hermes/inspector/RuntimeAdapter.h>
 
 namespace facebook {
@@ -363,6 +365,18 @@ class Inspector : public facebook::hermes::debugger::EventObserver,
   std::unique_ptr<folly::Executor> executor_;
 };
 
+/// Helper function that guards user code execution in a try-catch block.
+template <typename C, typename... A>
+folly::Optional<UserCallbackException> runUserCallback(C &cb, A &&...arg) {
+  try {
+    cb(std::forward<A>(arg)...);
+  } catch (const std::exception &e) {
+    return UserCallbackException(e);
+  }
+
+  return {};
+}
+
 } // namespace inspector
 } // namespace hermes
 } // namespace facebook

ReactCommon/hermes/inspector/InspectorState.cpp

@@ -249,10 +249,14 @@ std::pair<NextStatePtr, CommandPtr> InspectorState::Running::didPause(
   } else if (reason == debugger::PauseReason::EvalComplete) {
     assert(pendingEvalPromise_);
 
-    pendingEvalResultTransformer_(
-        inspector_.debugger_.getProgramState().getEvalResult());
-    pendingEvalPromise_->setValue(
-        inspector_.debugger_.getProgramState().getEvalResult());
+    if (auto userCallbackException = runUserCallback(
+            pendingEvalResultTransformer_,
+            inspector_.debugger_.getProgramState().getEvalResult())) {
+      pendingEvalPromise_->setException(*userCallbackException);
+    } else {
+      pendingEvalPromise_->setValue(
+          inspector_.debugger_.getProgramState().getEvalResult());
+    }
     pendingEvalPromise_.reset();
   } else if (
       reason == debugger::PauseReason::Breakpoint &&
@@ -364,10 +368,14 @@ std::pair<NextStatePtr, CommandPtr> InspectorState::Paused::didPause(
       break;
     case debugger::PauseReason::EvalComplete: {
       assert(pendingEvalPromise_);
-      pendingEvalResultTransformer_(
-          inspector_.debugger_.getProgramState().getEvalResult());
-      pendingEvalPromise_->setValue(
-          inspector_.debugger_.getProgramState().getEvalResult());
+      if (auto userCallbackException = runUserCallback(
+              pendingEvalResultTransformer_,
+              inspector_.debugger_.getProgramState().getEvalResult())) {
+        pendingEvalPromise_->setException(*userCallbackException);
+      } else {
+        pendingEvalPromise_->setValue(
+            inspector_.debugger_.getProgramState().getEvalResult());
+      }
       pendingEvalPromise_.reset();
     } break;
     case debugger::PauseReason::ScriptLoaded: