Skip to content

Commit 5acef93

Browse files
committed
feat: improved exception handling to not stop execution and show stack to allow continued development during debug
1 parent ef9a75a commit 5acef93

File tree

4 files changed

+157
-3
lines changed

4 files changed

+157
-3
lines changed

NativeScript/runtime/ModuleInternal.mm

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -522,8 +522,22 @@ throw NativeScriptException(isolate,
522522
}
523523

524524
Local<Script> script = ModuleInternal::LoadClassicScript(isolate, path);
525-
// run it and return the value
526-
return script->Run(isolate->GetCurrentContext()).ToLocalChecked();
525+
526+
// run it and return the value with proper exception handling
527+
Local<Context> context = isolate->GetCurrentContext();
528+
TryCatch tc(isolate);
529+
Local<Value> result;
530+
531+
if (!script->Run(context).ToLocal(&result)) {
532+
// Script execution failed, throw a proper exception instead of aborting V8
533+
if (tc.HasCaught()) {
534+
throw NativeScriptException(isolate, tc, "Cannot execute script " + path);
535+
} else {
536+
throw NativeScriptException(isolate, "Script execution failed for " + path);
537+
}
538+
}
539+
540+
return result;
527541
}
528542

529543
Local<Script> ModuleInternal::LoadClassicScript(Isolate* isolate, const std::string& path) {
@@ -709,7 +723,14 @@ ScriptOrigin origin(
709723
ScriptCompiler::CompileOptions options = ScriptCompiler::kNoCompileOptions;
710724
ScriptCompiler::Source source(tns::ToV8String(isolate, scriptString));
711725
TryCatch tc(isolate);
712-
Local<Script> script = ScriptCompiler::Compile(context, &source, options).ToLocalChecked();
726+
727+
// Handle script compilation safely
728+
Local<Script> script;
729+
if (!ScriptCompiler::Compile(context, &source, options).ToLocal(&script)) {
730+
// Compilation failed - return empty MaybeLocal to indicate failure
731+
return MaybeLocal<Value>();
732+
}
733+
713734
MaybeLocal<Value> result = script->Run(context);
714735
return result;
715736
}

NativeScript/runtime/NativeScriptException.mm

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#include "Caches.h"
44
#include "Helpers.h"
55
#include "Runtime.h"
6+
#include "RuntimeConfig.h"
67

78
using namespace v8;
89

@@ -148,6 +149,56 @@
148149
Exception::Error(tns::ToV8String(isolate, "No javascript exception or message provided."));
149150
}
150151

