From 48fc99531d5cb8482792532054d428612a9cdc62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Branimir=20Karad=C5=BEi=C4=87=20=28via=20Copilot=29?= <223556219+Copilot@users.noreply.github.com> Date: Tue, 16 Jun 2026 13:39:22 -0700 Subject: [PATCH] Hop 4: honor init.signal in fetch and modernize AbortSignal fetch() previously ignored init.signal entirely (it passed arcana::cancellation::none() to both continuations), so AbortController could never cancel a request or reject with AbortError, and the AbortSignal polyfill predated the modern spec. This is hop 4 of BabylonJS/JsRuntimeHost#196. AbortSignal: - Add `reason` (read-only) and `throwIfAborted()`. - Add static `AbortSignal.abort(reason?)`. - Make `aborted` read-only (remove the setter). - Abort(reason) now records the reason, defaulting to an AbortError (an Error whose name is "AbortError", as there is no DOMException polyfill). - AbortController.abort(reason) forwards the reason to the signal. fetch: - Honor init.signal via its JS interface (so fetch stays decoupled from the AbortController C++ types): an already-aborted signal rejects synchronously with the signal's reason; otherwise an "abort" listener cancels the transport (UrlRequest::Abort()) and the completion continuation rejects with the signal's reason (an AbortError) instead of a network-error TypeError. The listener is torn down when the request settles, breaking the ownership cycle. Transport cancellation is effective on backends where UrlRequest::Abort() is implemented (Windows today; the curl/NSURLSession backends are a UrlLib follow-up noted in #196). AbortSignal.timeout() is left as a follow-up. Adds JS tests for the modern AbortSignal API and for fetch rejecting with an AbortError both pre-aborted and in-flight (validated on Win32, where the UrlLib backend honors Abort()). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Polyfills/AbortController/Readme.md | 15 +++- .../Source/AbortController.cpp | 6 +- .../AbortController/Source/AbortSignal.cpp | 53 +++++++++++- .../AbortController/Source/AbortSignal.h | 17 +++- Polyfills/Fetch/Source/Fetch.cpp | 84 ++++++++++++++++++- Tests/UnitTests/Scripts/tests.ts | 62 ++++++++++++++ 6 files changed, 225 insertions(+), 12 deletions(-) diff --git a/Polyfills/AbortController/Readme.md b/Polyfills/AbortController/Readme.md index 4ec00a7f..f5b8b596 100644 --- a/Polyfills/AbortController/Readme.md +++ b/Polyfills/AbortController/Readme.md @@ -1,10 +1,21 @@ # AbortController Implements parts of [`AbortController`](https://developer.mozilla.org/en-US/docs/Web/API/AbortController/) and [`AbortSignal`](https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal). Provides a way to trigger the abort signal. *Work In Progress* +Supported on `AbortSignal`: +* `aborted` (read-only) and `reason` +* [`throwIfAborted()`](https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/throwIfAborted) +* static [`AbortSignal.abort(reason?)`](https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/abort) +* `onabort`, `addEventListener("abort", ...)`, `removeEventListener` + +`AbortController.abort(reason?)` forwards the reason to the signal; when no reason is given the +signal's `reason` defaults to an `AbortError` (an `Error` whose `name` is `"AbortError"`, since +there is no `DOMException` polyfill). `fetch()` honors an `AbortSignal` passed via `init.signal`: +an already-aborted signal rejects the promise synchronously, and an in-flight abort cancels the +transport and rejects with the signal's `reason`. (Transport cancellation is effective on backends +where `UrlLib::UrlRequest::Abort()` is implemented.) + Currently not implemented: -* [`ThrowIfAborted`](https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/throwIfAborted) * [`Timeout`](https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/timeout) -* [`Abort`](https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/abort) Both the AbortController and AbortSignal polyfills are initialized inside AbortController's initialize method: ```c++ diff --git a/Polyfills/AbortController/Source/AbortController.cpp b/Polyfills/AbortController/Source/AbortController.cpp index 3e6bcb43..0b5068be 100644 --- a/Polyfills/AbortController/Source/AbortController.cpp +++ b/Polyfills/AbortController/Source/AbortController.cpp @@ -25,12 +25,12 @@ namespace Babylon::Polyfills::Internal return m_signal.Value(); } - void AbortController::Abort(const Napi::CallbackInfo&) + void AbortController::Abort(const Napi::CallbackInfo& info) { AbortSignal* sig = AbortSignal::Unwrap(m_signal.Value()); - + assert(sig != nullptr); - sig->Abort(); + sig->Abort(info.Length() > 0 ? info[0] : info.Env().Undefined()); } AbortController::AbortController(const Napi::CallbackInfo& info) diff --git a/Polyfills/AbortController/Source/AbortSignal.cpp b/Polyfills/AbortController/Source/AbortSignal.cpp index e0e77834..c2d44544 100644 --- a/Polyfills/AbortController/Source/AbortSignal.cpp +++ b/Polyfills/AbortController/Source/AbortSignal.cpp @@ -11,10 +11,13 @@ namespace Babylon::Polyfills::Internal env, JS_ABORT_SIGNAL_CONSTRUCTOR_NAME, { - InstanceAccessor("aborted", &AbortSignal::GetAborted, &AbortSignal::SetAborted), + InstanceAccessor("aborted", &AbortSignal::GetAborted, nullptr), + InstanceAccessor("reason", &AbortSignal::GetReason, nullptr), InstanceAccessor("onabort", &AbortSignal::GetOnAbort, &AbortSignal::SetOnAbort), + InstanceMethod("throwIfAborted", &AbortSignal::ThrowIfAborted), InstanceMethod("addEventListener", &AbortSignal::AddEventListener), InstanceMethod("removeEventListener", &AbortSignal::RemoveEventListener), + StaticMethod("abort", &AbortSignal::AbortStatic), }); env.Global().Set(JS_ABORT_SIGNAL_CONSTRUCTOR_NAME, func); @@ -26,10 +29,30 @@ namespace Babylon::Polyfills::Internal { } - void AbortSignal::Abort() + Napi::Value AbortSignal::CreateAbortError(Napi::Env env, const char* message) { + // There is no DOMException polyfill, so represent the abort reason as an Error whose `name` + // is "AbortError" -- the value web code checks (`err.name === "AbortError"`). + Napi::Error error = Napi::Error::New(env, message); + error.Set("name", Napi::String::New(env, "AbortError")); + return error.Value(); + } + + void AbortSignal::Abort(const Napi::Value& reason) + { + if (m_aborted) + { + return; + } + m_aborted = true; + Napi::Env env = Env(); + const Napi::Value resolvedReason = (reason.IsUndefined() || reason.IsEmpty()) + ? CreateAbortError(env, "The operation was aborted.") + : reason; + m_reason = Napi::Persistent(resolvedReason); + auto onabort = m_onabort.Value(); if (!onabort.IsNull() && !onabort.IsUndefined()) { @@ -39,14 +62,36 @@ namespace Babylon::Polyfills::Internal RaiseEvent("abort"); } + Napi::Value AbortSignal::AbortStatic(const Napi::CallbackInfo& info) + { + Napi::Env env = info.Env(); + Napi::Object signalObject = env.Global().Get(JS_ABORT_SIGNAL_CONSTRUCTOR_NAME).As().New({}); + AbortSignal* signal = AbortSignal::Unwrap(signalObject); + signal->Abort(info.Length() > 0 ? info[0] : env.Undefined()); + return signalObject; + } + Napi::Value AbortSignal::GetAborted(const Napi::CallbackInfo&) { return Napi::Value::From(Env(), m_aborted); } - void AbortSignal::SetAborted(const Napi::CallbackInfo&, const Napi::Value& value) + Napi::Value AbortSignal::GetReason(const Napi::CallbackInfo&) { - m_aborted = value.As(); + if (m_reason.IsEmpty()) + { + return Env().Undefined(); + } + + return m_reason.Value(); + } + + void AbortSignal::ThrowIfAborted(const Napi::CallbackInfo& info) + { + if (m_aborted) + { + throw Napi::Error{info.Env(), GetReason(info)}; + } } Napi::Value AbortSignal::GetOnAbort(const Napi::CallbackInfo&) diff --git a/Polyfills/AbortController/Source/AbortSignal.h b/Polyfills/AbortController/Source/AbortSignal.h index a2de0e4f..a2888c80 100644 --- a/Polyfills/AbortController/Source/AbortSignal.h +++ b/Polyfills/AbortController/Source/AbortSignal.h @@ -17,11 +17,23 @@ namespace Babylon::Polyfills::Internal static void Initialize(Napi::Env env); explicit AbortSignal(const Napi::CallbackInfo& info); - void Abort(); + // Transition the signal to the aborted state with the given reason (undefined -> a default + // "AbortError"), firing onabort and any "abort" listeners. No-op if already aborted. + void Abort(const Napi::Value& reason); + + // Build the default abort reason: an Error whose name is "AbortError" (there is no + // DOMException polyfill), matching what the platform uses when abort() is called with no + // reason and what fetch() rejects with on abort. + static Napi::Value CreateAbortError(Napi::Env env, const char* message); private: Napi::Value GetAborted(const Napi::CallbackInfo& info); - void SetAborted(const Napi::CallbackInfo&, const Napi::Value& value); + + Napi::Value GetReason(const Napi::CallbackInfo& info); + void ThrowIfAborted(const Napi::CallbackInfo& info); + + // AbortSignal.abort(reason?) -- returns an AbortSignal already in the aborted state. + static Napi::Value AbortStatic(const Napi::CallbackInfo& info); Napi::Value GetOnAbort(const Napi::CallbackInfo& info); void SetOnAbort(const Napi::CallbackInfo&, const Napi::Value& value); @@ -33,6 +45,7 @@ namespace Babylon::Polyfills::Internal std::unordered_map> m_eventHandlerRefs; Napi::FunctionReference m_onabort; + Napi::Reference m_reason; bool m_aborted = false; }; } \ No newline at end of file diff --git a/Polyfills/Fetch/Source/Fetch.cpp b/Polyfills/Fetch/Source/Fetch.cpp index 7ffee288..a9112e0e 100644 --- a/Polyfills/Fetch/Source/Fetch.cpp +++ b/Polyfills/Fetch/Source/Fetch.cpp @@ -30,6 +30,32 @@ namespace Babylon::Polyfills::Internal std::vector body; }; + // Shared state for honoring an AbortSignal passed via init.signal. Co-owned by the "abort" + // listener (which sets the flag, captures the reason, and cancels the transport) and the + // completion continuation (which reports the AbortError and tears the listener down). + struct AbortState + { + bool aborted{false}; + Napi::Reference reason; + Napi::ObjectReference signal; + Napi::FunctionReference listener; + }; + + // The reason a fetch was aborted: the signal's `reason` (per the modern AbortSignal), or a + // fresh AbortError if the signal does not expose one. + Napi::Value GetAbortReason(Napi::Env env, const Napi::Object& signal) + { + const Napi::Value reason = signal.Get("reason"); + if (!reason.IsUndefined() && !reason.IsNull()) + { + return reason; + } + + Napi::Error error = Napi::Error::New(env, "The operation was aborted."); + error.Set("name", Napi::String::New(env, "AbortError")); + return error.Value(); + } + bool EqualsIgnoreCase(std::string_view a, std::string_view b) { return std::equal(a.begin(), a.end(), b.begin(), b.end(), [](unsigned char l, unsigned char r) { @@ -273,6 +299,7 @@ namespace Babylon::Polyfills::Internal UrlLib::UrlMethod method = UrlLib::UrlMethod::Get; std::optional body; Napi::Value headers = env.Undefined(); + Napi::Value signal = env.Undefined(); if (info.Length() > 1 && info[1].IsObject()) { @@ -295,6 +322,7 @@ namespace Babylon::Polyfills::Internal } headers = init.Get("headers"); + signal = init.Get("signal"); } auto request = std::make_shared(); @@ -306,6 +334,39 @@ namespace Babylon::Polyfills::Internal request->SetRequestBody(std::move(*body)); } + // Honor an AbortSignal passed via init.signal (WHATWG fetch). The signal is used + // through its JS interface (aborted / reason / add/removeEventListener) so fetch + // stays decoupled from the AbortController polyfill's C++ types. + std::shared_ptr abortState; + if (signal.IsObject()) + { + const Napi::Object signalObject = signal.As(); + + // Already aborted: reject synchronously with the signal's reason, never + // touching the transport. + if (signalObject.Get("aborted").ToBoolean().Value()) + { + deferred.Reject(GetAbortReason(env, signalObject)); + return deferred.Promise(); + } + + abortState = std::make_shared(); + abortState->signal = Napi::Persistent(signalObject); + + Napi::Function listener = Napi::Function::New(env, [abortState, request, env](const Napi::CallbackInfo&) { + if (!abortState->aborted) + { + abortState->aborted = true; + abortState->reason = Napi::Persistent(GetAbortReason(env, abortState->signal.Value())); + // Cancel the in-flight transport; the completion continuation then + // rejects with the AbortError instead of a transport TypeError. + request->Abort(); + } + }); + abortState->listener = Napi::Persistent(listener); + signalObject.Get("addEventListener").As().Call(signalObject, {Napi::String::New(env, "abort"), listener}); + } + // arcana::task::then captures the scheduler by reference (see arcana task.h) and // invokes it on the worker thread when the request completes -- after this fetch() // call has returned. A stack-local scheduler would therefore dangle. Heap-allocate @@ -313,7 +374,28 @@ namespace Babylon::Polyfills::Internal auto scheduler = std::make_shared(JsRuntime::GetFromJavaScript(env)); request->SendAsync() .then(*scheduler, arcana::cancellation::none(), - [deferred, request, env](const arcana::expected& result) { + [deferred, request, env, abortState](const arcana::expected& result) { + // The request has settled: stop listening for aborts (breaking the + // listener <-> abortState ownership cycle) before deciding the outcome. + if (abortState) + { + if (!abortState->signal.IsEmpty() && !abortState->listener.IsEmpty()) + { + Napi::Object signalObject = abortState->signal.Value(); + signalObject.Get("removeEventListener").As().Call(signalObject, {Napi::String::New(env, "abort"), abortState->listener.Value()}); + } + abortState->listener.Reset(); + abortState->signal.Reset(); + + if (abortState->aborted) + { + // Per the fetch spec, an aborted request rejects with the + // signal's reason (an AbortError), not a network error. + deferred.Reject(abortState->reason.Value()); + return; + } + } + const int status = static_cast(request->StatusCode()); // Per the WHATWG fetch spec, only transport-level failures reject. A completed diff --git a/Tests/UnitTests/Scripts/tests.ts b/Tests/UnitTests/Scripts/tests.ts index e0647092..8c719fa5 100644 --- a/Tests/UnitTests/Scripts/tests.ts +++ b/Tests/UnitTests/Scripts/tests.ts @@ -64,6 +64,37 @@ describe("AbortController", function () { expect(controller.signal.aborted).to.equal(true); }); + + it("AbortSignal.abort() returns a signal already aborted with an AbortError reason", function () { + const signal = (AbortSignal as any).abort(); + expect(signal.aborted).to.equal(true); + expect(signal.reason).to.be.an.instanceof(Error); + expect(signal.reason.name).to.equal("AbortError"); + }); + + it("throwIfAborted() throws the reason only once aborted", function () { + const controller = new AbortController(); + // Not aborted yet: must not throw. + (controller.signal as any).throwIfAborted(); + + controller.abort(); + expect(() => (controller.signal as any).throwIfAborted()).to.throw(); + }); + + it("abort(reason) records the provided reason", function () { + const controller = new AbortController(); + const reason = new Error("custom reason"); + controller.abort(reason); + expect((controller.signal as any).reason).to.equal(reason); + }); + + it("abort() with no reason defaults to an AbortError", function () { + const controller = new AbortController(); + controller.abort(); + const reason = (controller.signal as any).reason; + expect(reason).to.be.an.instanceof(Error); + expect(reason.name).to.equal("AbortError"); + }); }); describe("XMLHTTPRequest", function () { @@ -331,6 +362,37 @@ describe("fetch", function () { } expect(rejected).to.equal(true); }); + + it("should reject immediately with an AbortError when the signal is already aborted", async function () { + const controller = new AbortController(); + controller.abort(); + + let error: any; + try { + await fetch("https://github.com/", { signal: controller.signal } as any); + } catch (e) { + error = e; + } + expect(error, "fetch should have rejected").to.not.equal(undefined); + expect(error.name).to.equal("AbortError"); + }); + + it("should reject with an AbortError when aborted in-flight", async function () { + this.timeout(30000); + const controller = new AbortController(); + const promise = fetch("https://github.com/", { signal: controller.signal } as any); + // Abort before the response can arrive. + controller.abort(); + + let error: any; + try { + await promise; + } catch (e) { + error = e; + } + expect(error, "fetch should have rejected").to.not.equal(undefined); + expect(error.name).to.equal("AbortError"); + }); }); describe("setTimeout", function () {