Diagnosability: fetch/XHR error fidelity and modern AbortSignal (#196) by bkaradzic-microsoft · Pull Request #200 · BabylonJS/JsRuntimeHost

bkaradzic-microsoft · 2026-06-16T19:49:32Z

Implements the fetch/XHR diagnosability work tracked in #196 — making transport failures observable to the embedding app instead of opaque — plus modern AbortSignal support.

Requires the UrlLib transport-error accessors merged in BabylonJS/UrlLib#33 (this PR bumps the UrlLib pin to e86ffb3).

Note: the unhandled-promise-rejection hop that was originally part of this PR has been split out into #204 for its own design discussion (it's engine-conditional — V8/Apple JSC only — and needs an #ifdef-free rewrite). Everything remaining here is uniform across all JS engines and carries no engine/platform preprocessor guards.

Each hop is a self-contained commit.

Hop 2 — fetch/XHR transport-error fidelity

Previously every transport failure (DNS failure, connection refused, TLS rejection, missing app:/// asset) was flattened: fetch() rejected with a plain Error{"fetch: network request failed"} (no cause/code/url, stack snapshotted in a worker tick = zero user frames); XHR exposed nothing beyond status === 0.

fetch rejects with a TypeError (stable message "fetch failed"), detail nested under error.cause = {code, detail, url, status} (Node/undici shape, not top-level own-properties).
cause.code/cause.detail come from UrlLib; present on Apple/Linux, omitted on Windows/Android while the standard shape (TypeError + stable message + cause.url/status) is preserved everywhere — a strict, additive superset of the spec. (This variance is a UrlLib HTTP-backend difference, not a JS-engine one.)
The JS call-site stack is captured synchronously inside fetch() (via the global Error constructor, which materializes frames even on Chakra) and reattached to the rejection.
XMLHttpRequest gains additive read-only errorCode/errorDetail accessors; its standard error event + status === 0 behavior is unchanged.

Hop 4 — `init.signal` + modern AbortSignal

fetch() ignored init.signal entirely (arcana::cancellation::none()), and the AbortSignal polyfill predated the modern spec.

AbortSignal: reason (read-only), throwIfAborted(), static AbortSignal.abort(reason?), read-only aborted, and a default AbortError reason (an Error whose name is "AbortError"; no DOMException polyfill exists). AbortController.abort(reason?) forwards the reason.
fetch honors init.signal via its JS interface: an already-aborted signal rejects synchronously; an in-flight abort cancels the transport (UrlRequest::Abort()) and rejects with the signal's reason.

Validation

Built and ran the UnitTests suite on Win32 / Chakra and Win32 / V8; CI is green across the full matrix (Chakra/V8/JSC/JSI × Win32/UWP/Android/iOS/macOS/Linux, incl. sanitizers).

New tests (all engine-agnostic): fetch TypeError+cause shape (refused / missing-asset), XHR errorCode/errorDetail, the modern AbortSignal API, and fetch AbortError (pre-aborted + in-flight).

Cross-repo follow-up

UrlRequest::Abort() only cancels the Windows backend today — making it interrupt the curl/NSURLSession transports is a separate UrlLib change (noted in #196). AbortSignal.timeout() is also a follow-up.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

Previously every fetch()/XMLHttpRequest transport failure (DNS failure, connection refused, TLS failure, missing local asset, ...) was flattened to a single opaque signal -- fetch rejected with `std::runtime_error{"fetch: network request failed"}` (a plain Error with no cause/code/url and a stack snapshotted in a worker-thread scheduler tick, i.e. zero user frames), and XHR exposed nothing beyond status 0. Field crash reports could not tell the failure classes apart. This is hop 2 of BabylonJS#196, now unblocked by the UrlLib transport-error accessors (BabylonJS/UrlLib#33). fetch: - Reject with a TypeError whose message is the stable string "fetch failed" (kept constant so crash-report grouping stays intact), carrying the variable detail on `error.cause = { code, detail, url, status }` -- the Node/undici shape -- rather than as non-standard top-level own-properties. - `cause.code`/`cause.detail` come from UrlLib's ErrorSymbol()/ErrorString(); they are present on backends that populate them (Apple, Linux) and omitted on those that do not yet (Windows, Android), while the standard observable shape (TypeError + stable message + cause.url/status) is preserved everywhere. This is a strict, additive superset of the spec: spec-conformant code sees only a TypeError, as in a browser; BN-aware code can distinguish failure classes. - Capture the JS call-site stack synchronously inside fetch(), before SendAsync() hands off to a worker thread, and reattach it to the rejection so crash reports attribute the failing call. Uses the global JS Error constructor so engines that materialize .stack from that path (incl. Chakra) capture live frames. XMLHttpRequest: - Add non-standard, additive read-only `errorCode`/`errorDetail` accessors that expose the same UrlLib detail; the standard error event + status===0 behavior is unchanged. Also bump the UrlLib pin to e86ffb3 (BabylonJS#33) so the accessors are available, add JS tests for the rejection shape and XHR diagnostics, and document both in the polyfill Readmes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

This PR improves diagnosability of transport-level failures in the fetch() and XMLHttpRequest polyfills by preserving structured error detail (from UrlLib) and capturing a synchronous call-site stack before work hops to a worker thread.

Changes:

fetch() now rejects transport failures as TypeError("fetch failed") with a structured error.cause object and an optionally reattached call-site stack.
XMLHttpRequest gains additive errorCode / errorDetail accessors for transport-failure diagnostics.
UrlLib pin is bumped to include transport-error accessor support; unit tests and READMEs are updated accordingly.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
Tests/UnitTests/Scripts/tests.ts	Adds unit tests asserting the new fetch `TypeError` + `cause` shape and XHR `errorCode`/`errorDetail` behavior.
Polyfills/XMLHttpRequest/Source/XMLHttpRequest.h	Declares new `errorCode` / `errorDetail` accessors.
Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp	Implements the new diagnostics accessors and wires them as instance properties.
Polyfills/XMLHttpRequest/Readme.md	Documents the new XHR transport-error diagnostics.
Polyfills/Fetch/Source/Fetch.cpp	Implements `fetch failed` TypeError rejections with `cause` and synchronous stack capture/reattachment.
Polyfills/Fetch/Readme.md	Documents the new fetch transport-failure rejection shape and stack behavior.
CMakeLists.txt	Updates UrlLib dependency pin to the commit providing transport-error accessors.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…or event - CaptureCallSiteStack: detect the global Error constructor via IsUndefined()/IsNull() instead of IsFunction(), matching the repo pattern (some JavaScriptCore/JSI builds report constructors as typeof 'object', which would silently disable synchronous stack capture). - XMLHttpRequest Readme: add the 'error' event to the supported-events list, since the implementation and tests rely on it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

bghgary

[Reviewed by Copilot on behalf of @bghgary]

Comments inline. Main asks: implement JSC in this PR (it has the API, CI covers it); drop the opt-in flag — always track, routing to the existing UnhandledExceptionHandler; and make the two test-gating mechanisms consistent (compile-time #if defined, not a runtime engine-name string compare). Plus two minor V8-tracker robustness notes.

bkaradzic-microsoft · 2026-06-19T00:36:56Z

@bghgary Thanks for the thorough review — all of it is addressed in 97e2c68 (hop 3 refactor):

JSC implemented in this PR via JSGlobalContextSetUnhandledRejectionCallback; engine-agnostic bookkeeping factored into AppRuntime_PromiseRejection.h (PromiseRejectionTracker<CandidateT> + ToError), so JSC is a thin adapter and V8 instantiates the shared tracker.
Flag dropped — unhandled rejections always route to UnhandledExceptionHandler; coverage documented once on AppRuntime.h (V8 + JSC supported; Chakra/JSI no-op).
V8 iterator invalidation fixed (drain candidates via std::move before reporting).
V8 GetIdentityHash collision fixed (vector + promise-identity match, hash key removed).
Compile-time test gate — CMake emits JSRUNTIMEHOST_NAPI_ENGINE_${ENGINE}; tests gate on #if defined(...), no more std::string_view compare.
Chakra note trimmed to defer to the header.

Validated locally on Win32: V8 Release — both AppRuntime rejection tests pass and JavaScript.All stays green (203 passing) with always-on tracking; Chakra Debug — compiles and the rejection tests skip via the gate. JSC validates on the macOS CI matrix; watching that job (the rejection callback uses an SPI from JSContextRefPrivate.h, forward-declared and __builtin_available-guarded — I'll iterate if the CI SDK doesn't link it).

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#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 BabylonJS#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>

bghgary

[Reviewed by Copilot on behalf of @bghgary]

The fetch/XHR error-fidelity and AbortSignal changes look good. My one concern is the unhandled-promise-rejection tracking (inline) — could you split that into its own PR? I'll approve the rest as-is.

bghgary

[Reviewed by Copilot on behalf of @bghgary]

LGTM

Copilot AI review requested due to automatic review settings June 16, 2026 19:49

Copilot started reviewing on behalf of bkaradzic-microsoft June 16, 2026 19:50 View session

Copilot AI reviewed Jun 16, 2026

View reviewed changes

Comment thread Polyfills/Fetch/Source/Fetch.cpp

Comment thread Polyfills/XMLHttpRequest/Readme.md

bkaradzic-microsoft marked this pull request as draft June 16, 2026 19:58

bkaradzic-microsoft requested a review from bghgary June 16, 2026 19:59

This was referenced Jun 16, 2026

AppRuntime: route unhandled promise rejections to the host handler (V8) — hop 3 of #196 #201

Closed

fetch/AbortSignal: honor init.signal + modernize AbortSignal — hop 4 of #196 #202

Closed

bkaradzic-microsoft changed the title ~~fetch/XHR: surface transport-failure detail (TypeError + cause + sync stack) — hop 2 of #196~~ Debuggability: fetch/XHR error fidelity, unhandled-rejection handler, and AbortSignal (#196) Jun 16, 2026

bkaradzic-microsoft marked this pull request as ready for review June 18, 2026 15:12

bghgary requested changes Jun 19, 2026

View reviewed changes

bkaradzic-microsoft requested a review from bghgary July 6, 2026 18:13

bkaradzic-microsoft force-pushed the hop2-fetch-error-fidelity branch from b2553a2 to fa8c1f0 Compare July 7, 2026 15:54

bkaradzic-microsoft mentioned this pull request Jul 7, 2026

Diagnosability: route unhandled promise rejections to the host handler (#196) #204

Draft

bkaradzic-microsoft changed the title ~~Debuggability: fetch/XHR error fidelity, unhandled-rejection handler, and AbortSignal (#196)~~ Diagnosability: fetch/XHR error fidelity and modern AbortSignal (#196) Jul 7, 2026

bghgary requested changes Jul 7, 2026

View reviewed changes

Comment thread Core/AppRuntime/Include/Babylon/AppRuntime.h Outdated

bkaradzic-microsoft requested a review from bghgary July 7, 2026 16:07

bghgary approved these changes Jul 7, 2026

View reviewed changes

bkaradzic-microsoft merged commit 8c8e32f into BabylonJS:main Jul 7, 2026
21 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Diagnosability: fetch/XHR error fidelity and modern AbortSignal (#196)#200

Diagnosability: fetch/XHR error fidelity and modern AbortSignal (#196)#200
bkaradzic-microsoft merged 3 commits into
BabylonJS:mainfrom
bkaradzic-microsoft:hop2-fetch-error-fidelity

bkaradzic-microsoft commented Jun 16, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

bghgary left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bkaradzic-microsoft commented Jun 19, 2026

Uh oh!

bghgary left a comment

Uh oh!

Uh oh!

bghgary left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

bkaradzic-microsoft commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Hop 2 — fetch/XHR transport-error fidelity

Hop 4 — init.signal + modern AbortSignal

Validation

Cross-repo follow-up

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

bghgary left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bkaradzic-microsoft commented Jun 19, 2026

Uh oh!

bghgary left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bghgary left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

bkaradzic-microsoft commented Jun 16, 2026 •

edited

Loading

Hop 4 — `init.signal` + modern AbortSignal