Don't crash on missing local files on Unix and Apple#28
Merged
bkaradzic-microsoft merged 3 commits intoMay 14, 2026
Conversation
Two pre-existing bugs caused a missing local `app:///` (or `file://`) target to crash the host process instead of producing a clean failure on the URL request: - **Unix** (`UrlRequest_Unix.cpp`): the worker `std::thread` calls `curl_check(curl_easy_perform(m_curl))`. `curl_check` throws `std::runtime_error` on any non-OK libcurl code -- including `CURLE_FILE_COULDNT_READ_FILE` (37) for a missing local file. The uncaught exception in the `std::thread` callable calls `std::terminate`. Wrap the libcurl invocation in try/catch and retain the default status code of 0 on failure. - **Apple** (`UrlRequest_Apple.mm`): `Open` throws synchronously with `"No file exists at local path"` when `[NSBundle pathForResource:]` returns nil for an `app:///` URL. This propagates back to the JS caller through `xhr.open()` rather than going through the normal async failure path. Set `m_url = nil` and let `SendAsync`'s existing `if (m_url == nil)` branch produce the same `status=0` + `loadend` outcome as the rest of the platforms. Both fixes now match the convention already in use by `UrlRequest_UWP.cpp` (`catch (winrt::hresult_error)` -> retain status 0) and `UrlRequest_Win32.cpp` (HTTP path returns `task_from_result<>()` on non-success status). Discovered while investigating BabylonJS/JsRuntimeHost#165 -- with that XHR fix in place, JS-side observers will now reliably receive an `error` event on missing local files instead of seeing the host crash. Co-authored-by: Copilot <[email protected]>
This was referenced May 13, 2026
There was a problem hiding this comment.
Pull request overview
This PR prevents host-process crashes when local app:/// / file:// requests target missing files on Unix and Apple platforms, aligning failure semantics with the library’s existing “client-side failure => status 0, async completion” convention.
Changes:
- Unix: Wrap
curl_easy_perform/curl_easy_getinfoin atry/catchinside the worker thread to preventstd::terminateon libcurl failures. - Apple: Stop throwing synchronously from
Open()whenNSBundlecan’t resolve anapp:///resource; instead defer failure toSendAsync()viam_url = nil.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Source/UrlRequest_Unix.cpp | Adds exception handling around libcurl calls in the async worker thread to avoid terminating on missing local files. |
| Source/UrlRequest_Apple.mm | Changes missing bundled resource handling to avoid synchronous throws and complete asynchronously with the existing error path. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
If a UrlRequest is reused (Open + Send + Open + Send) and the first request succeeded with a non-zero status, then the second request hits the new client-side-failure path (Unix worker exception, Apple missing `app:///` resource), the caller would observe the stale prior status instead of the intended `UrlStatusCode::None` (0). Reset the status at the start of `Open()` so client-side failures consistently report 0 regardless of prior history. Co-authored-by: Copilot <[email protected]>
Address @bghgary's cross-platform consistency concern on PR BabylonJS#28 (discussion_r3242603881). The previous commit reset only `m_statusCode`, and only on Unix and Apple -- creating a 2/5-platform 1/5-field inconsistency. Win32, UWP, and Android still relied on the implicit "construct a new `UrlRequest` per request" convention. Other per-request response state (`m_responseUrl`, `m_responseString`, `m_headers`, `m_responseBuffer`) was not reset on any platform. Add a protected `ImplBase::ResetForOpen()` helper that resets the response state living in the base class. Each platform's `Open()` calls it at the top, and additionally resets its own platform-specific response buffer (different types across backends: `std::vector<std::byte>` on Unix/Android, `NSData*` on Apple, `Storage::Streams::IBuffer` + `std::vector<std::byte>` on Win32/UWP). This makes `UrlRequest` properly reusable across all 5 platforms with consistent semantics: after `Open()` returns, the previous response is fully gone regardless of what happened before. Request-side caller-owned state (`m_requestBody`, `m_requestHeaders`) is intentionally not reset to preserve the existing pattern of setting headers/body between `Open()` and `SendAsync()`. Co-authored-by: Copilot <[email protected]>
This was referenced May 14, 2026
bkaradzic-microsoft
added a commit
that referenced
this pull request
May 14, 2026
…ON_DESKTOP (#29) Follow-up to #28: the unconditional `m_fileResponseBuffer.clear()` call added to `Open()` in `UrlRequest_Windows_Shared.h` references a member that only exists on Win32 desktop. UWP builds fail with: ``` UrlRequest_Windows_Shared.h(51,13): error C2065: 'm_fileResponseBuffer': undeclared identifier (compiling source file '../../_deps/urllib-src/Source/UrlRequest_UWP.cpp') ``` Spotted by [JsRuntimeHost #168](BabylonJS/JsRuntimeHost#168) CI (4 UWP build jobs failing: `UWP_x64_Chakra`, `UWP_x64_JSI`, `UWP_x64_V8`, `UWP_arm64_JSI`). The member declaration at line 150 of the same header is already gated by `#if WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_DESKTOP)`. The fix is to apply the same guard around the `clear()` call so it only fires on desktop. After this, `JsRuntimeHost #168` (UrlLib pin bump) goes green on UWP. Co-authored-by: Copilot <[email protected]> Co-authored-by: Copilot <[email protected]>
bkaradzic-microsoft
added a commit
to BabylonJS/JsRuntimeHost
that referenced
this pull request
May 14, 2026
Picks up [UrlLib #28](BabylonJS/UrlLib#28) so consumers (including BabylonNative) get the client-side-failure fixes: - `ImplBase::ResetForOpen()` clears `m_statusCode`, `m_responseUrl`, `m_responseString`, `m_headers` between `Open()` calls on all 5 platforms (Win32, UWP, Unix, Apple, Android). - Unix `PerformAsync` no longer lets `curl_check` exceptions escape `std::thread` -> `std::terminate`; falls back to `status=0 + loadend` like Win32/UWP. - Apple `Open` no longer throws synchronously when `[NSBundle pathForResource:]` returns nil for missing local files; sets `m_url = nil` and lets the existing `SendAsync` branch return `status=0`. This is a pin-bump only: no source changes in JsRuntimeHost itself. Co-authored-by: Copilot <[email protected]> Co-authored-by: Copilot <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Two pre-existing bugs caused a missing local
app:///(orfile://) URL request to crash the host process instead of producing a clean failure surfaceable to the consumer:Bug 1: Unix worker thread terminates on missing local file
Source/UrlRequest_Unix.cpp-- the workerstd::threadinPerformAsynccallscurl_check(curl_easy_perform(m_curl)).curl_checkthrowsstd::runtime_erroron any non-OK libcurl code -- includingCURLE_FILE_COULDNT_READ_FILE(37) for a missing local file. The uncaught exception in thestd::threadcallable callsstd::terminate.Reproduced by JsRuntimeHost CI: https://github.com/BabylonJS/JsRuntimeHost/actions/runs/25810440805
[log] should have status=404 for a file that does not exist (204ms) terminate called after throwing an instance of 'std::runtime_error' what(): CURL call failed with code (37) ./UnitTests: line 1: 3639 Aborted (core dumped)Bug 2: Apple
Openthrows synchronously on missingapp:///Source/UrlRequest_Apple.mm:39--Openthrowsstd::runtime_error{"No file exists at local path"}synchronously when[NSBundle pathForResource:]returns nil for anapp:///URL. This propagates back to the JS caller throughxhr.open()as a synchronous throw, breaking platform parity: every other platform defers the failure to the asyncSendAsyncpath withstatus=0 + complete.Fix
Make both platforms match the existing convention (already implemented by
UrlRequest_UWP.cppcatch (winrt::hresult_error)andUrlRequest_Win32.cppnon-success branch): retain the default status code of 0 on client-side failure, complete the task normally.Open, instead of throwing whenpathForResource:returns nil, setm_url = niland letSendAsync's existingif (m_url == nil)branch handle it.After these fixes, JS-side observers (e.g. via JsRuntimeHost's XMLHttpRequest polyfill + BabylonJS/JsRuntimeHost#165) will reliably receive an
errorevent on missing local files across all platforms instead of seeing the host crash.Tests
UrlLib doesn't have its own test suite. End-to-end verification will land in BabylonJS/JsRuntimeHost once both PR #165 (XHR async-failure events) and this UrlLib fix merge -- at that point a regression test for the
app:///missing-file case can be safely added to JsRuntimeHost'stests.ts(see this comment). Locally testable today via the BabylonNative Playground (any test exercising a missing local script/asset on Linux or macOS).cc @bghgary