Skip to content

Commit 33f7906

Browse files
committed
Proper exception catching
1 parent bf6ac1d commit 33f7906

File tree

3 files changed

+167
-1
lines changed

3 files changed

+167
-1
lines changed

code/framework/src/integrations/server/scripting/module.cpp

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,19 @@ namespace Framework::Integrations::Server::Scripting {
5757
// Initialize Framework SDK in the engine
5858
_nodeEngine->InitFrameworkSDK();
5959

60+
// Install uncaught exception handler to prevent async errors from
61+
// crashing the server process and route them to the resource system
62+
{
63+
v8::Isolate *isolate = _nodeEngine->GetIsolate();
64+
v8::Locker locker(isolate);
65+
v8::Isolate::Scope isolateScope(isolate);
66+
v8::HandleScope handleScope(isolate);
67+
v8::Local<v8::Context> context = _nodeEngine->GetContext();
68+
v8::Context::Scope contextScope(context);
69+
70+
_nodeEngine->InstallUncaughtExceptionHandler(_resourcesPath);
71+
}
72+
6073
Logging::GetLogger(FRAMEWORK_INNER_SCRIPTING)->info(
6174
"JS Server scripting module initialized with Node.js engine");
6275

@@ -136,6 +149,16 @@ namespace Framework::Integrations::Server::Scripting {
136149
if (_nodeEngine && _nodeEngine->IsInitialized()) {
137150
// Process Node.js event loop
138151
_nodeEngine->Tick();
152+
153+
// Process uncaught errors captured during Tick().
154+
// Deferred to here so HandleResourceRuntimeError can safely
155+
// stop/restart resources outside of V8/libuv callbacks.
156+
if (_resourceManager) {
157+
auto errors = _nodeEngine->DrainPendingErrors();
158+
for (const auto &err : errors) {
159+
_resourceManager->HandleResourceRuntimeError(err.resourceName, err.errorMessage);
160+
}
161+
}
139162
}
140163

141164
if (_resourceManager) {

code/framework/src/scripting/node_engine.cpp

Lines changed: 112 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
#include <logging/logger.h>
66

7+
#include <algorithm>
78
#include <filesystem>
89

910

@@ -175,13 +176,51 @@ namespace Framework::Scripting {
175176
v8::HandleScope handle_scope(_isolate);
176177
v8::Context::Scope context_scope(_setup->context());
177178

178-
// Load Node.js internals with require setup
179+
// Load Node.js internals with require setup and uncaught exception/rejection
180+
// handlers. These prevent async errors (timers, promises) from crashing
181+
// the host process.
182+
//
183+
// setUncaughtExceptionCaptureCallback is preferred over process.on('uncaughtException')
184+
// because it:
185+
// - Cannot be removed by user scripts (process.removeAllListeners)
186+
// - Prevents abort even with --abort-on-uncaught-exception
187+
// - Is designed for embedder use cases
188+
//
189+
// For unhandled promise rejections, process.on('unhandledRejection') is used
190+
// as there is no capture callback equivalent.
191+
//
192+
// Both route to __fw_handleUncaughtError if installed later via
193+
// InstallUncaughtExceptionHandler(), otherwise log to stderr.
179194
v8::MaybeLocal<v8::Value> loadResult = node::LoadEnvironment(
180195
_env,
181196
"const publicRequire = require('node:module').createRequire(process.cwd() + '/');"
182197
"globalThis.require = publicRequire;"
183198
"globalThis.Framework = {};"
184199
"globalThis.Core = {};"
200+
"process.setUncaughtExceptionCaptureCallback((err) => {"
201+
" try {"
202+
" const msg = err instanceof Error ? (err.stack || err.message) : String(err);"
203+
" if (typeof globalThis.__fw_handleUncaughtError === 'function') {"
204+
" globalThis.__fw_handleUncaughtError(msg, 'uncaughtException');"
205+
" } else {"
206+
" console.error('[uncaughtException]', msg);"
207+
" }"
208+
" } catch(e) {"
209+
" console.error('Error in uncaught exception handler:', e);"
210+
" }"
211+
"});"
212+
"process.on('unhandledRejection', (reason) => {"
213+
" try {"
214+
" const msg = reason instanceof Error ? (reason.stack || reason.message) : String(reason);"
215+
" if (typeof globalThis.__fw_handleUncaughtError === 'function') {"
216+
" globalThis.__fw_handleUncaughtError(msg, 'unhandledRejection');"
217+
" } else {"
218+
" console.error('[unhandledRejection]', msg);"
219+
" }"
220+
" } catch(e) {"
221+
" console.error('Error in unhandled rejection handler:', e);"
222+
" }"
223+
"});"
185224
);
186225

187226
if (loadResult.IsEmpty()) {
@@ -280,6 +319,78 @@ namespace Framework::Scripting {
280319
return v8::Local<v8::Context>();
281320
}
282321

322+
void NodeEngine::InstallUncaughtExceptionHandler(const std::string &resourcesPath) {
323+
// Store canonical resources path for extracting resource names from error stacks
324+
std::error_code ec;
325+
auto canonicalPath = std::filesystem::weakly_canonical(resourcesPath, ec);
326+
_resourcesPath = ec ? resourcesPath : canonicalPath.string();
327+
328+
// Create C++ handler function accessible from JS
329+
v8::Local<v8::Context> context = _setup->context();
330+
v8::Local<v8::External> data = v8::External::New(_isolate, this);
331+
v8::Local<v8::FunctionTemplate> tmpl = v8::FunctionTemplate::New(
332+
_isolate, OnUncaughtError, data);
333+
v8::Local<v8::Function> fn = tmpl->GetFunction(context).ToLocalChecked();
334+
335+
v8::Local<v8::String> key = v8::String::NewFromUtf8(
336+
_isolate, "__fw_handleUncaughtError").ToLocalChecked();
337+
context->Global()->Set(context, key, fn).Check();
338+
}
339+
340+
void NodeEngine::OnUncaughtError(const v8::FunctionCallbackInfo<v8::Value> &info) {
341+
v8::Isolate *isolate = info.GetIsolate();
342+
auto *engine = static_cast<NodeEngine *>(
343+
v8::Local<v8::External>::Cast(info.Data())->Value());
344+
345+
std::string errorMsg = "Unknown error";
346+
std::string origin = "uncaughtException";
347+
348+
if (info.Length() > 0) {
349+
v8::String::Utf8Value msg(isolate, info[0]);
350+
if (*msg) errorMsg = *msg;
351+
}
352+
if (info.Length() > 1) {
353+
v8::String::Utf8Value orig(isolate, info[1]);
354+
if (*orig) origin = *orig;
355+
}
356+
357+
// Try to extract resource name from the error stack trace by matching
358+
// file paths against the configured resources directory
359+
std::string resourceName;
360+
if (!engine->_resourcesPath.empty()) {
361+
// Normalize path separators for cross-platform matching
362+
std::string normalizedError = errorMsg;
363+
std::string normalizedResPath = engine->_resourcesPath;
364+
std::replace(normalizedError.begin(), normalizedError.end(), '\\', '/');
365+
std::replace(normalizedResPath.begin(), normalizedResPath.end(), '\\', '/');
366+
367+
if (!normalizedResPath.empty() && normalizedResPath.back() != '/') {
368+
normalizedResPath += '/';
369+
}
370+
371+
size_t pos = normalizedError.find(normalizedResPath);
372+
if (pos != std::string::npos) {
373+
size_t nameStart = pos + normalizedResPath.size();
374+
size_t nameEnd = normalizedError.find('/', nameStart);
375+
if (nameEnd != std::string::npos) {
376+
resourceName = normalizedError.substr(nameStart, nameEnd - nameStart);
377+
}
378+
}
379+
}
380+
381+
// Queue for processing outside of Tick()
382+
engine->_pendingErrors.push_back({
383+
resourceName.empty() ? "unknown" : resourceName,
384+
"[" + origin + "] " + errorMsg
385+
});
386+
}
387+
388+
std::vector<NodeEngine::PendingUncaughtError> NodeEngine::DrainPendingErrors() {
389+
std::vector<PendingUncaughtError> errors;
390+
errors.swap(_pendingErrors);
391+
return errors;
392+
}
393+
283394
bool NodeEngine::ApplySandbox() {
284395
// This function disables dangerous Node.js APIs for client-side sandboxing.
285396
// We override require() to block dangerous modules and remove dangerous

code/framework/src/scripting/node_engine.h

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,31 @@ namespace Framework::Scripting {
8484
*/
8585
void Tick();
8686

87+
/**
88+
* Pending uncaught error from process.on('uncaughtException') or
89+
* process.on('unhandledRejection'), queued during Tick() for
90+
* deferred processing outside of V8/libuv callbacks.
91+
*/
92+
struct PendingUncaughtError {
93+
std::string resourceName;
94+
std::string errorMessage;
95+
};
96+
97+
/**
98+
* Install the C++ handler for uncaught exceptions/rejections.
99+
* The bootstrap already installs JS process handlers that prevent crashes.
100+
* This method connects them to a C++ callback for error attribution.
101+
* Must be called with V8 scopes active.
102+
* @param resourcesPath Path to resources directory for extracting resource names from stacks
103+
*/
104+
void InstallUncaughtExceptionHandler(const std::string &resourcesPath);
105+
106+
/**
107+
* Drain pending uncaught errors. Returns and clears the queue.
108+
* Call after Tick() to process errors outside of V8/libuv callbacks.
109+
*/
110+
std::vector<PendingUncaughtError> DrainPendingErrors();
111+
87112
/**
88113
* Check if this engine is running in sandboxed mode.
89114
*/
@@ -122,6 +147,8 @@ namespace Framework::Scripting {
122147
bool CreateEnvironment();
123148
bool ApplySandbox();
124149

150+
static void OnUncaughtError(const v8::FunctionCallbackInfo<v8::Value> &info);
151+
125152
NodeEngineOptions _options;
126153

127154
static std::unique_ptr<node::MultiIsolatePlatform> _platform;
@@ -141,6 +168,11 @@ namespace Framework::Scripting {
141168
// RunAndClearNativeImmediates → RunAndClearInterrupts internally.
142169
// Only created when FW_NODE_INSPECTOR is defined and inspector enabled.
143170
v8::Global<v8::Function> _interruptDrainFn;
171+
172+
// Pending uncaught errors queued during Tick() for deferred processing
173+
std::vector<PendingUncaughtError> _pendingErrors;
174+
// Canonical resources path for extracting resource names from error stacks
175+
std::string _resourcesPath;
144176
};
145177

146178
} // namespace Framework::Scripting

0 commit comments

Comments
 (0)