Skip to content

Commit 5c06ce7

Browse files
authored
A bunch of miscellaneous fixes (#88)
- Move Node-API extensions from Babylon Native into JsRuntimeHost. - Add helper method to Node-API extensions to get error string from either callstack or message if callstack is not available. Use this method where appropriate (e.g., DefaultUnhandledExceptionHandler) to avoid crash when callstack is not available. - Add EnableDebugger flag to AppRuntime to allow user configuration. - Add Node-API defines required for JsRuntimeHost directly into the headers instead in CMake to avoid having all consumers add defines to the compile options. - Fix an issue in Napi::Error::New to allow Napi::Error to propogate properly. - Don't throw in Chakra's napi_add_finalizer to avoid compiler warning. - Fix an issue in XMLHttpRequest where some errors are being dropped.
1 parent 66c863b commit 5c06ce7

File tree

29 files changed

+279
-106
lines changed

29 files changed

+279
-106
lines changed

CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ FetchContent_Declare(llhttp
3030
URL "https://github.com/nodejs/llhttp/archive/refs/tags/release/v8.1.0.tar.gz")
3131
FetchContent_Declare(UrlLib
3232
GIT_REPOSITORY https://github.com/BabylonJS/UrlLib.git
33-
GIT_TAG 59917f32f6ddfa26af07dd981842c51ce02dafcd)
33+
GIT_TAG c7c162be129e170a12bb8766c727b59c42853826)
3434
# --------------------------------------------------
3535

3636
FetchContent_MakeAvailable(CMakeExtensions)

Core/AppRuntime/Include/Babylon/AppRuntime.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
#include <Babylon/JsRuntime.h>
66

7+
#include <napi/utilities.h>
8+
79
#include <memory>
810
#include <functional>
911
#include <exception>
@@ -21,6 +23,9 @@ namespace Babylon
2123
// Optional handler for unhandled exceptions.
2224
std::function<void(const Napi::Error&)> UnhandledExceptionHandler{DefaultUnhandledExceptionHandler};
2325

26+
// Defines whether to enable the debugger. Only implemented for V8 and Chakra.
27+
bool EnableDebugger{false};
28+
2429
// Waits for the debugger to be attached before the execution of any script. Only implemented for V8.
2530
bool WaitForDebugger{false};
2631
};

