Propagate Load Errors through JSExecutor API

Reviewed By: mhorowitz Differential Revision: D4315204 fbshipit-source-id: ae282e0a0398a602dd790de3a2b1adb5fdc1f50c
neroti9n · Dec 17, 2016 · 6ca6b49 · 6ca6b49
1 parent 9f3ef48
commit 6ca6b49
Show file tree

Hide file tree

Showing 5 changed files with 131 additions and 18 deletions.
diff --git a/ReactCommon/cxxreact/BUCK b/ReactCommon/cxxreact/BUCK
@@ -126,6 +126,7 @@ CXXREACT_PUBLIC_HEADERS = [
   'NativeModule.h',
   'NativeToJsBridge.h',
   'Platform.h',
+  'RecoverableError.h',
   'SystraceSection.h',
 ]
 

diff --git a/ReactCommon/cxxreact/JSCExecutor.cpp b/ReactCommon/cxxreact/JSCExecutor.cpp
@@ -15,6 +15,7 @@
 #include <folly/Conv.h>
 #include <fcntl.h>
 #include <sys/time.h>
+#include <system_error>
 
 #include <jschelpers/JSCHelpers.h>
 #include <jschelpers/Value.h>
@@ -30,6 +31,7 @@
 #include "JSCUtils.h"
 #include "JSModulesUnbundle.h"
 #include "ModuleRegistry.h"
+#include "RecoverableError.h"
 
 #if defined(WITH_JSC_EXTRA_TRACING) || DEBUG
 #include "JSCTracing.h"
