Skip to content

Commit 201ba8c

Browse files
fkgozalifacebook-github-bot
authored andcommittedJul 25, 2018
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
1 parent c36e8b3 commit 201ba8c

File tree

2 files changed

+33
-7
lines changed

2 files changed

+33
-7
lines changed
 

‎ReactCommon/cxxreact/NativeToJsBridge.cpp

+28-7
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include <folly/json.h>
99
#include <folly/Memory.h>
1010
#include <folly/MoveWrapper.h>
11+
#include <glog/logging.h>
1112

1213
#include "Instance.h"
1314
#include "JSBigString.h"
@@ -99,16 +100,22 @@ void NativeToJsBridge::loadApplication(
99100
std::unique_ptr<const JSBigString> startupScript,
100101
std::string startupScriptSourceURL) {
101102
runOnExecutorQueue(
102-
[bundleRegistryWrap=folly::makeMoveWrapper(std::move(bundleRegistry)),
103+
[this,
104+
bundleRegistryWrap=folly::makeMoveWrapper(std::move(bundleRegistry)),
103105
startupScript=folly::makeMoveWrapper(std::move(startupScript)),
104106
startupScriptSourceURL=std::move(startupScriptSourceURL)]
105107
(JSExecutor* executor) mutable {
106108
auto bundleRegistry = bundleRegistryWrap.move();
107109
if (bundleRegistry) {
108110
executor->setBundleRegistry(std::move(bundleRegistry));
109111
}
110-
executor->loadApplicationScript(std::move(*startupScript),
111-
std::move(startupScriptSourceURL));
112+
try {
113+
executor->loadApplicationScript(std::move(*startupScript),
114+
std::move(startupScriptSourceURL));
115+
} catch (...) {
116+
m_applicationScriptHasFailure = true;
117+
throw;
118+
}
112119
});
113120
}
114121

@@ -119,8 +126,13 @@ void NativeToJsBridge::loadApplicationSync(
119126
if (bundleRegistry) {
120127
m_executor->setBundleRegistry(std::move(bundleRegistry));
121128
}
122-
m_executor->loadApplicationScript(std::move(startupScript),
123-
std::move(startupScriptSourceURL));
129+
try {
130+
m_executor->loadApplicationScript(std::move(startupScript),
131+
std::move(startupScriptSourceURL));
132+
} catch (...) {
133+
m_applicationScriptHasFailure = true;
134+
throw;
135+
}
124136
}
125137

126138
void NativeToJsBridge::callFunction(
@@ -136,8 +148,13 @@ void NativeToJsBridge::callFunction(
136148
systraceCookie);
137149
#endif
138150

139-
runOnExecutorQueue([module = std::move(module), method = std::move(method), arguments = std::move(arguments), systraceCookie]
151+
runOnExecutorQueue([this, module = std::move(module), method = std::move(method), arguments = std::move(arguments), systraceCookie]
140152
(JSExecutor* executor) {
153+
if (m_applicationScriptHasFailure) {
154+
LOG(ERROR) << "Attempting to call JS function on a bad application bundle: " << module.c_str() << "." << method.c_str() << "()";
155+
throw std::runtime_error("Attempting to call JS function on a bad application bundle: " + module + "." + method + "()");
156+
}
157+
141158
#ifdef WITH_FBSYSTRACE
142159
FbSystraceAsyncFlow::end(
143160
TRACE_TAG_REACT_CXX_BRIDGE,
@@ -162,8 +179,12 @@ void NativeToJsBridge::invokeCallback(double callbackId, folly::dynamic&& argume
162179
systraceCookie);
163180
#endif
164181

165-
runOnExecutorQueue([callbackId, arguments = std::move(arguments), systraceCookie]
182+
runOnExecutorQueue([this, callbackId, arguments = std::move(arguments), systraceCookie]
166183
(JSExecutor* executor) {
184+
if (m_applicationScriptHasFailure) {
185+
LOG(ERROR) << "Attempting to call JS callback on a bad application bundle: " << callbackId;
186+
throw std::runtime_error("Attempting to invoke JS callback on a bad application bundle.");
187+
}
167188
#ifdef WITH_FBSYSTRACE
168189
FbSystraceAsyncFlow::end(
169190
TRACE_TAG_REACT_CXX_BRIDGE,

‎ReactCommon/cxxreact/NativeToJsBridge.h

+5
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,11 @@ class NativeToJsBridge {
9494
std::unique_ptr<JSExecutor> m_executor;
9595
std::shared_ptr<MessageQueueThread> m_executorMessageQueueThread;
9696

97+
// Keep track of whether the JS bundle containing the application logic causes
98+
// exception when evaluated initially. If so, more calls to JS will very
99+
// likely fail as well, so this flag can help prevent them.
100+
bool m_applicationScriptHasFailure = false;
101+
97102
#ifdef WITH_FBSYSTRACE
98103
std::atomic_uint_least32_t m_systraceCookie = ATOMIC_VAR_INIT();
99104
#endif

0 commit comments

Comments
 (0)
Please sign in to comment.