152+
// For critical exceptions (like module loading failures), provide detailed error reporting
153+
bool isCriticalException = false;
154+
155+
// Check if this is a critical exception that should show detailed error info
156+
if (!this->message_.empty()) {
157+
// Module-related errors should show detailed stack traces
158+
isCriticalException =
159+
(this->message_.find("Error calling module function") != std::string::npos ||
160+
this->message_.find("Cannot evaluate module") != std::string::npos ||
161+
this->message_.find("Cannot instantiate module") != std::string::npos ||
162+
this->message_.find("Cannot compile") != std::string::npos);
163+
}
164+
165+
if (isCriticalException) {
166+
// Create detailed error message similar to OnUncaughtError
167+
std::string stackTrace = this->stackTrace_;
168+
std::string fullMessage;
169+
170+
if (!this->fullMessage_.empty()) {
171+
fullMessage = this->fullMessage_;
172+
} else {
173+
fullMessage = this->message_ + "\n at \n" + stackTrace;
174+
}
175+
176+
// Always log the detailed error for critical exceptions (both debug and release)
177+
NSLog(@"***** JavaScript exception occurred - detailed stack trace follows *****\n");
178+
NSLog(@"NativeScript encountered an error:");
179+
NSString* errorStr = [NSString stringWithUTF8String:fullMessage.c_str()];
180+
if (errorStr != nil) {
181+
NSLog(@"%@", errorStr);
182+
} else {
183+
NSLog(@"(error message contained invalid UTF-8)");
184+
}
185+
186+
// In debug mode, continue execution; in release mode, terminate
187+
if (RuntimeConfig.IsDebug) {
188+
NSLog(@"***** End stack trace - continuing execution *****\n");
189+
// In debug mode, just throw the exception normally - don't terminate
190+
isolate->ThrowException(errObj);
191+
return;
192+
} else {
193+
NSLog(@"***** End stack trace - terminating application *****\n");
194+
// In release mode, create proper message and call OnUncaughtError for termination
195+
Local<v8::Message> message = Exception::CreateMessage(isolate, errObj);
196+
OnUncaughtError(message, errObj);
197+
return; // OnUncaughtError will terminate, so we don't continue
198+
}
199+
}
200+
201+
// For non-critical exceptions, just re-throw normally
151202
isolate->ThrowException(errObj);
152203
}
153204

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
describe("Exception Handling Tests", function () {
2+
it("should provide detailed stack trace for undefined global property access without crashing in debug mode", function (done) {
3+
// This test simulates the exact scenario mentioned in the issue:
4+
// global.CanvasModule.__addFontFamily('poppins', [font]);
5+
// where CanvasModule is undefined
6+
7+
let originalOnUncaughtError = global.__onUncaughtError;
8+
let errorCaught = false;
9+
let errorDetails = null;
10+
11+
// In debug mode, the error should be caught by a regular try/catch
12+
// rather than the uncaught error handler, since the app doesn't crash
13+
14+
try {
15+
// Simulate the problematic code from bundle.js
16+
// This will throw "Cannot read properties of undefined (reading '__addFontFamily')"
17+
global.CanvasModule.__addFontFamily('poppins', []);
18+
19+
// Should not reach here
20+
done(new Error("Expected error to be thrown"));
21+
} catch (error) {
22+
// In debug mode, this should be caught here instead of crashing
23+
expect(error).toBeDefined();
24+
expect(error.message).toContain("Cannot read properties of undefined");
25+
26+
console.log("✓ Development-friendly error handling:");
27+
console.log("Message:", error.message);
28+
console.log("✓ App continues running without crash");
29+
30+
done();
31+
}
32+
});
33+
34+
it("should provide detailed error logging for critical exceptions in debug mode", function (done) {
35+
// This test checks that critical exceptions log detailed info without crashing
36+
37+
try {
38+
// Trigger a reference error that would normally cause detailed logging
39+
someNonExistentGlobalFunction.call();
40+
done(new Error("Expected error to be thrown"));
41+
} catch (error) {
42+
expect(error).toBeDefined();
43+
expect(error.name).toBe("ReferenceError");
44+
console.log("✓ Reference error handled gracefully:");
45+
console.log("Message:", error.message);
46+
console.log("✓ App continues running");
47+
done();
48+
}
49+
});
50+
51+
it("should demonstrate hot-reload friendly error handling", function (done) {
52+
// This test simulates what happens when you have an error in your code
53+
// but want to fix it and hot-reload without the app crashing
54+
55+
let errorCount = 0;
56+
let successCount = 0;
57+
58+
// Simulate multiple error-fix cycles
59+
for (let i = 0; i < 3; i++) {
60+
try {
61+
if (i < 2) {
62+
// First two iterations: cause errors (simulating buggy code)
63+
global.someUndefinedObject.method();
64+
} else {
65+
// Third iteration: success (simulating fixed code)
66+
successCount++;
67+
}
68+
} catch (error) {
69+
errorCount++;
70+
console.log(`Iteration ${i + 1}: Caught error (${error.message}) - app continues`);
71+
}
72+
}
73+
74+
expect(errorCount).toBe(2);
75+
expect(successCount).toBe(1);
76+
console.log("✓ Hot-reload friendly: 2 errors caught, 1 success, app never crashed");
77+
done();
78+
});
79+
});

TestRunner/app/tests/index.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,9 @@ require("./URL");
8484
require("./URLSearchParams");
8585
require("./URLPattern");
8686

87+
// Exception handling tests
88+
require("./ExceptionHandlingTests");
89+
8790
// Tests common for all runtimes.
8891
require("./shared/index").runAllTests();
8992

0 commit comments

Comments
 (0)