Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 13 additions & 7 deletions Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ namespace Babylon::Polyfills::Internal
{
constexpr const char* ReadyStateChange = "readystatechange";
constexpr const char* LoadEnd = "loadend";
constexpr const char* Error = "error";
}
}

Expand Down Expand Up @@ -262,19 +263,24 @@ namespace Babylon::Polyfills::Internal
.then(arcana::inline_scheduler, arcana::cancellation::none(), [sendRegion{std::move(sendRegion)}]() mutable {
sendRegion.reset();
})
.then(m_runtimeScheduler, arcana::cancellation::none(), [this]() {
.then(m_runtimeScheduler, arcana::cancellation::none(), [this](const arcana::expected<void, std::exception_ptr>& result) {
// Run on every outcome -- transport exception OR underlying request succeeded but ended in a non-2xx
// status (e.g. a missing local file on UWP, where UrlLib silently retains status 0). The previous
// success-only continuation here skipped readyState=Done / loadend / error and let the JS observer
// hang.
const auto statusCode = arcana::underlying_cast(m_request.StatusCode());
const bool failed = result.has_error() || statusCode < 200 || statusCode >= 300;

SetReadyState(ReadyState::Done);
if (failed)
{
RaiseEvent(EventType::Error);
}
RaiseEvent(EventType::LoadEnd);

// Assume the XMLHttpRequest will only be used for a single request and clear the event handlers.
// Single use seems to be the standard pattern, and we need to release our strong refs to event handlers.
m_eventHandlerRefs.clear();
})
.then(arcana::inline_scheduler, arcana::cancellation::none(), [env = info.Env()](arcana::expected<void, std::exception_ptr> result) {
if (result.has_error())
{
Napi::Error::New(env, result.error()).ThrowAsJavaScriptException();
}
});
}

Expand Down
25 changes: 25 additions & 0 deletions Tests/UnitTests/Scripts/tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,31 @@ describe("XMLHTTPRequest", function () {
expect(xhr.status).to.equal(404);
});

it("should fire 'error' event for a remote URL that returns HTTP 404", async function () {
// Regression test: previously the success-only continuation in XMLHttpRequest::Send
// skipped 'error' on async failures including non-2xx HTTP responses, so onerror
// observers never ran. See https://github.com/BabylonJS/JsRuntimeHost/pull/165.
this.timeout(30000);
const result = await new Promise<{ errorFired: boolean; loadendFired: boolean; status: number; readyState: number }>((resolve, reject) => {
const xhr = new XMLHttpRequest();
let errorFired = false;
let loadendFired = false;
const guard = setTimeout(() => reject(new Error("XHR neither errored nor loadended within 25s")), 25000);
xhr.addEventListener("error", () => { errorFired = true; });
xhr.addEventListener("loadend", () => {
loadendFired = true;
clearTimeout(guard);
resolve({ errorFired, loadendFired, status: xhr.status, readyState: xhr.readyState });
});
xhr.open("GET", "https://github.com/babylonJS/BabylonNative404");
xhr.send();
});
expect(result.status).to.equal(404);
expect(result.errorFired).to.equal(true);
expect(result.loadendFired).to.equal(true);
expect(result.readyState).to.equal(4);
});
Comment thread
bkaradzic-microsoft marked this conversation as resolved.

it("should throw something when opening //", async function () {
function openDoubleSlash() {
const xhr = new XMLHttpRequest();
Expand Down
Loading