Skip to content

Commit c96df81

Browse files
committed
fix(runtime): fail loud on debug-binding construction; contain fetch-thread throws
BuildNsModuleBinding returns false when the debug canonicalizeHttpUrlKey function cannot be constructed, instead of reporting the binding built with an exception pending and the member silently missing. RunModuleFetchJob no longer lets anything escape: it runs at the top of a detached thread, where an unwinding exception is std::terminate for the process and a throw past the caller's loop would strand the fetch-queue bookkeeping. An exception in the fetch phase becomes a transport-error result so the completion still runs exactly once; the completion call itself gets a log-only guard, since by then delivery has either happened or cannot be retried.
1 parent 037e878 commit c96df81

2 files changed

Lines changed: 67 additions & 30 deletions

File tree

test-app/runtime/src/main/cpp/HttpLoader.cpp

Lines changed: 61 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -898,40 +898,72 @@ std::deque<ModuleFetchJob> g_fetchQueue;
898898
size_t g_fetchThreadCount = 0;
899899

900900
void RunModuleFetchJob(const ModuleFetchJob& job) {
901-
std::string body;
902-
std::string contentType;
903-
int status = 0;
904-
const auto start = std::chrono::steady_clock::now();
905-
bool bustApplied = false;
906-
bool transportOk = PerformHttpFetchOnceSync(job.url, job.canonicalKey, body, contentType,
907-
status, bustApplied);
908-
if (!transportOk) {
909-
// Transport error → one retry, the same single-retry policy the
910-
// sync path applies.
911-
TNS_DEBUG(Esm, "[http-loader][fetch-async] retrying %s after transport error",
912-
job.url.c_str());
913-
usleep(120 * 1000);
914-
transportOk = PerformHttpFetchOnceSync(job.url, job.canonicalKey, body, contentType, status,
915-
bustApplied);
916-
}
917-
901+
// Nothing may escape this function: it runs at the top of a detached
902+
// thread, where an unwinding exception is std::terminate for the whole
903+
// process — and a throw past the caller's loop would also strand the
904+
// queue bookkeeping. The fetch phase converts an escaped exception into a
905+
// transport-error result so the completion still runs exactly once; the
906+
// completion itself gets a log-only guard, since by then delivery has
907+
// either happened or cannot be retried.
918908
ModuleFetchResult result;
919-
ClassifyModuleResponse(job.url, transportOk, status, contentType, body, result);
909+
try {
910+
std::string body;
911+
std::string contentType;
912+
int status = 0;
913+
const auto start = std::chrono::steady_clock::now();
914+
bool bustApplied = false;
915+
bool transportOk = PerformHttpFetchOnceSync(job.url, job.canonicalKey, body, contentType,
916+
status, bustApplied);
917+
if (!transportOk) {
918+
// Transport error → one retry, the same single-retry policy the
919+
// sync path applies.
920+
TNS_DEBUG(Esm, "[http-loader][fetch-async] retrying %s after transport error",
921+
job.url.c_str());
922+
usleep(120 * 1000);
923+
transportOk = PerformHttpFetchOnceSync(job.url, job.canonicalKey, body, contentType,
924+
status, bustApplied);
925+
}
920926

921-
if (result.ok && bustApplied) {
922-
ClearCacheBustForUrl(job.canonicalKey);
927+
ClassifyModuleResponse(job.url, transportOk, status, contentType, body, result);
928+
929+
if (result.ok && bustApplied) {
930+
ClearCacheBustForUrl(job.canonicalKey);
931+
}
932+
933+
if (!result.ok) {
934+
TNS_DEBUG(Esm, "[http-loader][fetch-async][reject] %s", result.failureReason.c_str());
935+
} else if (LogCategoryEnabled(LogCategory::Fetch)) {
936+
const auto ms = std::chrono::duration_cast<std::chrono::milliseconds>(
937+
std::chrono::steady_clock::now() - start)
938+
.count();
939+
TNS_DEBUG(Fetch, "[http-loader][fetch][async] %s bytes=%lu ms=%lld", job.url.c_str(),
940+
(unsigned long)result.body.size(), (long long)ms);
941+
}
942+
} catch (NativeScriptException& e) {
943+
result = ModuleFetchResult{};
944+
result.failureReason = "HTTP import failed: " + job.url + " (network error)";
945+
DEBUG_WRITE_FORCE("[http-loader][fetch-async] native exception fetching %s: %s",
946+
job.url.c_str(), e.what());
947+
} catch (const std::exception& e) {
948+
result = ModuleFetchResult{};
949+
result.failureReason = "HTTP import failed: " + job.url + " (network error)";
950+
DEBUG_WRITE_FORCE("[http-loader][fetch-async] c++ exception fetching %s: %s",
951+
job.url.c_str(), e.what());
952+
} catch (...) {
953+
result = ModuleFetchResult{};
954+
result.failureReason = "HTTP import failed: " + job.url + " (network error)";
955+
DEBUG_WRITE_FORCE("[http-loader][fetch-async] unknown exception fetching %s",
956+
job.url.c_str());
923957
}
924958

925-
if (!result.ok) {
926-
TNS_DEBUG(Esm, "[http-loader][fetch-async][reject] %s", result.failureReason.c_str());
927-
} else if (LogCategoryEnabled(LogCategory::Fetch)) {
928-
const auto ms = std::chrono::duration_cast<std::chrono::milliseconds>(
929-
std::chrono::steady_clock::now() - start)
930-
.count();
931-
TNS_DEBUG(Fetch, "[http-loader][fetch][async] %s bytes=%lu ms=%lld", job.url.c_str(),
932-
(unsigned long)result.body.size(), (long long)ms);
959+
try {
960+
job.completion(std::move(result));
961+
} catch (const std::exception& e) {
962+
DEBUG_WRITE_FORCE("[http-loader][fetch-async] completion threw for %s: %s",
963+
job.url.c_str(), e.what());
964+
} catch (...) {
965+
DEBUG_WRITE_FORCE("[http-loader][fetch-async] completion threw for %s", job.url.c_str());
933966
}
934-
job.completion(std::move(result));
935967
}
936968

937969
// A fetch thread serves its own job and then drains the queue, so the JVM

test-app/runtime/src/main/cpp/ModuleInternalCallbacks.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3766,7 +3766,12 @@ bool BuildNsModuleBinding(v8::Local<v8::Context> context, v8::Local<v8::Object>
37663766
}
37673767
};
37683768
v8::Local<v8::Function> fn;
3769-
if (v8::Function::New(context, canonicalizeCb).ToLocal(&fn)) {
3769+
if (!v8::Function::New(context, canonicalizeCb).ToLocal(&fn)) {
3770+
// The member is absent-by-design in release, but a debug build
3771+
// must not report the binding built with a pending exception.
3772+
return false;
3773+
}
3774+
{
37703775
fn->SetName(ToV8String(isolate, "canonicalizeHttpUrlKey"));
37713776
if (!binding
37723777
->CreateDataProperty(context, ToV8String(isolate, "canonicalizeHttpUrlKey"),

0 commit comments

Comments
 (0)