Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/fix unhandled exception #1137

Merged
merged 15 commits into from
Jan 29, 2022
2 changes: 2 additions & 0 deletions bridge/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,8 @@ if ($ENV{KRAKEN_JS_ENGINE} MATCHES "quickjs")
bindings/qjs/host_class.h
bindings/qjs/qjs_patch.cc
bindings/qjs/qjs_patch.h
bindings/qjs/rejected_promises.cc
bindings/qjs/rejected_promises.h
bindings/qjs/module_manager.cc
bindings/qjs/module_manager.h
bindings/qjs/html_parser.cc
Expand Down
5 changes: 2 additions & 3 deletions bridge/bindings/qjs/dom/element.cc
Original file line number Diff line number Diff line change
Expand Up @@ -330,9 +330,8 @@ JSValue Element::toBlob(JSContext* ctx, JSValue this_val, int argc, JSValue* arg
JS_FreeValue(ctx, argumentsArray);
JS_FreeValue(ctx, arrayBuffer);
} else {
JSValue errorObject = JS_NewError(ctx);
JSValue errorMessage = JS_NewString(ctx, error);
JS_SetPropertyStr(ctx, errorObject, "message", errorMessage);
JS_ThrowInternalError(ctx, "%s", error);
JSValue errorObject = JS_GetException(ctx);
JSValue ret = JS_Call(ctx, promiseContext->rejectFunc, promiseContext->promise, 1, &errorObject);
promiseContext->context->handleException(&ret);
promiseContext->context->drainPendingPromiseJobs();
Expand Down
25 changes: 23 additions & 2 deletions bridge/bindings/qjs/executing_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,9 @@ void ExecutionContext::drainPendingPromiseJobs() {
break;
}
}

// Throw error when promise are not handled.
m_rejectedPromise.process(this);
}

