Skip to content

Commit af257ee

Browse files
NathanWalkerNathanWalker
committed
chore: cleanup error display logic with remap
1 parent 55a219e commit af257ee

File tree

9 files changed

+687
-602
lines changed

9 files changed

+687
-602
lines changed

‎NativeScript/runtime/Console.cpp‎

Lines changed: 24 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,6 @@ using namespace v8;
1717

1818
namespace tns {
1919

20-
// Flag to prevent duplicate error modals from OnUncaughtError
21-
bool consoleModalShown = false;
22-
2320
void Console::Init(Local<Context> context) {
2421
Isolate* isolate = context->GetIsolate();
2522
Context::Scope context_scope(context);
@@ -61,56 +58,12 @@ bool isErrorMessage(const std::string& line) {
6158
}
6259

6360
bool isStackFrame(const std::string& line) {
64-
// e.g. " at foo (/path/to/file.ts:123:45)"
65-
static const std::regex stackRe(R"(\s+at\s+\S+:\d+:\d+)");
66-
return std::regex_search(line, stackRe);
67-
}
68-
69-
std::vector<std::string> filterErrorLines(
70-
const std::vector<std::string>& allLines) {
71-
std::vector<std::string> result;
72-
result.reserve(allLines.size());
73-
for (auto& line : allLines) {
74-
if (isErrorMessage(line) || isStackFrame(line)) {
75-
result.push_back(line);
76-
}
77-
}
78-
return result;
79-
}
80-
81-
std::string Console::RemapStackTrace(v8::Isolate* isolate, const std::string& stackTrace) {
82-
// Get the current context from the isolate
83-
Local<Context> context = isolate->GetCurrentContext();
84-
85-
// Get the global object
86-
Local<Object> global = context->Global();
87-
88-
// Get the __ns_remapStack function from global
89-
Local<Value> remapStackValue;
90-
bool success =
91-
global->Get(context, tns::ToV8String(isolate, "__ns_remapStack"))
92-
.ToLocal(&remapStackValue);
93-
94-
if (success && remapStackValue->IsFunction()) {
95-
Local<v8::Function> remapStackFunction =
96-
remapStackValue.As<v8::Function>();
97-
98-
// Prepare arguments - convert your string to V8 string
99-
Local<Value> args[] = {tns::ToV8String(isolate, stackTrace)};
100-
101-
// Call the function
102-
Local<Value> result;
103-
bool callSuccess =
104-
remapStackFunction->Call(context, global, 1, args).ToLocal(&result);
105-
106-
if (callSuccess && result->IsString()) {
107-
// If the function returns a modified string, use it
108-
return tns::ToString(isolate, result);
109-
}
110-
}
111-
112-
// Return original string if remapping failed or function not available
113-
return stackTrace;
61+
// Recognize both styles:
62+
// " at foo (/path/to/file.ts:123:45)" -> with parentheses
63+
// " at /path/to/file.ts:123:45" -> bare location
64+
static const std::regex withParens(R"(\s+at\s+.*\(.+?:\d+:\d+\))");
65+
static const std::regex bare(R"(\s+at\s+[^\s\(\)]+:\d+:\d+)");
66+
return std::regex_search(line, withParens) || std::regex_search(line, bare);
11467
}
11568

11669
void Console::LogCallback(const FunctionCallbackInfo<Value>& args) {
@@ -127,52 +80,19 @@ void Console::LogCallback(const FunctionCallbackInfo<Value>& args) {
12780
Local<v8::String> data = args.Data().As<v8::String>();
12881
std::string verbosityLevel = tns::ToString(isolate, data);
12982

130-
if (RuntimeConfig.IsDebug && Runtime::showErrorDisplay() &&
131-
(verbosityLevel == "error" || verbosityLevel == "log")) {
132-
// Show in-flight error display when enabled
133-
// Simple universal error detection - any error with stack trace
134-
bool hasStackTrace = isStackFrame(stringResult);
135-
136-
if (hasStackTrace && !consoleModalShown) {
137-
std::stringstream stackTraceLines;
138-
stackTraceLines << stringResult;
139-
140-
std::string stacktrace = tns::GetStackTrace(isolate);
141-
stackTraceLines << std::endl << stacktrace << std::endl;
142-
143-
std::string errorToDisplay = stackTraceLines.str();
144-
145-
// Extract error details
146-
std::string errorTitle = "JavaScript Error";
147-
148-
// Apply source map remapping to the error display
149-
errorToDisplay = RemapStackTrace(isolate, errorToDisplay);
150-
151-
try {
152-
NativeScriptException::ShowErrorModal(errorTitle, errorToDisplay,
153-
errorToDisplay);
154-
consoleModalShown = true; // Prevent duplicate modals
155-
156-
} catch (const std::exception& e) {
157-
Log("Console.cpp: Exception showing modal: %s", e.what());
158-
} catch (...) {
159-
Log("Console.cpp: Unknown exception showing modal");
160-
}
161-
}
83+
// Compute remapped payload ONCE and use it for both the modal and terminal
84+
// so they always match exactly.
85+
bool hasStackTrace = isStackFrame(stringResult);
86+
std::string processedStringResult = stringResult;
87+
if (hasStackTrace) {
88+
processedStringResult = tns::RemapStackTraceIfAvailable(isolate, processedStringResult);
16289
}
90+
16391
std::string verbosityLevelUpper = verbosityLevel;
16492
std::transform(verbosityLevelUpper.begin(), verbosityLevelUpper.end(),
16593
verbosityLevelUpper.begin(), ::toupper);
16694

16795
std::stringstream ss;
168-
std::string processedStringResult = stringResult;
169-
170-
// Apply source map remapping if this contains a stack trace
171-
bool hasStackTrace = isStackFrame(stringResult);
172-
if (hasStackTrace) {
173-
processedStringResult = RemapStackTrace(isolate, processedStringResult);
174-
}
175-
17696
ss << processedStringResult;
17797

17898
if (verbosityLevel == "trace") {
@@ -187,6 +107,17 @@ void Console::LogCallback(const FunctionCallbackInfo<Value>& args) {
187107
std::string msgWithVerbosity =
188108
"CONSOLE " + verbosityLevelUpper + ": " + msgToLog;
189109
Log("%s", msgWithVerbosity.c_str());
110+
111+
if (RuntimeConfig.IsDebug && Runtime::showErrorDisplay() && verbosityLevel == "error" && hasStackTrace) {
112+
try {
113+
// Log("Console.cpp: Forwarding console payload to error display: %s", msgToLog.c_str());
114+
NativeScriptException::SubmitConsoleErrorPayload(isolate, msgToLog);
115+
} catch (const std::exception& e) {
116+
Log("Console.cpp: Exception updating modal: %s", e.what());
117+
} catch (...) {
118+
Log("Console.cpp: Unknown exception updating modal");
119+
}
120+
}
190121
}
191122

192123
void Console::AssertCallback(const FunctionCallbackInfo<Value>& args) {

‎NativeScript/runtime/Console.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ class Console {
2525
static const v8::Local<v8::String> BuildStringFromArg(v8::Local<v8::Context> context, const v8::Local<v8::Value>& val);
2626
static const v8::Local<v8::String> TransformJSObject(v8::Local<v8::Object> object);
2727
static ConsoleAPIType VerbosityToInspectorMethod(const std::string level);
28-
static std::string RemapStackTrace(v8::Isolate* isolate, const std::string& stackTrace);
2928

3029
static void SendToDevToolsFrontEnd(ConsoleAPIType method,
3130
const v8::FunctionCallbackInfo<v8::Value>& args);

‎NativeScript/runtime/Helpers.h‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,17 @@ const std::string BuildStacktraceFrameMessage(v8::Isolate* isolate,
333333
const std::string GetStackTrace(v8::Isolate* isolate);
334334
const std::string GetCurrentScriptUrl(v8::Isolate* isolate);
335335

336+
// Returns stack trace string remapped to original sources using global __ns_remapStack if present.
337+
std::string RemapStackTraceIfAvailable(v8::Isolate* isolate, const std::string& stackTrace);
338+
339+
// Smart stack extraction that prefers:
340+
// 1) exception.stack if provided
341+
// 2) TryCatch.StackTrace / Message()->GetStackTrace when TryCatch provided
342+
// 3) Current stack via GetStackTrace
343+
std::string GetSmartStackTrace(v8::Isolate* isolate,
344+
v8::TryCatch* tryCatch = nullptr,
345+
v8::Local<v8::Value> exception = v8::Local<v8::Value>());
346+
336347
bool LiveSync(v8::Isolate* isolate);
337348

338349
void Assert(bool condition, v8::Isolate* isolate = nullptr,

‎NativeScript/runtime/Helpers.mm‎

Lines changed: 113 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -336,30 +336,28 @@
336336
std::condition_variable cv;
337337
};
338338

339-
void tns::ExecuteOnRunLoop(CFRunLoopRef queue, std::function<void()> func, bool async) {
340-
if (!async) {
341-
bool __block finished = false;
342-
auto v = new LockAndCV;
343-
std::unique_lock<std::mutex> lock(v->m);
344-
CFRunLoopPerformBlock(queue, kCFRunLoopCommonModes, ^(void) {
345-
func();
346-
{
347-
std::unique_lock lk(v->m);
348-
finished = true;
349-
}
350-
v->cv.notify_all();
351-
});
352-
CFRunLoopWakeUp(queue);
353-
while (!finished) {
354-
v->cv.wait(lock);
339+
void tns::ExecuteOnRunLoop(CFRunLoopRef queue, void (^func)(void), bool async) {
340+
if(!async) {
341+
bool __block finished = false;
342+
auto v = new LockAndCV;
343+
std::unique_lock<std::mutex> lock(v->m);
344+
CFRunLoopPerformBlock(queue, kCFRunLoopCommonModes, ^(void) {
345+
func();
346+
{
347+
std::unique_lock lk(v->m);
348+
finished = true;
349+
}
350+
v->cv.notify_all();
351+
});
352+
CFRunLoopWakeUp(queue);
353+
while(!finished) {
354+
v->cv.wait(lock);
355+
}
356+
delete v;
357+
} else {
358+
CFRunLoopPerformBlock(queue, kCFRunLoopCommonModes, func);
359+
CFRunLoopWakeUp(queue);
355360
}
356-
delete v;
357-
} else {
358-
CFRunLoopPerformBlock(queue, kCFRunLoopCommonModes, ^(void) {
359-
func();
360-
});
361-
CFRunLoopWakeUp(queue);
362-
}
363361
}
364362

365363
void tns::ExecuteOnDispatchQueue(dispatch_queue_t queue, std::function<void()> func, bool async) {
@@ -573,6 +571,98 @@
573571
return "";
574572
}
575573

574+
std::string tns::RemapStackTraceIfAvailable(Isolate* isolate, const std::string& stackTrace) {
575+
if (stackTrace.empty()) {
576+
return stackTrace;
577+
}
578+
if (isolate == nullptr) {
579+
return stackTrace;
580+
}
581+
582+
v8::Locker locker(isolate);
583+
Isolate::Scope isolateScope(isolate);
584+
HandleScope handleScope(isolate);
585+
Local<Context> context = Caches::Get(isolate)->GetContext();
586+
if (context.IsEmpty()) {
587+
return stackTrace;
588+
}
589+
Context::Scope contextScope(context);
590+
Local<Object> global = context->Global();
591+
Local<Value> remapFnVal;
592+
bool hasRemap =
593+
global->Get(context, tns::ToV8String(isolate, "__ns_remapStack")).ToLocal(&remapFnVal);
594+
if (hasRemap && remapFnVal->IsFunction()) {
595+
Local<v8::Function> remapFn = remapFnVal.As<v8::Function>();
596+
Local<Value> args[] = {tns::ToV8String(isolate, stackTrace)};
597+
Local<Value> remapped;
598+
if (remapFn->Call(context, global, 1, args).ToLocal(&remapped) && remapped->IsString()) {
599+
return tns::ToString(isolate, remapped.As<v8::String>());
600+
}
601+
}
602+
return stackTrace;
603+
}
604+
605+
std::string tns::GetSmartStackTrace(Isolate* isolate, v8::TryCatch* tryCatch,
606+
v8::Local<v8::Value> exception) {
607+
std::string stack;
608+
Local<Context> context = isolate->GetCurrentContext();
609+
610+
// 1) Prefer exception.stack when provided
611+
if (!exception.IsEmpty()) {
612+
if (exception->IsObject()) {
613+
Local<Object> excObj = exception.As<Object>();
614+
Local<Value> stackVal;
615+
if (excObj->Get(context, tns::ToV8String(isolate, "stack")).ToLocal(&stackVal) &&
616+
stackVal->IsString()) {
617+
stack = tns::ToString(isolate, stackVal.As<v8::String>());
618+
}
619+
}
620+
621+
// Fallback to v8::Exception::GetStackTrace on the exception value
622+
if (stack.empty()) {
623+
Local<StackTrace> v8Stack = v8::Exception::GetStackTrace(exception);
624+
if (!v8Stack.IsEmpty()) {
625+
int framesCount = v8Stack->GetFrameCount();
626+
std::stringstream ss;
627+
for (int i = 0; i < framesCount; i++) {
628+
Local<StackFrame> frame = v8Stack->GetFrame(isolate, i);
629+
ss << BuildStacktraceFrameMessage(isolate, frame) << std::endl;
630+
}
631+
stack = ss.str();
632+
}
633+
}
634+
}
635+
636+
// 2) TryCatch-provided stack if available
637+
if (stack.empty() && tryCatch != nullptr) {
638+
Local<Value> stackVal;
639+
if (tryCatch->StackTrace(context).ToLocal(&stackVal) && stackVal->IsString()) {
640+
stack = tns::ToString(isolate, stackVal.As<v8::String>());
641+
} else {
642+
Local<Message> message = tryCatch->Message();
643+
if (!message.IsEmpty()) {
644+
Local<StackTrace> v8Stack = message->GetStackTrace();
645+
if (!v8Stack.IsEmpty()) {
646+
int framesCount = v8Stack->GetFrameCount();
647+
std::stringstream ss;
648+
for (int i = 0; i < framesCount; i++) {
649+
Local<StackFrame> frame = v8Stack->GetFrame(isolate, i);
650+
ss << BuildStacktraceFrameMessage(isolate, frame) << std::endl;
651+
}
652+
stack = ss.str();
653+
}
654+
}
655+
}
656+
}
657+
658+
// 3) Finally, current stack if still empty
659+
if (stack.empty()) {
660+
stack = tns::GetStackTrace(isolate);
661+
}
662+
663+
return stack;
664+
}
665+
576666
const std::string tns::BuildStacktraceFrameLocationPart(Isolate* isolate, Local<StackFrame> frame) {
577667
std::stringstream ss;
578668

‎NativeScript/runtime/ModuleInternal.mm‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -724,6 +724,28 @@ throw NativeScriptException(isolate,
724724
}
725725
Log(@"***** End stack trace - continuing execution *****");
726726
Log(@"Debug mode - Script execution failed, returning gracefully: %s", path.c_str());
727+
728+
std::string errorTitle = "Uncaught JavaScript Exception";
729+
std::string errorMessage = "Error executing script.";
730+
731+
// Extract error message for modal when available
732+
if (tc.HasCaught()) {
733+
Local<Value> exception = tc.Exception();
734+
if (!exception.IsEmpty()) {
735+
Local<Context> ctx = isolate->GetCurrentContext();
736+
Local<v8::String> excStr;
737+
if (exception->ToString(ctx).ToLocal(&excStr)) {
738+
std::string excMsg = tns::ToString(isolate, excStr);
739+
if (!excMsg.empty()) {
740+
errorMessage = excMsg;
741+
}
742+
}
743+
}
744+
}
745+
746+
std::string stackTrace = tns::GetSmartStackTrace(isolate, &tc, tc.Exception());
747+
748+
NativeScriptException::ShowErrorModal(isolate, errorTitle, errorMessage, stackTrace);
727749
return Local<Value>();
728750
} else {
729751
if (tc.HasCaught()) {
@@ -1003,6 +1025,7 @@ ScriptOrigin origin(isolate, urlString, 0, 0, false, -1, Local<Value>(), false,
10031025

10041026
// Log the extracted error information
10051027
Log(@"NativeScript encountered a fatal error: %s", errorMessage.c_str());
1028+
// Reverted: do not remap before logging/displaying
10061029
if (!stackTrace.empty()) {
10071030
Log(@"JavaScript stack trace:\n%s", stackTrace.c_str());
10081031
}
@@ -1015,7 +1038,14 @@ ScriptOrigin origin(isolate, urlString, 0, 0, false, -1, Local<Value>(), false,
10151038

10161039
Log(@"***** End stack trace - Fix to continue *****");
10171040

1018-
NativeScriptException::ShowErrorModal(errorTitle, errorMessage, stackTrace);
1041+
// Ensure we have a stack for the modal
1042+
if (stackTrace.empty()) {
1043+
stackTrace = tns::GetSmartStackTrace(isolate);
1044+
} else {
1045+
stackTrace = tns::RemapStackTraceIfAvailable(isolate, stackTrace);
1046+
}
1047+
1048+
NativeScriptException::ShowErrorModal(isolate, errorTitle, errorMessage, stackTrace);
10191049

10201050
// In debug mode, don't throw any exceptions - just return empty value
10211051
return Local<Value>();

‎NativeScript/runtime/NativeScriptException.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,14 @@ class NativeScriptException {
2020
const std::string& getStackTrace() const { return stackTrace_; }
2121
static void OnUncaughtError(v8::Local<v8::Message> message,
2222
v8::Local<v8::Value> error);
23-
static void ShowErrorModal(const std::string& title,
23+
static void ShowErrorModal(v8::Isolate* isolate,
24+
const std::string& title,
2425
const std::string& message,
2526
const std::string& stackTrace);
27+
static void SubmitConsoleErrorPayload(v8::Isolate* isolate,
28+
const std::string& payload);
2629

2730
private:
28-
static void showErrorModalSynchronously(const std::string& title,
29-
const std::string& message,
30-
const std::string& stackTrace);
3131
v8::Persistent<v8::Value>* javascriptException_;
3232
std::string name_;
3333
std::string message_;

0 commit comments

Comments
 (0)