Core/AppRuntime/Source/AppRuntime_Android.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ namespace Babylon
55
{
66
void AppRuntime::DefaultUnhandledExceptionHandler(const Napi::Error& error)
77
{
8-
__android_log_print(ANDROID_LOG_ERROR, "BabylonNative", "[Uncaught Error] %s", error.Get("stack").As<Napi::String>().Utf8Value().data());
8+
__android_log_print(ANDROID_LOG_ERROR, "BabylonNative", "[Uncaught Error] %s", Napi::GetErrorString(error).data());
99
}
1010

1111
void AppRuntime::RunPlatformTier()

Core/AppRuntime/Source/AppRuntime_Chakra.cpp

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
#include <gsl/gsl>
88
#include <cassert>
9+
#include <sstream>
910

1011
namespace Babylon
1112
{
@@ -15,7 +16,9 @@ namespace Babylon
1516
{
1617
if (errorCode != JsErrorCode::JsNoError)
1718
{
18-
throw std::exception();
19+
std::ostringstream ss;
20+
ss << "Chakra function failed with error code (" << static_cast<int>(errorCode) << ")";
21+
throw std::runtime_error{ss.str()};
1922
}
2023
}
2124
}
@@ -35,29 +38,28 @@ namespace Babylon
3538
JsContextRef context;
3639
ThrowIfFailed(JsCreateContext(jsRuntime, &context));
3740
ThrowIfFailed(JsSetCurrentContext(context));
38-
ThrowIfFailed(JsSetPromiseContinuationCallback([](JsValueRef task, void* callbackState) {
39-
ThrowIfFailed(JsAddRef(task, nullptr));
40-
auto* dispatch = reinterpret_cast<DispatchFunction*>(callbackState);
41-
dispatch->operator()([task]() {
42-
JsValueRef undefined;
43-
ThrowIfFailed(JsGetUndefinedValue(&undefined));
44-
ThrowIfFailed(JsCallFunction(task, &undefined, 1, nullptr));
45-
ThrowIfFailed(JsRelease(task, nullptr));
46-
});
47-
},
41+
ThrowIfFailed(JsSetPromiseContinuationCallback(
42+
[](JsValueRef task, void* callbackState) {
43+
ThrowIfFailed(JsAddRef(task, nullptr));
44+
auto* dispatch = reinterpret_cast<DispatchFunction*>(callbackState);
45+
dispatch->operator()([task]() {
46+
JsValueRef undefined;
47+
ThrowIfFailed(JsGetUndefinedValue(&undefined));
48+
ThrowIfFailed(JsCallFunction(task, &undefined, 1, nullptr));
49+
ThrowIfFailed(JsRelease(task, nullptr));
50+
});
51+
},
4852
&dispatchFunction));
4953
ThrowIfFailed(JsProjectWinRTNamespace(L"Windows"));
5054

51-
#if defined(_DEBUG)
52-
// Put Chakra in debug mode.
55+
if (m_options.EnableDebugger)
5356
{
5457
auto result = JsStartDebugging();
5558
if (result != JsErrorCode::JsNoError)
5659
{
5760
OutputDebugStringW(L"Failed to initialize JavaScript debugging support.\n");
5861
}
5962
}
60-
#endif
6163

6264
Napi::Env env = Napi::Attach();
6365

Core/AppRuntime/Source/AppRuntime_JavaScriptCore.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@ namespace Babylon
66
void AppRuntime::RunEnvironmentTier(const char*)
77
{
88
auto globalContext = JSGlobalContextCreateInGroup(nullptr, nullptr);
9+
10+
// REVIEW: Ideally, we should call this, but it's not always available in all situations.
11+
//JSGlobalContextSetInspectable(globalContext, m_options.EnableDebugger);
12+
913
Napi::Env env = Napi::Attach(globalContext);
1014

1115
Run(env);

Core/AppRuntime/Source/AppRuntime_UWP.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ namespace Babylon
77
void BABYLON_API AppRuntime::DefaultUnhandledExceptionHandler(const Napi::Error& error)
88
{
99
std::ostringstream ss{};
10-
ss << "[Uncaught Error] " << error.Get("stack").As<Napi::String>().Utf8Value() << std::endl;
10+
ss << "[Uncaught Error] " << Napi::GetErrorString(error) << std::endl;
1111
OutputDebugStringA(ss.str().data());
1212
}
1313

Core/AppRuntime/Source/AppRuntime_Unix.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ namespace Babylon
55
{
66
void AppRuntime::DefaultUnhandledExceptionHandler(const Napi::Error& error)
77
{
8-
std::cerr << "[Uncaught Error] " << error.Get("stack").As<Napi::String>().Utf8Value().data() << std::endl;
8+
std::cerr << "[Uncaught Error] " << Napi::GetErrorString(error) << std::endl;
99
}
1010

1111
void AppRuntime::RunPlatformTier()

Core/AppRuntime/Source/AppRuntime_V8.cpp

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,14 @@
11
#include "AppRuntime.h"
22
#include <napi/env.h>
33

4-
#if defined(_MSC_VER)
5-
#pragma warning(disable : 4100 4267 4127)
6-
#endif
7-
#if defined(__clang__)
8-
#pragma clang diagnostic ignored "-Wunused-parameter"
9-
#endif
10-
#if defined(__GNUC__)
11-
#pragma GCC diagnostic ignored "-Wunused-parameter"
12-
#endif
13-
#include <v8.h>
144
#include <libplatform/libplatform.h>
155

166
#ifdef ENABLE_V8_INSPECTOR
177
#include <V8InspectorAgent.h>
188
#endif
199

10+
#include <optional>
11+
2012
namespace Babylon
2113
{
2214
namespace
@@ -90,19 +82,26 @@ namespace Babylon
9082
Napi::Env env = Napi::Attach(context);
9183

9284
#ifdef ENABLE_V8_INSPECTOR
93-
V8InspectorAgent agent{Module::Instance().Platform(), isolate, context, "JsRuntimeHost"};
94-
agent.Start(5643, "JsRuntimeHost");
95-
96-
if (m_options.WaitForDebugger)
85+
std::optional<V8InspectorAgent> agent;
86+
if (m_options.EnableDebugger)
9787
{
98-
agent.WaitForDebugger();
88+
agent.emplace(Module::Instance().Platform(), isolate, context, "JsRuntimeHost");
89+
agent->Start(5643, "JsRuntimeHost");
90+
91+
if (m_options.WaitForDebugger)
92+
{
93+
agent->WaitForDebugger();
94+
}
9995
}
10096
#endif
10197

10298
Run(env);
10399

104100
#ifdef ENABLE_V8_INSPECTOR
105-
agent.Stop();
101+
if (agent.has_value())
102+
{
103+
agent->Stop();
104+
}
106105
#endif
107106

108107
Napi::Detach(env);

Core/AppRuntime/Source/AppRuntime_Win32.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ namespace Babylon
1010
void BABYLON_API AppRuntime::DefaultUnhandledExceptionHandler(const Napi::Error& error)
1111
{
1212
std::ostringstream ss{};
13-
ss << "[Uncaught Error] " << error.Get("stack").As<Napi::String>().Utf8Value() << std::endl;
13+
ss << "[Uncaught Error] " << Napi::GetErrorString(error) << std::endl;
1414
OutputDebugStringA(ss.str().data());
1515
}
1616

Core/AppRuntime/Source/AppRuntime_iOS.mm

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
{
66
void AppRuntime::DefaultUnhandledExceptionHandler(const Napi::Error& error)
77
{
8-
NSLog(@"[Uncaught Error] %s", error.Get("stack").As<Napi::String>().Utf8Value().data());
8+
NSLog(@"[Uncaught Error] %s", Napi::GetErrorString(error).data());
99
}
1010

1111
void AppRuntime::RunPlatformTier()

0 commit comments

Comments
 (0)