UrlLib StatusText for fetch + XMLHttpRequest.statusText (closes #193)#195
Conversation
fcfd6d1 to
b8e5c13
Compare
b8e5c13 to
79937de
Compare
There was a problem hiding this comment.
Pull request overview
This PR aligns the fetch and XMLHttpRequest polyfills with upstream UrlLib’s newly exposed HTTP reason phrase API (UrlRequest::StatusText()), removing the local status-code→text mapping and adding test coverage for statusText.
Changes:
- Fetch: remove the local
StatusTextmapping table and populateResponse.statusTextfromUrlRequest::StatusText(). - XMLHttpRequest: add a read-only
statusTextaccessor wired toUrlRequest::StatusText(). - Tests: add/extend unit tests asserting
statusTextfor200 OKand404 Not Found, and bump the UrlLib pin to the merge commit that providesStatusText().
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| Tests/UnitTests/Scripts/tests.ts | Adds/extends unit tests for fetch + XMLHttpRequest statusText on 200/404 responses. |
| Polyfills/XMLHttpRequest/Source/XMLHttpRequest.h | Declares GetStatusText and exposes statusText on the JS wrapper. |
| Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp | Implements statusText getter using UrlRequest::StatusText(). |
| Polyfills/Fetch/Source/Fetch.cpp | Drops the local status-text table and sets Response.statusText from UrlLib. |
| CMakeLists.txt | Updates the UrlLib GIT_TAG pin to include UrlRequest::StatusText(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Follow-up to BabylonJS#188. UrlLib now exposes UrlRequest::StatusText() (the wire reason phrase where available, else a canonical code->text table), so: - Fetch: drop the private status-code->text table and read request.StatusText() into Response.statusText. - XMLHttpRequest: expose statusText (previously absent) for free. Adds fetch + XMLHttpRequest statusText tests (OK / Not Found). Bumps the UrlLib pin to BabylonJS/UrlLib 74985214, which adds UrlRequest::StatusText() (BabylonJS/UrlLib#30). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
79937de to
814c5f0
Compare
|
This lands the statusText hop nicely — single source of truth in UrlLib with the wire phrase preferred and the shared fallback table keeping platforms consistent (and avoiding the Heads-up that this slots into a larger chain we just wrote up in #196: BabylonJS/UrlLib#31 (open) adds normalized transport-error detail to UrlLib ( Question on logistics so we don't collide: would you like us to submit PRs for consuming UrlLib#31 (fetch/XHR error shape) and the related wiring from #196 (unhandled-rejection delivery, |
Closes #193.
Summary
Now that BabylonJS/UrlLib#30 is merged,
UrlRequest::StatusText()(the wire reason phrase where available, else a canonical code→text fallback table) is available upstream:Fetch.cppand readrequest.StatusText()intoResponse.statusText.statusText(previously absent) — now free from the same UrlLib API.fetch+XMLHttpRequeststatusTextunit tests (OK/Not Found).The
UrlLibpin is bumped toBabylonJS/UrlLib74985214(the UrlLib#30 merge commit).Verified locally:
FetchandXMLHttpRequestpolyfill libraries build clean against the bumped UrlLib pin (Win32 Chakra). Unit tests (incl. the newstatusTexttests) run in CI.Notes
statusTextis consumed purely to decorate error messages (nothing branches on it), so the concrete win is e.g.404 Not Foundinstead of404 undefined. The code→text fallback table in UrlLib is the right shape for how the value is used (real browsers return""over HTTP/2).getResponseMessage()is a possible follow-up.