Catch JS bundle load failure and prevent calls to JS after that

Summary: There are cases where JS bundle fails to be evaluated, which throws an exception already, but then there were pending calls into JS which would fail in a weird way. This prevents those calls (because it's mostly meaningless at that point). For now, those extra calls will still throw an exception, but with a specific message so that it doesn't confuse people. Reviewed By: yungsters Differential Revision: D8961622 fbshipit-source-id: 3f67fb63fdfa9fc5b249de0096e893b07956776a
facebook · Jul 25, 2018 · 201ba8c · 201ba8c
1 parent c36e8b3
commit 201ba8c
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 7 deletions.
diff --git a/ReactCommon/cxxreact/NativeToJsBridge.cpp b/ReactCommon/cxxreact/NativeToJsBridge.cpp
@@ -8,6 +8,7 @@
 #include <folly/json.h>
 #include <folly/Memory.h>
 #include <folly/MoveWrapper.h>
+#include <glog/logging.h>
 
 #include "Instance.h"
 #include "JSBigString.h"
@@ -99,16 +100,22 @@ void NativeToJsBridge::loadApplication(
     std::unique_ptr<const JSBigString> startupScript,
     std::string startupScriptSourceURL) {
   runOnExecutorQueue(
-      [bundleRegistryWrap=folly::makeMoveWrapper(std::move(bundleRegistry)),
+      [this,
+       bundleRegistryWrap=folly::makeMoveWrapper(std::move(bundleRegistry)),
        startupScript=folly::makeMoveWrapper(std::move(startupScript)),
        startupScriptSourceURL=std::move(startupScriptSourceURL)]
         (JSExecutor* executor) mutable {
     auto bundleRegistry = bundleRegistryWrap.move();
     if (bundleRegistry) {
       executor->setBundleRegistry(std::move(bundleRegistry));
     }
-    executor->loadApplicationScript(std::move(*startupScript),
-                                    std::move(startupScriptSourceURL));
+    try {
+      executor->loadApplicationScript(std::move(*startupScript),
+                                      std::move(startupScriptSourceURL));
+    } catch (...) {
+      m_applicationScriptHasFailure = true;
+      throw;
+    }
   });
 }
 
@@ -119,8 +126,13 @@ void NativeToJsBridge::loadApplicationSync(
   if (bundleRegistry) {
     m_executor->setBundleRegistry(std::move(bundleRegistry));
   }
-  m_executor->loadApplicationScript(std::move(startupScript),
-                                        std::move(startupScriptSourceURL));
+  try {
+    m_executor->loadApplicationScript(std::move(startupScript),
+                                          std::move(startupScriptSourceURL));
+  } catch (...) {
+    m_applicationScriptHasFailure = true;
+    throw;
+  }
 }
 
 void NativeToJsBridge::callFunction(
@@ -136,8 +148,13 @@ void NativeToJsBridge::callFunction(
       systraceCookie);
   #endif
 
-  runOnExecutorQueue([module = std::move(module), method = std::move(method), arguments = std::move(arguments), systraceCookie]
+  runOnExecutorQueue([this, module = std::move(module), method = std::move(method), arguments = std::move(arguments), systraceCookie]
     (JSExecutor* executor) {
+      if (m_applicationScriptHasFailure) {
+        LOG(ERROR) << "Attempting to call JS function on a bad application bundle: " << module.c_str() << "." << method.c_str() << "()";
+        throw std::runtime_error("Attempting to call JS function on a bad application bundle: " + module + "." + method + "()");
+      }
+
       #ifdef WITH_FBSYSTRACE
       FbSystraceAsyncFlow::end(
           TRACE_TAG_REACT_CXX_BRIDGE,
@@ -162,8 +179,12 @@ void NativeToJsBridge::invokeCallback(double callbackId, folly::dynamic&& argume
       systraceCookie);
   #endif
 
-  runOnExecutorQueue([callbackId, arguments = std::move(arguments), systraceCookie]
+  runOnExecutorQueue([this, callbackId, arguments = std::move(arguments), systraceCookie]
     (JSExecutor* executor) {
+      if (m_applicationScriptHasFailure) {
+        LOG(ERROR) << "Attempting to call JS callback on a bad application bundle: " << callbackId;
+        throw std::runtime_error("Attempting to invoke JS callback on a bad application bundle.");
+      }
       #ifdef WITH_FBSYSTRACE
       FbSystraceAsyncFlow::end(
           TRACE_TAG_REACT_CXX_BRIDGE,

diff --git a/ReactCommon/cxxreact/NativeToJsBridge.h b/ReactCommon/cxxreact/NativeToJsBridge.h
@@ -94,6 +94,11 @@ class NativeToJsBridge {
   std::unique_ptr<JSExecutor> m_executor;
   std::shared_ptr<MessageQueueThread> m_executorMessageQueueThread;
 
+  // Keep track of whether the JS bundle containing the application logic causes
+  // exception when evaluated initially. If so, more calls to JS will very
+  // likely fail as well, so this flag can help prevent them.
+  bool m_applicationScriptHasFailure = false;
+
   #ifdef WITH_FBSYSTRACE
   std::atomic_uint_least32_t m_systraceCookie = ATOMIC_VAR_INIT();
   #endif