XMLHttpRequest: report URL-open failures asynchronously from send

Polyfills/XMLHttpRequest/Readme.md

@@ -15,17 +15,29 @@ Unlike the web, XMLHttpRequest supports loading local files using two schemes:
 ## Other things to be aware of:
 * Only `GET` requests are currently supported
 * For `readyState`, we only support `UNSENT`, `OPENED`, and `DONE`
+* If the platform transport rejects a URL during `open()`, the request still
+  enters `OPENED`. Calling `send()` reports `DONE`, `error`, and `loadend`
+  asynchronously, with `status === 0`. This lets asset loaders handle unsupported
+  or scheme-less Native URLs through their error callbacks rather than aborting
+  scene parsing. No document-relative URL resolution is added.
+* Invalid methods, arguments, and unsupported request-body types still throw
+  synchronously. A deferred URL-open failure exposes `errorCode === "UrlOpenFailed"`
+  and the original error in `errorDetail`; reopening clears those diagnostics.
+  Aborting its pending notification returns `readyState` to `UNSENT` without
+  dispatching failure events; a new `open()` is required before another `send()`.
 
 ## Transport-error diagnostics (non-standard)
 A transport-level failure surfaces the standard way -- an `error` event followed by `loadend`,
 with `status === 0` -- exactly as on the web. In addition, two **non-standard, additive**
 read-only properties expose the normalized `UrlLib` transport-error detail so BN-aware code can
 tell a DNS failure from a refused connection or a missing local asset:
