diff --git a/Polyfills/XMLHttpRequest/Readme.md b/Polyfills/XMLHttpRequest/Readme.md index ea00d006..c53425d4 100644 --- a/Polyfills/XMLHttpRequest/Readme.md +++ b/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 `":(): "` string +* `errorDetail` -- the original opening error for `UrlOpenFailed`, or the normalized + `":(): "` 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. diff --git a/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp b/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp index bac1f90f..b8e41495 100644 --- a/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp +++ b/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp @@ -2,6 +2,7 @@ #include #include #include +#include #include #include @@ -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 ":(): " 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 + // ":(): " 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().Utf8Value()); + } + catch (const std::exception& e) + { + throw Napi::Error::New(info.Env(), e.what()); + } + m_url = info[1].As(); + m_openGeneration = ++m_requestGeneration; + m_openError.reset(); + m_openErrorSent = false; try { - m_request.Open(MethodType::StringToEnum(info[0].As().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::Persistent(info.This().As())); + 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(traceName.c_str()); diff --git a/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.h b/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.h index 74d2c3b9..66640a66 100644 --- a/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.h +++ b/Polyfills/XMLHttpRequest/Source/XMLHttpRequest.h @@ -6,6 +6,8 @@ #include #include +#include +#include #include namespace Babylon::Polyfills::Internal @@ -49,6 +51,10 @@ namespace Babylon::Polyfills::Internal void RaiseEvent(const char* eventType); std::string m_url{}; + std::optional 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}; diff --git a/Tests/UnitTests/Source/Scripts/tests.xmlHttpRequest.ts b/Tests/UnitTests/Source/Scripts/tests.xmlHttpRequest.ts index ab50e950..7da072a7 100644 --- a/Tests/UnitTests/Source/Scripts/tests.xmlHttpRequest.ts +++ b/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((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((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((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((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((resolve) => setTimeout(resolve, 10)); + const beforeReopen = callbacks; + xhr.open("GET", "app:///Assets/symlink_target.js"); + const completed = new Promise((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((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() {