From 9ecb94fa71154fa61ec044dda235d8aa9841f55c Mon Sep 17 00:00:00 2001 From: Nathan Walker Date: Tue, 11 Aug 2026 10:55:35 -0700 Subject: [PATCH 1/7] feat(runtime): ESM resolver hardening and async module-graph loader MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Canonicalize module identity into three registry shapes — http(s) URLs, custom schemes (node:, blob:, optional:), and absolute file paths — and key the module registries by v8::Isolate instead of thread_local storage. import() now rejects missing bare specifiers instead of installing placeholders; optional-module placeholders are built without string interpolation, detection is unified in IsLikelyOptionalModule, and module source preserves embedded NUL bytes. Thenables handed to the loader from JS are adopted properly. Blob URLs (blob:nativescript/) become first-class module identities via URL.createObjectURL and URL.InternalAccessor. The prewarm/prefetch machinery is replaced by an async module-graph loader; boot hands off to a manual runloop that pumps pending module work when the entry script has not reached UIApplicationMain yet (e.g. a top-level-await entry still loading its graph). RunModule surfaces the failure cause to callers through an error out-parameter. --- NativeScript/NativeScript.mm | 18 + NativeScript/runtime/ModuleInternal.h | 12 +- NativeScript/runtime/ModuleInternal.mm | 878 ++++- .../runtime/ModuleInternalCallbacks.h | 96 +- .../runtime/ModuleInternalCallbacks.mm | 2975 ++++++++++++----- NativeScript/runtime/Runtime.h | 5 +- NativeScript/runtime/Runtime.mm | 140 +- 7 files changed, 3113 insertions(+), 1011 deletions(-) diff --git a/NativeScript/NativeScript.mm b/NativeScript/NativeScript.mm index caf37186..1ed91eb9 100644 --- a/NativeScript/NativeScript.mm +++ b/NativeScript/NativeScript.mm @@ -3,6 +3,7 @@ #include "inspector/JsV8InspectorClient.h" #include "runtime/Console.h" #include "runtime/Helpers.h" +#include "runtime/ModuleInternalCallbacks.h" #include "runtime/Runtime.h" #include "runtime/RuntimeConfig.h" #include "runtime/Tasks.h" @@ -43,6 +44,23 @@ - (void)runMainApplication { CFRunLoopRunInMode(kCFRunLoopDefaultMode, 0, true); tns::Tasks::Drain(); + + // Async-pipeline boot handoff. For UI apps Tasks::Drain() invokes + // UIApplicationMain and never returns — the app's main runloop services + // any in-flight async module loads. When Drain returns (the entry never + // called UIApplicationMain — e.g. a top-level-await entry still loading + // its graph), pump a manual runloop until the pending module work + // settles, Node-like. A load completion may itself register the + // UIApplicationMain task, so drain after each slice; if that drain calls + // UIApplicationMain, it takes over from here and never returns. + if (tns::HasPendingAsyncModuleGraphWork()) { + const CFAbsoluteTime deadline = CFAbsoluteTimeGetCurrent() + 120.0; + while (tns::HasPendingAsyncModuleGraphWork() && CFAbsoluteTimeGetCurrent() < deadline) { + CFRunLoopRunInMode(kCFRunLoopDefaultMode, 0.01, true); + tns::Tasks::Drain(); + } + tns::Tasks::Drain(); + } } - (bool)liveSync { diff --git a/NativeScript/runtime/ModuleInternal.h b/NativeScript/runtime/ModuleInternal.h index 1b835979..d768049f 100644 --- a/NativeScript/runtime/ModuleInternal.h +++ b/NativeScript/runtime/ModuleInternal.h @@ -9,7 +9,14 @@ namespace tns { class ModuleInternal { public: ModuleInternal(v8::Local context); - bool RunModule(v8::Isolate* isolate, std::string path); + // When `outErrorMessage` is non-null, the failure cause is written into + // it on a false return: `NativeScriptException::getMessage()` for + // thrown exceptions, the V8 exception text for require() failures, the + // top-level-await rejection/timeout reason for ES modules, or a + // directional hint when the module returned an empty namespace without + // throwing. + bool RunModule(v8::Isolate* isolate, std::string path, + std::string* outErrorMessage = nullptr); void RunScript(v8::Isolate* isolate, std::string script); static v8::Local LoadScript(v8::Isolate* isolate, const std::string& path); @@ -39,6 +46,9 @@ class ModuleInternal { const std::string& moduleName); std::string ResolvePathFromPackageJson(const std::string& packageJson, bool& error); + v8::Local CreatePlaceholderModule(v8::Isolate* isolate, + const std::string& moduleName, + const std::string& cacheKey); static v8::ScriptCompiler::CachedData* LoadScriptCache( const std::string& path); static void SaveScriptCache(const v8::Local script, diff --git a/NativeScript/runtime/ModuleInternal.mm b/NativeScript/runtime/ModuleInternal.mm index f7cb2820..687a5c08 100644 --- a/NativeScript/runtime/ModuleInternal.mm +++ b/NativeScript/runtime/ModuleInternal.mm @@ -4,10 +4,12 @@ #include #include #include +#include #include #include "BuiltinLoader.h" #include "Caches.h" #include "DevFlags.h" +#include "HMRSupport.h" #include "Helpers.h" #include "ModuleInternalCallbacks.h" // for ResolveModuleCallback #include "NativeScriptException.h" @@ -20,6 +22,46 @@ namespace tns { +// require()-path policy only: import() rejects a missing bare specifier outright +// (ESM optionality is `try { await import(x) } catch {}` at the call site, and in +// dev sessions a bare specifier the import map doesn't cover is a config bug that +// must fail loudly — see docs/knowledge/hmr-simplification-pass.md §2). +static bool IsLikelyOptionalModule(const std::string& moduleName) { + // Node built-ins are handled by their own dedicated resolution path; never treat them as + // an optional external module. + if (moduleName.rfind("node:", 0) == 0) { + return false; + } + + // Check if it's a bare module name (no path separators) that could be an npm package. + // + // Bare specifiers that end in a recognizable script/data extension (e.g. "foo.js", + // "config.json") are explicit file references, not npm-style package names — real npm + // package names don't carry a file extension. Treating them as "likely optional" would + // swallow a genuine "module not found" failure behind a lazily-throwing placeholder + // instead of letting require()/import() fail immediately, which is what callers (and the + // existing "should throw error if cant find node module" test) expect for those names. + // + // This carve-out is deliberately narrow: a dotted bare name that doesn't end in one of + // these exact extensions (e.g. "lodash.debounce") is still treated as optional, same as + // before. See ModuleInternal.mm/ModuleInternalCallbacks.mm optional-module tests for the + // cases this boundary is expected to hold for. + static const char* kExplicitFileExtensions[] = {".js", ".mjs", ".cjs", ".json", ".node", ".ts"}; + + if (moduleName.find('/') == std::string::npos && moduleName.find('\\') == std::string::npos && + moduleName[0] != '.' && moduleName[0] != '~' && moduleName[0] != '/') { + for (const char* ext : kExplicitFileExtensions) { + size_t extLen = strlen(ext); + if (moduleName.size() > extLen && + moduleName.compare(moduleName.size() - extLen, extLen, ext) == 0) { + return false; + } + } + return true; + } + return false; +} + // Helper function to check if a file path is an ES module (.mjs) but not a source map (.mjs.map) bool IsESModule(const std::string& path) { return path.size() >= 4 && path.compare(path.size() - 4, 4, ".mjs") == 0 && @@ -37,6 +79,45 @@ static bool IsBareSpecifier(const std::string& specifier) { return specifier.find(':') == std::string::npos; } +static std::string NormalizePath(const std::string& path); + +static inline bool StartsWith(const std::string& value, const char* prefix) { + size_t n = strlen(prefix); + return value.size() >= n && value.compare(0, n, prefix) == 0; +} + +static std::string NormalizeHttpModuleUrl(const std::string& path) { + if (path.empty()) { + return path; + } + + std::string normalized = path; + if (StartsWith(normalized, "file://http://") || StartsWith(normalized, "file://https://")) { + normalized = normalized.substr(strlen("file://")); + } + + if (normalized.rfind("http:/", 0) == 0 && normalized.rfind("http://", 0) != 0) { + normalized.insert(5, "/"); + } else if (normalized.rfind("https:/", 0) == 0 && normalized.rfind("https://", 0) != 0) { + normalized.insert(6, "/"); + } + + return normalized; +} + +static bool IsHttpModulePath(const std::string& path) { + std::string normalized = NormalizeHttpModuleUrl(path); + return StartsWith(normalized, "http://") || StartsWith(normalized, "https://"); +} + +static std::string CanonicalizeModulePath(const std::string& path) { + if (IsHttpModulePath(path)) { + return CanonicalizeHttpUrlKey(NormalizeHttpModuleUrl(path)); + } + + return NormalizePath(path); +} + // Normalize file system paths to a canonical representation so lookups in // g_moduleRegistry remain consistent regardless of how the path was provided. static std::string NormalizePath(const std::string& path) { @@ -129,10 +210,21 @@ static bool IsBareSpecifier(const std::string& specifier) { } } -bool ModuleInternal::RunModule(Isolate* isolate, std::string path) { +// Forward `message` into the caller's optional out-param. The caller +// is responsible for any "missing message" presentation; this helper +// writes the raw value (which may be empty) when an out-param was +// supplied, and is a no-op otherwise. +static inline void SetOutErrorMessage(std::string* outErrorMessage, const std::string& message) { + if (outErrorMessage != nullptr) { + *outErrorMessage = message; + } +} + +bool ModuleInternal::RunModule(Isolate* isolate, std::string path, std::string* outErrorMessage) { std::shared_ptr cache = Caches::Get(isolate); Local context = cache->GetContext(); Local globalObject = context->Global(); + bool isHttpModule = IsHttpModulePath(path); // Ensure global.__dirname is defined so ESM/CommonJS shims relying on it work. { Local dirVal; @@ -149,20 +241,63 @@ static bool IsBareSpecifier(const std::string& specifier) { } // ES module fast path - if (IsESModule(path)) { + if (IsESModule(path) || isHttpModule) { TryCatch tc(isolate); Local moduleNamespace; + if (isHttpModule && RuntimeConfig.IsDebug && IsScriptLoadingLogEnabled()) { + Log(@"[run-module][http-esm][begin] %s", NormalizeHttpModuleUrl(path).c_str()); + } try { moduleNamespace = ModuleInternal::LoadESModule(isolate, path); - } catch (NativeScriptException& ex) { - if (RuntimeConfig.IsDebug) { + } catch (const NativeScriptException& ex) { + if (isHttpModule && RuntimeConfig.IsDebug && IsScriptLoadingLogEnabled()) { + Log(@"[run-module][http-esm][exception] %s message=%s", + NormalizeHttpModuleUrl(path).c_str(), ex.getMessage().c_str()); + } + if (RuntimeConfig.IsDebug && !isHttpModule) { + Log(@"***** JavaScript exception occurred - detailed stack trace follows *****"); Log(@"Error loading ES module: %s", path.c_str()); Log(@"Exception: %s", ex.getMessage().c_str()); + Log(@"***** End stack trace - continuing execution *****"); + Log(@"Debug mode - ES module loading failed, but telling iOS it succeeded to prevent app " + @"termination"); + return true; // avoid termination in debug + } else { + // Surface the inner exception's message so callers passing + // `outErrorMessage` see the real cause instead of just a + // false return. + SetOutErrorMessage(outErrorMessage, ex.getMessage()); + return false; } - ex.ReThrowToV8(isolate); - return false; } - return true; + if (moduleNamespace.IsEmpty()) { + if (isHttpModule && RuntimeConfig.IsDebug && IsScriptLoadingLogEnabled()) { + Log(@"[run-module][http-esm][empty] %s", NormalizeHttpModuleUrl(path).c_str()); + } + if (RuntimeConfig.IsDebug && !isHttpModule) { + Log(@"Debug mode - ES module returned empty namespace, but telling iOS it succeeded"); + return true; + } else { + // `LoadESModule` returned an empty value without throwing — + // typically a HTTP TLA timeout / rejection swallowed by the + // debug-modal path. Provide a directional hint so the JS + // rejection isn't empty; this is the only case where we + // *don't* have the actual reason text (see the rejection + // throw additions in `LoadESModule` to surface real causes + // when possible). + SetOutErrorMessage(outErrorMessage, + std::string("ES module returned empty namespace for ") + path + + " — likely top-level await timeout or rejection swallowed by " + "debug error modal; check the device console for the matching " + "[esm][evaluate][promise-rejected:detail] or " + "[esm][evaluate][promise-timeout] entry."); + return false; + } + } + if (isHttpModule && RuntimeConfig.IsDebug && IsScriptLoadingLogEnabled()) { + Log(@"[run-module][http-esm][ok] %s", NormalizeHttpModuleUrl(path).c_str()); + } + return true; // ES module loaded successfully } // For CommonJS modules (.js), use the traditional require() approach @@ -170,6 +305,7 @@ static bool IsBareSpecifier(const std::string& specifier) { bool success = globalObject->Get(context, ToV8String(isolate, "require")).ToLocal(&requireObj); if (!success || !requireObj->IsFunction()) { Log(@"Warning: Failed to get require function from global object"); + SetOutErrorMessage(outErrorMessage, "require function unavailable on globalThis"); return false; } Local requireFunc = requireObj.As(); @@ -181,18 +317,59 @@ static bool IsBareSpecifier(const std::string& specifier) { success = requireFunc->Call(context, globalObject, 1, args).ToLocal(&result); if (!success || tc.HasCaught()) { - if (RuntimeConfig.IsDebug) { + // Main isolate stays alive in debug for HMR; worker isolates must surface + // the failure so `worker.onerror` fires (handled in the else branch). + if (RuntimeConfig.IsDebug && !cache->isWorker) { + Log(@"***** JavaScript exception occurred - detailed stack trace follows *****"); Log(@"Error in require() call:"); Log(@" Requested module: '%s'", path.c_str()); Log(@" Called from: %s", RuntimeConfig.ApplicationPath.c_str()); + if (tc.HasCaught()) { tns::LogError(isolate, tc); } + + Log(@"***** End stack trace - continuing execution *****"); + Log(@"Debug mode - Main script execution failed, but telling iOS it succeeded to prevent " + @"app termination"); + + // Add a small delay to ensure error modal has time to render before we return + dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(0.3 * NSEC_PER_SEC)), + dispatch_get_main_queue(), ^{ + Log(@"🛡️ Debug mode - Crash prevention complete, app should remain stable"); + }); + + return true; // LIE TO iOS - return success to prevent app termination + } else { + // Best-effort extract the V8 exception text so the rejection + // upstream isn't empty. Leaves the out-param empty when the + // TryCatch has no exception to stringify; callers that need a + // placeholder string are expected to substitute one themselves. + std::string requireFailureMessage; + if (tc.HasCaught()) { + Local ex = tc.Exception(); + if (!ex.IsEmpty()) { + v8::Local exStr; + if (ex->ToString(context).ToLocal(&exStr)) { + v8::String::Utf8Value utf8(isolate, exStr); + if (*utf8) { + requireFailureMessage.assign(*utf8, utf8.length()); + } + } + } + } + if (requireFailureMessage.empty()) { + requireFailureMessage = std::string("require() failed for module ") + path; + } + SetOutErrorMessage(outErrorMessage, requireFailureMessage); + // For worker isolates, keep the V8 exception pending so the worker entry's + // TryCatch (Worker.mm) catches it and routes it to worker.onerror. The + // main isolate's release path is unchanged (no rethrow). + if (cache->isWorker && tc.HasCaught()) { + tc.ReThrow(); + } + return false; } - if (tc.HasCaught()) { - tc.ReThrow(); - } - return false; } return success; @@ -212,13 +389,33 @@ static bool IsBareSpecifier(const std::string& specifier) { bool success = requireFuncFactory->Call(context, thiz, 2, args).ToLocal(&result); if (!success || tc.HasCaught()) { if (tc.HasCaught()) { - throw NativeScriptException(isolate, tc, "Failed to call require factory function"); + tns::LogError(isolate, tc); } - throw NativeScriptException(isolate, "Failed to call require factory function"); + Log(@"FATAL: Failed to call require factory function"); + // Return a dummy function to avoid further crashes + result = v8::Function::New(context, [](const v8::FunctionCallbackInfo& info) { + if (RuntimeConfig.IsDebug) { + Log(@"Debug mode - Require function unavailable (factory failed)"); + info.GetReturnValue().SetUndefined(); + } else { + info.GetIsolate()->ThrowException(v8::Exception::Error( + tns::ToV8String(info.GetIsolate(), "Require function unavailable"))); + } + }).ToLocalChecked(); } if (result.IsEmpty() || !result->IsFunction()) { - throw NativeScriptException(isolate, "Require factory did not return a function"); + Log(@"FATAL: Require factory did not return a function"); + // Return a dummy function + result = v8::Function::New(context, [](const v8::FunctionCallbackInfo& info) { + if (RuntimeConfig.IsDebug) { + Log(@"Debug mode - Require function unavailable (no function returned)"); + info.GetReturnValue().SetUndefined(); + } else { + info.GetIsolate()->ThrowException(v8::Exception::Error( + tns::ToV8String(info.GetIsolate(), "Require function unavailable"))); + } + }).ToLocalChecked(); } return result.As(); @@ -427,6 +624,14 @@ static bool IsBareSpecifier(const std::string& specifier) { } if (path.empty()) { + // A bare specifier shaped like an npm package name resolves to a + // lazily-throwing placeholder, so an app can ship without an optional + // dependency installed and only fail if it actually touches it. A + // specifier resolved against "/" is an explicit absolute path, never an + // optional package, so it always hard-fails. + if (baseDir != "/" && IsLikelyOptionalModule(moduleName)) { + return this->CreatePlaceholderModule(isolate, moduleName, cacheKey); + } throw NativeScriptException(isolate, "Cannot find module '" + moduleName + "'", "Error"); } @@ -484,13 +689,62 @@ static bool IsBareSpecifier(const std::string& specifier) { // Compile/load the JavaScript/ESM source Local scriptValue = LoadScript(isolate, modulePath); + // Check if script loading failed (debug mode graceful returns) + if (scriptValue.IsEmpty()) { + if (RuntimeConfig.IsDebug) { + // NSLog(@"Debug mode - Script loading returned empty value, returning gracefully: %s", + // modulePath.c_str()); + return Local(); + } else { + throw NativeScriptException(isolate, "Script loading failed for " + modulePath); + } + } + // Check if this is an ES module bool isESM = IsESModule(modulePath); std::shared_ptr cache = Caches::Get(isolate); if (isESM) { + // For ES modules, the returned value is the namespace object + + // First check if scriptValue is empty (from debug mode graceful returns) + if (scriptValue.IsEmpty()) { + if (RuntimeConfig.IsDebug) { + Log(@"Debug mode - ES module returned empty value, returning gracefully: %s", + modulePath.c_str()); + return Local(); + } else { + throw NativeScriptException(isolate, "ES module load returned empty value " + modulePath); + } + } + if (!scriptValue->IsObject()) { - throw NativeScriptException(isolate, "Failed to load ES module " + modulePath); + if (RuntimeConfig.IsDebug) { + Log(@"Debug mode - ES module load failed, returning gracefully: %s", modulePath.c_str()); + // Return empty module object to prevent crashes + return Local(); + } else { + throw NativeScriptException(isolate, "Failed to load ES module " + modulePath); + } + } + + // Debug: Check if we're in a worker context and if self.onmessage is set + std::shared_ptr cache = Caches::Get(isolate); + if (cache->isWorker) { + Local context = isolate->GetCurrentContext(); + Local global = context->Global(); + + // Check if self exists + Local selfValue; + if (global->Get(context, ToV8String(isolate, "self")).ToLocal(&selfValue)) { + if (selfValue->IsObject()) { + Local selfObj = selfValue.As(); + Local onmessageValue; + if (selfObj->Get(context, ToV8String(isolate, "onmessage")).ToLocal(&onmessageValue)) { + // onmessage exists + } + } + } } // Handle exports differently for ES modules vs worker scripts @@ -618,19 +872,63 @@ throw NativeScriptException(isolate, Local