Add Pyodide/Emscripten client support via FetchConnector (continuation of #7803)#13057
Draft
bendichter wants to merge 4 commits into
Draft
Add Pyodide/Emscripten client support via FetchConnector (continuation of #7803)#13057bendichter wants to merge 4 commits into
bendichter wants to merge 4 commits into
Conversation
FetchConnector dispatches requests through the JavaScript fetch() API, decoding the serialized request with HttpRequestParser and re-serializing the fetch() response into HTTP/1.1 bytes for the standard ResponseHandler, so ClientSession needs no platform-specific changes beyond connector selection. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- tests/test_fetch_connector.py: unit tests for the whole fetch pipeline using an injected stub fetch(), runnable on any platform. - tests/test_pyodide.py: pytest-pyodide integration tests running the built wasm wheel in Node.js against a local HTTP server. - CI: test-pyodide job building the cp313 Emscripten wheel with cibuildwheel and running the integration tests with --rt node. - aiohttp/_llhttp_wasm_shim.c: no-op definitions for llhttp's JavaScript-oriented wasm_on_* callbacks so the C extensions load under Emscripten. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
for more information, see https://pre-commit.ci
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #13057 +/- ##
========================================
Coverage 98.96% 98.96%
========================================
Files 131 133 +2
Lines 48156 48469 +313
Branches 2499 2514 +15
========================================
+ Hits 47656 47968 +312
Misses 376 376
- Partials 124 125 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
Merging this PR will not alter performance
Comparing Footnotes
|
- Add 'js' (from Node.js) to the docs spelling wordlist. - Exclude tests/test_pyodide.py from codecov (it only executes inside the Pyodide runtime, where coverage is not collected). - Add unit tests covering the remaining FetchConnector lines: platform default-connector selection, large-body flow control, transports writelines/abort/double-close, fetch abort on close, and responses without Headers.getSetCookie(). - Sanitize the echoed Content-Type in the test HTTP server (CodeQL response-splitting warning). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What do these changes do?
Adds client support for running under Pyodide / Emscripten (WebAssembly in browsers and Node.js), picking up where #7803 left off. I'd really like to be able to use aiohttp from Pyodide (e.g. pyodide/pyodide#4998), and since #7803 invited someone to continue the work, I took a stab at it.
Full disclosure up front: AI (Claude) was used heavily to write this. I've reviewed it and tested it end-to-end and it looks reasonable to me, but I'm very aware this could be more noise than help for maintainers — if it isn't useful, please feel free to close it, no hard feelings. If the overall direction is right, I'm happy to keep working on it based on your feedback.
Design
Following the suggestion in #7803 (comment) (encapsulate the logic in a custom connector so
_request()needs minimal changes), everything lives in a newaiohttp/pyodide.pymodule providingFetchConnector, whichClientSessionselects as the default connector whensys.platform == "emscripten"— that default selection is the only change to the client flow.Rather than subclassing
ClientRequest/ClientResponse(as #7803 did against the 3.x codebase), the connector uses the HTTP/1.1 wire format as its interface:HttpRequestParser(so cookies, auto-added headers, and chunked bodies are all handled by existing code),fetch()API,ResponseHandler/HttpResponseParser.The fetch-specific details handled: framing/hop-by-hop headers are stripped in both directions (fetch delivers bodies already decompressed/unframed, so e.g. a stale
Content-Encodingwould corrupt parsing), repeatedSet-Cookieheaders are recovered viaHeaders.getSetCookie(), cancellation/timeouts abort the fetch viaAbortController,Expect: 100-continueis answered locally, and proxies /CONNECT/ upgrades (WebSockets) raise a clearClientConnectionError.Two side discoveries:
FetchConnector(fetch=...)accepts an injected fetch implementation, so nearly the whole pipeline is unit-testable on regular CPython without a WebAssembly runtime (tests/test_fetch_connector.py) — this should help with the coverage concerns from ENH Add Pyodide support #7803.api.creferences JavaScript-providedwasm_on_*callbacks whenever__wasm__is defined, leaving undefined symbols in the extension.aiohttp/_llhttp_wasm_shim.cprovides no-op definitions (aiohttp usesllhttp_init()with its own callbacks and never touches that path).Testing
tests/test_fetch_connector.py: 18 unit tests with a stubbedfetch()(GET/POST/chunked async-generator bodies, header filtering both directions, multiSet-Cookie→ cookie jar, 204/HEAD, error propagation,expect100,fetch_options, timeout-cancels-fetch, concurrency). These run in the normal test matrix.tests/test_pyodide.py: pytest-pyodide integration tests that run the built wasm wheel (with compiled C extensions) in Node.js against a live local HTTP server — real POSTs, redirects, cookies, 404s, concurrency, and connection errors. Run by the newtest-pyodideCI job (cibuildwheel--platform pyodide, modeled on thetest-mobilejob).raise_for_status— plainClientSession()+session.get(...)works unmodified.Are there changes in behavior for the user?
None on existing platforms. Under Emscripten/Pyodide (previously unusable —
getaddrinforaisesNotImplementedError),ClientSessionnow works out of the box viafetch(). Platform limitations are documented indocs/client_reference.rst: no proxies or WebSockets, redirects are followed transparently byfetch()before aiohttp sees the response,sslarguments are ignored, and browsers enforce their own cookie/CORS policies.Is it a substantial burden for the maintainers to support this?
I've tried to minimize it: the feature is a single self-contained module plus a 2-line default-connector selection; no existing code paths change. Because it talks to the rest of the client through the HTTP wire format and the public parser/protocol interfaces, it should be fairly insulated from internal refactors. The main ongoing costs are the extra CI job (one Ubuntu runner building a cp313 wasm wheel) and keeping the pinned Pyodide/cibuildwheel versions fresh. That said, you're the best judges of this — and if the answer is "yes, too much", closing this is a completely fair outcome.
Related issue number
Related to #7253 and continues #7803 (original work by @hoodmane, who is credited in the CHANGES fragment). Also relevant to pyodide/pyodide#4998.
Checklist
CONTRIBUTORS.txtCHANGES/folder🤖 Generated with Claude Code