void ExecutionContext::defineGlobalProperty(const char* prop, JSValue value) {
Expand Down Expand Up @@ -311,7 +314,7 @@ void ExecutionContext::dispatchGlobalErrorEvent(JSValueConst error) {
JS_FreeValue(m_ctx, errorHandler);
}

void ExecutionContext::dispatchGlobalPromiseRejectionEvent(JSValueConst promise, JSValueConst error) {
void ExecutionContext::dispatchGlobalUnhandledRejectionEvent(JSValueConst promise, JSValueConst error) {
JSValue errorHandler = JS_GetPropertyStr(m_ctx, globalObject, "__global_unhandled_promise_handler__");
JSValue arguments[] = {promise, error};
JSValue returnValue = JS_Call(m_ctx, errorHandler, globalObject, 2, arguments);
Expand All @@ -321,10 +324,28 @@ void ExecutionContext::dispatchGlobalPromiseRejectionEvent(JSValueConst promise,
JS_FreeValue(m_ctx, errorHandler);
}

void ExecutionContext::dispatchGlobalRejectionHandledEvent(JSValue promise, JSValue error) {
JSValue errorHandler = JS_GetPropertyStr(m_ctx, globalObject, "__global_rejection_handled_handler__");
JSValue arguments[] = {promise, error};
JSValue returnValue = JS_Call(m_ctx, errorHandler, globalObject, 2, arguments);
drainPendingPromiseJobs();
handleException(&returnValue);
JS_FreeValue(m_ctx, returnValue);
JS_FreeValue(m_ctx, errorHandler);
}

void ExecutionContext::promiseRejectTracker(JSContext* ctx, JSValue promise, JSValue reason, int is_handled, void* opaque) {
auto* context = static_cast<ExecutionContext*>(JS_GetContextOpaque(ctx));
context->reportError(reason);
context->dispatchGlobalPromiseRejectionEvent(promise, reason);

// The unhandledrejection event is the promise-equivalent of the global error event, which is fired for uncaught exceptions.
// Because a rejected promise could be handled after the fact, by attaching catch(onRejected) or then(onFulfilled, onRejected) to it,
// the additional rejectionhandled event is needed to indicate that a promise which was previously rejected should no longer be considered unhandled.
if (is_handled) {
context->m_rejectedPromise.trackHandledPromiseRejection(context, promise, reason);
} else {
context->m_rejectedPromise.trackUnhandledPromiseRejection(context, promise, reason);
}
}

DOMTimerCoordinator* ExecutionContext::timers() {
Expand Down
6 changes: 5 additions & 1 deletion bridge/bindings/qjs/executing_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "js_context_macros.h"
#include "kraken_foundation.h"
#include "qjs_patch.h"
#include "rejected_promises.h"

using JSExceptionHandler = std::function<void(int32_t contextId, const char* message)>;

Expand Down Expand Up @@ -113,10 +114,12 @@ class ExecutionContext {
static JSClassID kHostObjectClassId;
static JSClassID kHostExoticObjectClassId;

void dispatchGlobalUnhandledRejectionEvent(JSValueConst promise, JSValueConst error);
void dispatchGlobalRejectionHandledEvent(JSValueConst promise, JSValueConst error);

private:
static void promiseRejectTracker(JSContext* ctx, JSValueConst promise, JSValueConst reason, JS_BOOL is_handled, void* opaque);
void dispatchGlobalErrorEvent(JSValueConst error);
void dispatchGlobalPromiseRejectionEvent(JSValueConst promise, JSValueConst error);
void reportError(JSValueConst error);

int32_t contextId;
Expand All @@ -132,6 +135,7 @@ class ExecutionContext {
DOMTimerCoordinator m_timers;
ExecutionContextGCTracker* m_gcTracker{nullptr};
foundation::UICommandBuffer m_commandBuffer{contextId};
RejectedPromises m_rejectedPromise;
};

// The read object's method or properties via Proxy, we should redirect this_val from Proxy into target property of
Expand Down
148 changes: 148 additions & 0 deletions bridge/bindings/qjs/js_context_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,154 @@ TEST(Context, unrejectPromiseError) {
EXPECT_EQ(errorHandlerExecuted, true);
}

TEST(Context, unrejectPromiseWillTriggerUnhandledRejectionEvent) {
static bool errorHandlerExecuted = false;
static bool logCalled = false;
auto errorHandler = [](int32_t contextId, const char* errmsg) {
errorHandlerExecuted = true;
EXPECT_STREQ(errmsg,
"TypeError: cannot read property 'forceNullError' of null\n"
" at <anonymous> (file://:12)\n"
" at Promise (native)\n"
" at <eval> (file://:14)\n");
};
auto bridge = TEST_init(errorHandler);
static int logIndex = 0;
static std::string logs[] = {"Unhandled Promise Rejection: TypeError: cannot read property 'forceNullError' of null", "unhandled event {promise: Promise {...}, reason: Error {...}}",
"error event cannot read property 'forceNullError' of null"};
kraken::KrakenPage::consoleMessageHandler = [](void* ctx, const std::string& message, int logLevel) {
logCalled = true;
EXPECT_STREQ(logs[logIndex++].c_str(), message.c_str());
};

std::string code = R"(
window.onunhandledrejection = (e) => {
console.log('unhandled event', e);
};
window.onerror = (e) => {
console.log('error event', e);
}

var p = new Promise(function (resolve, reject) {
var nullObject = null;
// Raise a TypeError: Cannot read property 'forceNullError' of null
var x = nullObject.forceNullError();
resolve();
});
)";
bridge->evaluateScript(code.c_str(), code.size(), "file://", 0);
EXPECT_EQ(errorHandlerExecuted, true);
EXPECT_EQ(logCalled, true);
EXPECT_EQ(logIndex, 3);
kraken::KrakenPage::consoleMessageHandler = nullptr;
}

TEST(Context, handledRejectionWillNotTriggerUnHandledRejectionEvent) {
static bool errorHandlerExecuted = false;
static bool logCalled = false;
auto errorHandler = [](int32_t contextId, const char* errmsg) { errorHandlerExecuted = true; };
auto bridge = TEST_init(errorHandler);
kraken::KrakenPage::consoleMessageHandler = [](void* ctx, const std::string& message, int logLevel) {
logCalled = true;
EXPECT_STREQ(message.c_str(), "rejected");
};

std::string code = R"(
window.addEventListener('unhandledrejection', event => {
console.log('unhandledrejection fired: ' + event.reason);
});

window.addEventListener('rejectionhandled', event => {
console.log('rejectionhandled fired: ' + event.reason);
});

function generateRejectedPromise(isEventuallyHandled) {
// Create a promise which immediately rejects with a given reason.
var rejectedPromise = Promise.reject('Error at ' +
new Date().toLocaleTimeString());
rejectedPromise.catch(() => {
console.log('rejected');
});
}

generateRejectedPromise(true);
)";
bridge->evaluateScript(code.c_str(), code.size(), "file://", 0);
EXPECT_EQ(errorHandlerExecuted, false);
EXPECT_EQ(logCalled, true);
kraken::KrakenPage::consoleMessageHandler = nullptr;
}

TEST(Context, unhandledRejectionEventWillTriggerWhenNotHandled) {
static bool errorHandlerExecuted = false;
static bool logCalled = false;
auto errorHandler = [](int32_t contextId, const char* errmsg) { errorHandlerExecuted = true; };
auto bridge = TEST_init(errorHandler);
kraken::KrakenPage::consoleMessageHandler = [](void* ctx, const std::string& message, int logLevel) { logCalled = true; };

std::string code = R"(
window.addEventListener('unhandledrejection', event => {
console.log('unhandledrejection fired: ' + event.reason);
});

window.addEventListener('rejectionhandled', event => {
console.log('rejectionhandled fired: ' + event.reason);
});

function generateRejectedPromise(isEventuallyHandled) {
// Create a promise which immediately rejects with a given reason.
var rejectedPromise = Promise.reject('Error');
}

generateRejectedPromise(true);
)";
bridge->evaluateScript(code.c_str(), code.size(), "file://", 0);
EXPECT_EQ(errorHandlerExecuted, false);
EXPECT_EQ(logCalled, true);
kraken::KrakenPage::consoleMessageHandler = nullptr;
}

TEST(Context, handledRejectionEventWillTriggerWhenUnHandledRejectHandled) {
static bool errorHandlerExecuted = false;
static bool logCalled = false;
auto errorHandler = [](int32_t contextId, const char* errmsg) { errorHandlerExecuted = true; };
auto bridge = TEST_init(errorHandler);
kraken::KrakenPage::consoleMessageHandler = [](void* ctx, const std::string& message, int logLevel) { logCalled = true; };

std::string code = R"(
window.addEventListener('unhandledrejection', event => {
console.log('unhandledrejection fired: ' + event.reason);
});

window.addEventListener('rejectionhandled', event => {
console.log('rejectionhandled fired: ' + event.reason);
});

function generateRejectedPromise() {
// Create a promise which immediately rejects with a given reason.
var rejectedPromise = Promise.reject('Error');
// We need to handle the rejection "after the fact" in order to trigger a
// unhandledrejection followed by rejectionhandled. Here we simulate that
// via a setTimeout(), but in a real-world system this might take place due
// to, e.g., fetch()ing resources at startup and then handling any rejected
// requests at some point later on.
setTimeout(() => {
// We need to provide an actual function to .catch() or else the promise
// won't be considered handled.
rejectedPromise.catch(() => {});
});
}

generateRejectedPromise();
)";
bridge->evaluateScript(code.c_str(), code.size(), "file://", 0);

TEST_runLoop(bridge->getContext());
EXPECT_EQ(errorHandlerExecuted, false);
EXPECT_EQ(logCalled, true);
kraken::KrakenPage::consoleMessageHandler = nullptr;
}

TEST(Context, unrejectPromiseErrorWithMultipleContext) {
static bool errorHandlerExecuted = false;
static int32_t errorCalledCount = 0;
Expand Down
9 changes: 4 additions & 5 deletions bridge/bindings/qjs/module_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ JSValue krakenModuleListener(JSContext* ctx, JSValueConst this_val, int argc, JS
return JS_NULL;
}

void handleInvokeModuleTransientCallback(void* callbackContext, int32_t contextId, NativeString* errmsg, NativeString* json) {
void handleInvokeModuleTransientCallback(void* callbackContext, int32_t contextId, const char* errmsg, NativeString* json) {
auto* moduleContext = static_cast<ModuleContext*>(callbackContext);
ExecutionContext* context = moduleContext->context;

Expand All @@ -53,9 +53,8 @@ void handleInvokeModuleTransientCallback(void* callbackContext, int32_t contextI
JSValue callback = moduleContext->callback;
JSValue returnValue;
if (errmsg != nullptr) {
JSValue errorMessage = JS_NewUnicodeString(context->runtime(), ctx, errmsg->string, errmsg->length);
JSValue errorObject = JS_NewError(ctx);
JS_DefinePropertyValue(ctx, errorObject, JS_NewAtom(ctx, "message"), errorMessage, JS_PROP_WRITABLE | JS_PROP_CONFIGURABLE);
JS_ThrowInternalError(ctx, "%s", errmsg);
JSValue errorObject = JS_GetException(ctx);
JSValue arguments[] = {errorObject};
returnValue = JS_Call(ctx, callback, context->global(), 1, arguments);
JS_FreeValue(ctx, errorObject);
Expand All @@ -76,7 +75,7 @@ void handleInvokeModuleTransientCallback(void* callbackContext, int32_t contextI
list_del(&moduleContext->link);
}

void handleInvokeModuleUnexpectedCallback(void* callbackContext, int32_t contextId, NativeString* errmsg, NativeString* json) {
void handleInvokeModuleUnexpectedCallback(void* callbackContext, int32_t contextId, const char* errmsg, NativeString* json) {
static_assert("Unexpected module callback, please check your invokeModule implementation on the dart side.");
}

Expand Down
31 changes: 31 additions & 0 deletions bridge/bindings/qjs/module_manager_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,35 @@ kraken.methodChannel.invokeMethod('abc', 'fn', object);
EXPECT_EQ(errorCalled, true);
}

TEST(ModuleManager, invokeModuleError) {
bool static logCalled = false;
auto bridge = TEST_init([](int32_t contextId, const char* errmsg) {});
kraken::KrakenPage::consoleMessageHandler = [](void* ctx, const std::string& message, int logLevel) {
logCalled = true;
EXPECT_STREQ(message.c_str(),
"Error {message: 'kraken://', stack: ' at __kraken_invoke_module__ (native)\n"
" at f (vm://:9)\n"
" at <eval> (vm://:11)\n"
"'}");
};

auto context = bridge->getContext();

std::string code = std::string(R"(
function f() {
kraken.invokeModule('throwError', 'kraken://', null, (e, error) => {
if (e) {
console.log(e);
} else {
console.log('test failed');
}
});
}
f();
)");
context->evaluateJavaScript(code.c_str(), code.size(), "vm://", 0);

EXPECT_EQ(logCalled, true);
}

} // namespace kraken::binding::qjs
5 changes: 5 additions & 0 deletions bridge/bindings/qjs/native_value.cc
Original file line number Diff line number Diff line change
Expand Up @@ -272,4 +272,9 @@ JSValue nativeValueToJSValue(ExecutionContext* context, NativeValue& value) {
return JS_NULL;
}

std::string nativeStringToStdString(NativeString* nativeString) {
std::u16string u16EventType = std::u16string(reinterpret_cast<const char16_t*>(nativeString->string), nativeString->length);
return toUTF8(u16EventType);
}

} // namespace kraken::binding::qjs
2 changes: 2 additions & 0 deletions bridge/bindings/qjs/native_value.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ NativeValue Native_NewJSON(ExecutionContext* context, JSValue& value);
NativeValue jsValueToNativeValue(JSContext* ctx, JSValue& value);
JSValue nativeValueToJSValue(ExecutionContext* context, NativeValue& value);

std::string nativeStringToStdString(NativeString* nativeString);

} // namespace kraken::binding::qjs

#endif // KRAKENBRIDGE_NATIVE_VALUE_H
Loading