Skip to content

net/http: dispatch the client through its RoundTripper (fetch on js/wasm) - #83

Open
0pcom wants to merge 1 commit into
tinygo-org:mainfrom
0magnet:client-transport-dispatch
Open

net/http: dispatch the client through its RoundTripper (fetch on js/wasm)#83
0pcom wants to merge 1 commit into
tinygo-org:mainfrom
0magnet:client-transport-dispatch

Conversation

@0pcom

@0pcom 0pcom commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Fixes #66.

A Client with no explicit Transport called the package-level dialing roundTrip directly, so the build-tagged Transport.RoundTrip was never consulted — on js/wasm every request died with Netdev not set instead of using the fetch API. Client.Do also short-circuited an explicit Transport past the cookie jar, header initialization, and timeout handling.

This restores the upstream shape:

  • send takes a RoundTripper
  • Client.transport() falls back to DefaultTransport, as its docs already promised
  • Do always goes through do/send, so an explicit Transport gets cookies and timeouts too
  • a nil Transport and nil DefaultTransport reports http: no Client.Transport or DefaultTransport like upstream

The netdev path is unchanged: on !js targets DefaultTransport.RoundTrip (roundtrip.go) is the same package-level roundTrip as before.

Tests: three unit tests cover default dispatch, explicit dispatch, and the nil/nil error. Verified behaviorally on js/wasm under node — http.Get with a nil Transport now reaches the fetch transport (it reports its own no-fetch-API fallback error under node, as upstream Go does) instead of Netdev not set.

The client called the package-level dialing roundTrip directly, so a
Client with no explicit Transport never reached the build-tagged
Transport.RoundTrip — on js/wasm every request died with "Netdev not
set" instead of using the fetch API (tinygo-org#66). Client.Do also short-circuited
an explicit Transport past the cookie jar, header initialization, and
timeout handling.

send now takes a RoundTripper, Client.transport() falls back to
DefaultTransport as documented, and Do always goes through do/send. The
netdev path is unchanged: on !js targets DefaultTransport.RoundTrip is
the same package-level roundTrip as before.

Verified on js/wasm under node: http.Get with a nil Transport now
reaches the fetch transport (its no-fetch-API report) instead of
"Netdev not set".
@yohimik

yohimik commented Sep 6, 2026

Copy link
Copy Markdown

This overlaps the redirect loop in #72. I checked a Git merge of e12bf8a and your 13d50d3 and found a conflict in http/client.go. The combined client must retain redirects and sensitive-header rules while routing each request through the selected RoundTripper. Default and explicit transports should both exercise redirects and cookie handling. I have kept the tested fork candidate unchanged and have not opened a duplicate PR or changed your branch.

@0pcom

0pcom commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

Agreed the two must compose, so the composition is now built and tested rather than argued: main...0magnet:net:client-transport-dispatch-72 is your e12bf8a with this PR's commit rebased on top.

The conflict resolves one way that makes sense: Do loses both the c.Transport short-circuit and the comment #72 adds beside it (the comment stops being true once c.send reads the transport), do's redirect loop stays exactly as #72 wrote it, and each hop's c.send dispatches through c.transport(). Your roundTrip Host fallback stays live on the default path — on non-js targets DefaultTransport.RoundTrip is that same package-level dialer.

The combined branch also adds the coverage you named: a redirecting fake RoundTripper driven through both an explicit Transport and a swapped DefaultTransport (two hops observed on each), and a cross-host vs same-host redirect pair asserting the Authorization header is stripped and kept respectively — that pair exercises makeHeadersCopier and shouldCopyHeaderOnRedirect through the dispatch. All pass with the dev compiler (tinygo test net net/http), and a js/wasm client build still compiles. Those tests only make sense once both PRs exist, so they live on the combined branch, not here.

Proposed order: #72 lands first — it is older and carries the bulk — and this PR then rebases to the combined branch above, which is ready to fast-forward. If a maintainer merges this one first instead, the same resolution applies from the other side and either of us can push it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

net/http: client always uses TinyGo roundTrip instead of roundTrip_js in wasm

2 participants