-* `errorCode` -- the stable symbolic token (e.g. `"CURLE_COULDNT_CONNECT"`, `"NSURLErrorTimedOut"`,
+* `errorCode` -- the stable symbolic token (e.g. `"UrlOpenFailed"`, `"CURLE_COULDNT_CONNECT"`, `"NSURLErrorTimedOut"`,
   `"AppResourceNotFound"`)
-* `errorDetail` -- the full normalized `"<domain>:<symbol>(<code>): <detail>"` string
+* `errorDetail` -- the original opening error for `UrlOpenFailed`, or the normalized
+  `"<domain>:<symbol>(<code>): <detail>"` string for a failure during `send()`
 
-Both are empty strings unless the request failed at the transport layer, and are populated only
-on backends that expose the detail (Apple, Linux) -- empty on Windows/Android until those
-backends populate `UrlLib`'s accessors. Browsers do not expose these properties, so
-spec-conformant code is unaffected.
+URL-opening errors populate both properties on every platform. Failures during
+`send()` expose the diagnostics supplied by the platform's `UrlLib` backend;
+those strings can be empty when the backend has no detail. Successful requests
+leave both properties empty, and reopening clears an earlier opening error.
+Browsers do not expose these properties, so spec-conformant code is unaffected.

Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp

@@ -2,6 +2,7 @@
 #include <Babylon/JsRuntime.h>
 #include <Babylon/Polyfills/XMLHttpRequest.h>
 #include <arcana/tracing/trace_region.h>
+#include <gsl/gsl>
 #include <cstring>
 #include <sstream>
 
@@ -179,14 +180,14 @@ namespace Babylon::Polyfills::Internal
     {
         // Stable symbolic token for a transport failure (e.g. "CURLE_COULDNT_CONNECT",
         // "NSURLErrorTimedOut", "AppResourceNotFound"); empty when there was no transport failure.
-        return Napi::String::New(Env(), std::string{m_request.ErrorSymbol()});
+        return Napi::String::New(Env(), m_openError ? "UrlOpenFailed" : std::string{m_request.ErrorSymbol()});
     }
 
     Napi::Value XMLHttpRequest::GetErrorDetail(const Napi::CallbackInfo&)
     {
-        // Full normalized "<domain>:<symbol>(<code>): <detail>" string; empty when there was no
-        // transport failure.
-        return Napi::String::New(Env(), std::string{m_request.ErrorString()});
+        // Original opening error for UrlOpenFailed, or normalized
+        // "<domain>:<symbol>(<code>): <detail>" for a send failure; empty on success.
+        return Napi::String::New(Env(), m_openError.value_or(std::string{m_request.ErrorString()}));
     }
 
     Napi::Value XMLHttpRequest::GetResponseHeader(const Napi::CallbackInfo& info)
@@ -254,24 +255,42 @@ namespace Babylon::Polyfills::Internal
 
     void XMLHttpRequest::Abort(const Napi::CallbackInfo&)
     {
+        ++m_requestGeneration;
         m_request.Abort();
+        if (m_openErrorSent && m_readyState == ReadyState::Opened)
+        {
+            m_readyState = ReadyState::Unsent;
+        }
     }
 
     void XMLHttpRequest::Open(const Napi::CallbackInfo& info)
     {
+        UrlLib::UrlMethod method;
+        try
+        {
+            method = MethodType::StringToEnum(info[0].As<Napi::String>().Utf8Value());
+        }
+        catch (const std::exception& e)
+        {
+            throw Napi::Error::New(info.Env(), e.what());
+        }
+
         m_url = info[1].As<Napi::String>();
+        m_openGeneration = ++m_requestGeneration;
+        m_openError.reset();
+        m_openErrorSent = false;
 
         try
         {
-            m_request.Open(MethodType::StringToEnum(info[0].As<Napi::String>().Utf8Value()), m_url);
+            m_request.Open(method, m_url);
         }
         catch (const std::exception& e)
         {
-            throw Napi::Error::New(info.Env(), std::string{"Error opening URL: "} + e.what());
+            m_openError = std::string{"Error opening URL: "} + e.what();
         }
         catch (...)
         {
-            throw Napi::Error::New(info.Env(), "Unknown error opening URL");
+            m_openError = "Unknown error opening URL";
         }
 
         SetReadyState(ReadyState::Opened);
@@ -284,6 +303,11 @@ namespace Babylon::Polyfills::Internal
             throw Napi::Error::New(info.Env(), "XMLHttpRequest must be opened before it can be sent");
         }
 
+        if (m_openErrorSent)
+        {
+            throw Napi::Error::New(info.Env(), "XMLHttpRequest has already been sent");
+        }
+
         if (info.Length() > 0)
         {
             if (!info[0].IsString() && !info[0].IsUndefined() && !info[0].IsNull())
@@ -297,6 +321,38 @@ namespace Babylon::Polyfills::Internal
             }
         }
 
+        if (m_openError)
+        {
+            m_openErrorSent = true;
+            auto anchor = std::make_shared<Napi::ObjectReference>(Napi::Persistent(info.This().As<Napi::Object>()));
+            arcana::make_task(m_runtimeScheduler, arcana::cancellation::none(),
+                [this, anchor{std::move(anchor)}, generation{m_requestGeneration}, openGeneration{m_openGeneration}]() {
+                    // Release after dispatch unwinds, without clearing a reopened request's listeners.
+                    const auto releaseListeners = gsl::finally([this, openGeneration]() {
+                        if (m_openGeneration == openGeneration)
+                        {
+                            m_eventHandlerRefs.clear();
+                        }
+                    });
+                    if (generation != m_requestGeneration)
+                    {
+                        return;
+                    }
+
+                    // Match an asynchronous transport failure, unless a callback reopens or aborts the request.
+                    m_readyState = ReadyState::Done;
+                    for (const auto event : {EventType::ReadyStateChange, EventType::Error, EventType::LoadEnd})
+                    {
+                        if (generation != m_requestGeneration)
+                        {
+                            return;
+                        }
+                        RaiseEvent(event);
+                    }
+                });
+            return;
+        }
+
         std::string traceName = (std::ostringstream{} << "XMLHttpRequest::Send [" << m_url << "]").str();
         auto sendRegion = std::make_optional<arcana::trace_region>(traceName.c_str());
 