@@ -60,6 +62,8 @@
 namespace facebook {
 namespace react {
 
+using namespace detail;
+
 namespace {
 
 template<JSValueRef (JSCExecutor::*method)(size_t, const JSValueRef[])>
@@ -310,6 +314,30 @@ void JSCExecutor::terminateOnJSVMThread() {
   m_context = nullptr;
 }
 
+#ifdef WITH_FBJSCEXTENSIONS
+static const char* explainLoadSourceError(JSLoadSourceError err) {
+  switch (err) {
+  case JSLoadSourceErrorNone:
+    return "No error encountered during source load";
+
+  case JSLoadSourceErrorOnRead:
+    return "Error reading source";
+
+  case JSLoadSourceErrorNotCompiled:
+    return "Source is not compiled";
+
+  case JSLoadSourceErrorVersionMismatch:
+    return "Source version not supported";
+
+  case JSLoadSourceErrorUnknown:
+    return "Unknown error occurred when loading source";
+
+  default:
+    return "Bad error code";
+  }
+}
+#endif
+
 #ifdef WITH_FBJSCEXTENSIONS
 void JSCExecutor::loadApplicationScript(
     std::string bundlePath,
@@ -318,9 +346,9 @@ void JSCExecutor::loadApplicationScript(
   SystraceSection s("JSCExecutor::loadApplicationScript",
                     "sourceURL", sourceURL);
 
-  folly::throwOnFail<std::runtime_error>(
-    (flags & UNPACKED_JS_SOURCE) || (flags & UNPACKED_BYTECODE),
-    "Optimized bundle with no unpacked source or bytecode");
+  if (!(flags & (UNPACKED_JS_SOURCE | UNPACKED_BYTECODE))) {
+    throw RecoverableError("Optimized bundle with no unpacked source or bytecode");
+  }
 
   String jsSourceURL(m_context, sourceURL.c_str());
   JSSourceCodeRef sourceCode = nullptr;
@@ -332,14 +360,17 @@ void JSCExecutor::loadApplicationScript(
 
   if (flags & UNPACKED_BYTECODE) {
     int fd = open((bundlePath + UNPACKED_BYTECODE_SUFFIX).c_str(), O_RDONLY);
-    folly::checkUnixError(fd, "Couldn't open compiled bundle");
+    RecoverableError::runRethrowingAsRecoverable<std::system_error>([fd]() {
+        folly::checkUnixError(fd, "Couldn't open compiled bundle");
+      });
     SCOPE_EXIT { close(fd); };
-    sourceCode = JSCreateCompiledSourceCode(fd, jsSourceURL, nullptr);
 
-    folly::throwOnFail<std::runtime_error>(
-      sourceCode != nullptr,
-      "Could not create compiled source code"
-    );
+    JSLoadSourceError jsError;
+    sourceCode = JSCreateCompiledSourceCode(fd, jsSourceURL, &jsError);
+
+    if (!sourceCode) {
+      throw RecoverableError(explainLoadSourceError(jsError));
+    }
   } else {
     auto jsScriptBigString = JSBigOptimizedBundleString::fromOptimizedBundle(bundlePath);
     if (!jsScriptBigString->isAscii()) {
@@ -382,18 +413,15 @@ void JSCExecutor::loadApplicationScript(
     // Not bytecode, fall through.
     return JSExecutor::loadApplicationScript(fd, sourceURL);
 
-  case JSLoadSourceErrorVersionMismatch:
-    throw std::runtime_error("Compiled Source Version Mismatch");
-
   case JSLoadSourceErrorNone:
-    folly::throwOnFail<std::runtime_error>(
-      bcSourceCode != nullptr,
-      "Unexpected error opening compiled bundle"
-    );
+    if (!bcSourceCode) {
+      throw std::runtime_error("Unexpected error opening compiled bundle");
+    }
     break;
 
-  default:
-    throw std::runtime_error("Unhandled Compiled Source Error");
+  case JSLoadSourceErrorVersionMismatch:
+  case JSLoadSourceErrorUnknown:
+    throw RecoverableError(explainLoadSourceError(jsError));
   }
 
   ReactMarker::logMarker("RUN_JS_BUNDLE_START");

diff --git a/ReactCommon/cxxreact/RecoverableError.h b/ReactCommon/cxxreact/RecoverableError.h
@@ -0,0 +1,47 @@
+// Copyright 2004-present Facebook. All Rights Reserved.
+
+#pragma once
+
+#include <exception>
+#include <functional>
+#include <string>
+
+namespace facebook {
+namespace react {
+namespace detail {
+
+/**
+ * RecoverableError
+ *
+ * An exception that it is expected we should be able to recover from.
+ */
+struct RecoverableError : public std::exception {
+
+  explicit RecoverableError(const std::string &what_)
+    : m_what {what_}
+  {}
+
+  virtual const char* what() const throw() override { return m_what.c_str(); }
+
+  /**
+   * runRethrowingAsRecoverable
+   *
+   * Helper function that converts any exception of type `E`, thrown within the
+   * `act` routine into a recoverable error with the same message.
+   */
+  template <typename E>
+  inline static void runRethrowingAsRecoverable(std::function<void()> act) {
+    try {
+      act();
+    } catch(const E &err) {
+      throw RecoverableError(err.what());
+    }
+  }
+
+private:
+  std::string m_what;
+};
+
+} // namespace detail
+} // namespace react
+} // namespace facebook
diff --git a/ReactCommon/cxxreact/tests/BUCK b/ReactCommon/cxxreact/tests/BUCK
@@ -1,5 +1,6 @@
 TEST_SRCS = [
   'CxxMessageQueueTest.cpp',
+  'RecoverableErrorTest.cpp',
   'jsarg_helpers.cpp',
   'jsbigstring.cpp',
   'jscexecutor.cpp',

diff --git a/ReactCommon/cxxreact/tests/RecoverableErrorTest.cpp b/ReactCommon/cxxreact/tests/RecoverableErrorTest.cpp
@@ -0,0 +1,36 @@
+// Copyright 2004-present Facebook. All Rights Reserved.
+
+#include <gtest/gtest.h>
+
+#include <exception>
+#include <stdexcept>
+
+#include <cxxreact/RecoverableError.h>
+
+using namespace facebook::react::detail;
+
+TEST(RecoverableError, RunRethrowingAsRecoverableRecoverTest) {
+  try {
+    RecoverableError::runRethrowingAsRecoverable<std::runtime_error>([]() {
+        throw std::runtime_error("catch me");
+      });
+    FAIL() << "Unthrown exception";
+  } catch (const RecoverableError &err) {
+    ASSERT_STREQ(err.what(), "catch me");
+  } catch (...) {
+    FAIL() << "Uncaught exception";
+  }
+}
+
+TEST(RecoverableError, RunRethrowingAsRecoverableFallthroughTest) {
+  try {
+    RecoverableError::runRethrowingAsRecoverable<std::runtime_error>([]() {
+        throw std::logic_error("catch me");
+      });
+    FAIL() << "Unthrown exception";
+  } catch (const RecoverableError &err) {
+    FAIL() << "Recovered exception that should have fallen through";
+  } catch (const std::exception &err) {
+    ASSERT_STREQ(err.what(), "catch me");
+  }
+}