Polyfills/XMLHttpRequest/Source/XMLHttpRequest.h

@@ -6,6 +6,8 @@
 #include <UrlLib/UrlLib.h>
 
 #include <unordered_map>
+#include <cstdint>
+#include <optional>
 #include <vector>
 
 namespace Babylon::Polyfills::Internal
@@ -49,6 +51,10 @@ namespace Babylon::Polyfills::Internal
         void RaiseEvent(const char* eventType);
 
         std::string m_url{};
+        std::optional<std::string> m_openError{};
+        uint64_t m_requestGeneration{};
+        uint64_t m_openGeneration{};
+        bool m_openErrorSent{};
         UrlLib::UrlRequest m_request{};
         JsRuntimeScheduler m_runtimeScheduler;
         ReadyState m_readyState{ReadyState::Unsent};

Tests/UnitTests/Source/Scripts/tests.xmlHttpRequest.ts

@@ -104,23 +104,146 @@ describe("XMLHTTPRequest", function () {
         expect(xhr.errorDetail).to.equal("");
     });
 
-    it("should throw something when opening //", async function () {
-        function openDoubleSlash() {
-            const xhr = new XMLHttpRequest();
-            xhr.open("GET", "//");
+    for (const url of ["//", "noscheme.glb"]) {
+        it(`should report a URL-open failure asynchronously for ${url}`, async function () {
+            this.timeout(5000);
+            const xhr = new XMLHttpRequest() as XMLHttpRequest & { errorCode: string; errorDetail: string };
+            const events: string[] = [];
+            xhr.addEventListener("readystatechange", () => events.push(`state:${xhr.readyState}`));
+            xhr.open("GET", url);
+            expect(xhr.readyState).to.equal(XMLHttpRequest.OPENED);
+            expect(events).to.deep.equal(["state:1"]);
             xhr.send();
-        }
-        expect(openDoubleSlash).to.throw();
+            expect(() => xhr.send()).to.throw();
+            expect(events).to.deep.equal(["state:1"]);
+            await new Promise<void>((resolve) => {
+                xhr.addEventListener("error", () => events.push("error"));
+                xhr.addEventListener("loadend", () => {
+                    events.push("loadend");
+                    resolve();
+                });
+            });
+            expect(events).to.deep.equal(["state:1", "state:4", "error", "loadend"]);
+            expect(xhr.status).to.equal(0);
+            expect(xhr.statusText).to.equal("");
+            expect(xhr.responseText).to.equal("");
+            expect(xhr.errorCode).to.equal("UrlOpenFailed");
+            expect(xhr.errorDetail).to.contain("Error opening URL:");
+        });
+    }
+
+    it("should still reject an unsupported method synchronously", function () {
+        const xhr = new XMLHttpRequest();
+        expect(() => xhr.open("INVALID", "noscheme.glb")).to.throw();
+        expect(xhr.readyState).to.equal(XMLHttpRequest.UNSENT);
+    });
+
+    it("should still reject an unsupported body after a URL-open failure", async function () {
+        this.timeout(5000);
+        const xhr = new XMLHttpRequest();
+        xhr.open("GET", "noscheme.glb");
+        expect(() => xhr.send(new Uint8Array(1))).to.throw();
+        const completed = new Promise<void>((resolve) => xhr.addEventListener("loadend", () => resolve()));
+        xhr.send();
+        await completed;
+        expect(xhr.status).to.equal(0);
+    });
+
+    it("should discard a pending URL-open failure when reopened", async function () {
+        this.timeout(5000);
+        const xhr = new XMLHttpRequest() as XMLHttpRequest & { errorCode: string; errorDetail: string };
+        let errors = 0;
+        xhr.addEventListener("error", () => errors++);
+        xhr.open("GET", "noscheme.glb");
+        xhr.send();
+        xhr.open("GET", "app:///Assets/symlink_target.js");
+        expect(xhr.errorCode).to.equal("");
+        expect(xhr.errorDetail).to.equal("");
+        const completed = new Promise<void>((resolve) => xhr.addEventListener("loadend", () => resolve()));
+        xhr.send();
+        await completed;
+        expect(errors).to.equal(0);
+        expect(xhr.status).to.equal(200);
+        expect(xhr.responseText).to.equal("var symlink_target_js = true;");
+    });
+
+    it("should cancel a pending URL-open failure when aborted", async function () {
+        this.timeout(5000);
+        const xhr = new XMLHttpRequest();
+        const events: string[] = [];
+        xhr.addEventListener("error", () => events.push("error"));
+        xhr.addEventListener("loadend", () => events.push("loadend"));
+        xhr.open("GET", "noscheme.glb");
+        xhr.send();
+        xhr.abort();
+        expect(xhr.readyState).to.equal(XMLHttpRequest.UNSENT);
+        expect(() => xhr.send()).to.throw();
+        await new Promise<void>((resolve) => setTimeout(resolve, 10));
+        expect(events).to.deep.equal([]);
+        expect(xhr.readyState).to.equal(XMLHttpRequest.UNSENT);
     });
 
-    it("should throw something when opening a url with no scheme", function () {
-        function openNoProtocol() {
+    for (const event of ["before send", "before dispatch", "readystatechange", "error"]) {
+        it(`should release canceled URL-open listeners after aborting ${event}`, async function () {
+            this.timeout(5000);
             const xhr = new XMLHttpRequest();
+            let callbacks = 0;
+            const listener = () => {
+                callbacks++;
+                if ((event === "readystatechange" || event === "error") && xhr.readyState === XMLHttpRequest.DONE) {
+                    xhr.abort();
+                }
+            };
+            xhr.addEventListener("readystatechange", listener);
+            xhr.addEventListener("error", listener);
+            if (event === "error") {
+                xhr.removeEventListener("readystatechange", listener);
+            }
             xhr.open("GET", "noscheme.glb");
+            if (event === "before send") {
+                xhr.abort();
+            }
             xhr.send();
-        }
-        expect(openNoProtocol).to.throw();
-    });
+            if (event === "before dispatch") {
+                xhr.abort();
+                expect(xhr.readyState).to.equal(XMLHttpRequest.UNSENT);
+                expect(() => xhr.send()).to.throw();
+            }
+            await new Promise<void>((resolve) => setTimeout(resolve, 10));
+            const beforeReopen = callbacks;
+            xhr.open("GET", "app:///Assets/symlink_target.js");
+            const completed = new Promise<void>((resolve) => xhr.addEventListener("loadend", () => resolve()));
+            xhr.send();
+            await completed;
+            expect(xhr.readyState).to.equal(XMLHttpRequest.DONE);
+            expect(callbacks).to.equal(beforeReopen);
+        });
+    }
+
+    for (const event of ["readystatechange", "error"]) {
+        it(`should preserve a request reopened from the failure ${event} callback`, async function () {
+            this.timeout(5000);
+            const xhr = new XMLHttpRequest();
+            let reopened = false;
+            let errors = 0;
+            xhr.addEventListener("error", () => errors++);
+            xhr.addEventListener(event, () => {
+                if (!reopened && xhr.readyState === XMLHttpRequest.DONE) {
+                    reopened = true;
+                    xhr.open("GET", "app:///Assets/symlink_target.js");
+                    xhr.send();
+                }
+            });
+            const completed = new Promise<void>((resolve) => xhr.addEventListener("loadend", () => resolve()));
+            xhr.open("GET", "noscheme.glb");
+            xhr.send();
+            await completed;
+            expect(reopened).to.equal(true);
+            expect(errors).to.equal(event === "error" ? 1 : 0);
+            expect(xhr.status).to.equal(200);
+            expect(xhr.responseText).to.equal("var symlink_target_js = true;");
+        });
+    }
 
     it("should throw something when sending before opening", function () {
         function sendWithoutOpening() {