Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

net/http: Expose the normal transport RoundTripper for WASM/js #27495

Closed
antong opened this issue Sep 4, 2018 · 11 comments
Closed

net/http: Expose the normal transport RoundTripper for WASM/js #27495

antong opened this issue Sep 4, 2018 · 11 comments
Labels
arch-wasm WebAssembly issues FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.18

Comments

@antong
Copy link
Contributor

antong commented Sep 4, 2018

On WASM, the standard http.Transfer RoundTripper is not exposed for external use. Buildflags for wasm replace the standard implementation of RoundTripper with a special version that uses the browser Fetch API. The special JS RoundTripper does not seem to use or honor any of the documented settings provided by http.Transport, such as Dial or DialContext. But the way the special JS RoundTripper is implemented as methods on http.Transport, it prevents using the standard RoundTripper.

There are use cases for using the normal RoundTripper, for instance for testing and simulation in the browser. The normal RoundTripper is compiled and included in the binary anyway, and works perfectly well, but is masked by the browser version.

What version of Go are you using (go version)?

go1.11

What operating system and processor architecture are you using (go env)?

GOOS=js
GOARCH=wasm

What did you do?

I tried to configure a http.Client to use a transport with a custom dial function:
https://play.golang.org/p/dnyTivQ643J

This works on all other platforms including the playground, but on WASM, the same, special Roundtripper is always used and there is no way to use the standard Roundtripper.

What did you expect to see?

Get https://api.ipify.org/: expected error, all ok

What did you see instead?

On WASM:

192.0.2.77
Done.

On other platforms, I get the expected result
@andybons andybons added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. arch-wasm WebAssembly issues labels Sep 4, 2018
@andybons andybons added this to the Unplanned milestone Sep 4, 2018
@andybons
Copy link
Member

andybons commented Sep 4, 2018

@neelance @dmitshur

@antong
Copy link
Contributor Author

antong commented Sep 4, 2018

I quickly tested two hacks to do this, and both seemed to work. Both pass the tests on normal platforms, but I don't know how to run the unit tests on WASM.

Hack 1: Don't implement the special RoundTripper on http.Transport, but make an own jsTransport. Set an instance of this as http.DefaultTransport for GOARCH=wasm. Example: e8050da

Hack 2: Mod the special RoundTripper to instead use the normal RoundTripper if the caller has set options on the Transport. Example: 8137bf5

@agnivade
Copy link
Contributor

agnivade commented Sep 4, 2018

@johanbrandhorst

@dmitshur
Copy link
Contributor

dmitshur commented Sep 4, 2018

This is also related to #25695.

@rubensayshi
Copy link

rubensayshi commented Mar 7, 2019 •
edited

running into this problem when trying to do HTTPS requests in wasm/js (using self signed certs over a websocket connection).

e8050da seems like an easy and clean solution to enable a lot more flexibility when desired.

as far as I can tell the only place where http.Client and http.Transport are coupled to something that does not work in wasm/js is the DefaultTransport using net.Dialer{}).DialContext.
Given a working net.Dialer the code in roundTrip actually works just fine.

I understand #25695 discusses a more major problem, but specially considering the impact of fixing that as a whole, maybe a simple fix like this would be a good start?

@johanbrandhorst
Copy link
Member

I agree Hack 1 as proposed by @antong is a reasonable short term approach. Are you interested in contributing this?

@antong
Copy link
Contributor Author

antong commented Mar 7, 2019

Sure! Do you mean that I should make a CL based on Hack 1 (e8050da)?

@johanbrandhorst
Copy link
Member

Yes, that sounds good.

antong added a commit to antong/go that referenced this issue Mar 11, 2019
@antong
net/http: expose standard transport for js
805d8dd 
Implement a separate type jsTransport that implements RoundTripper using
the WHATWG Fetch API, and make it the DefaultTransport on js/wasm. This
allows using the standard Transport implementation on js/wasm by
specifying a DialContext for the Transport. Note that the default would
use net.Dial, that can not be used in the browser.

Before this change the same Transport type was used also on js, but with
a different implementation. This implementation disregarded any of the
documented Transport settings and just used the browser Fetch API. This
made it impossible to use the Transport implementation, e.g., by
supplying a DialContext that works in the browser.

Fixes golang#27495
antong added a commit to antong/go that referenced this issue Mar 11, 2019
@antong
net/http: expose standard transport for js
e77cf4b 
Implement a separate type jsTransport that implements RoundTripper using
the WHATWG Fetch API, and make it the DefaultTransport on js/wasm. This
allows using the standard Transport implementation on js/wasm by
specifying a DialContext for the Transport. Note that the default would
use net.Dial, that can not be used in the browser.

Before this change the same Transport type was used also on js, but with
a different implementation. This implementation disregarded any of the
documented Transport settings and just used the browser Fetch API. This
made it impossible to use the Transport implementation, e.g., by
supplying a DialContext that works in the browser.

Fixes golang#27495
@gopherbot
Copy link

Change https://golang.org/cl/166617 mentions this issue: net/http: expose standard transport for js

@hullarb
Copy link

hullarb commented Dec 8, 2019

Hi all,

i just bumped into this issues as i tried to implement some tunneling for http request in wasm.
It's a pity that the above PR was abandoned.
i also have a very minimalistic change( 8327844) that i could use for testing my http tunneling.

As Transport currently does not provide any ways to customize the wasm/js RoundTrip behavior i wonder if it would not be fair to simple fallback to the "normal" roundTrip implementation in case a custom Transport struct is the caller and not the DeafultTransport (
the js RoundTrip implementation has already a fallback to the real transport RoundTrip implementation in case the fetch api is not available).

@neilalexander neilalexander mentioned this issue Jun 25, 2021
Closed
@gopherbot
Copy link

Change https://golang.org/cl/330852 mentions this issue: net/http: guarantee that the Transport dial functions are respected in js/wasm

@dmitshur dmitshur modified the milestones: Unplanned, Go1.18 Nov 9, 2021
@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Nov 9, 2021
@golang golang locked and limited conversation to collaborators Nov 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
arch-wasm WebAssembly issues FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.18
Development

Successfully merging a pull request may close this issue.

net/http: guarantee that the Transport dial functions are respected in js/wasm neilalexander/go
8 participants
@antong @andybons @rubensayshi @agnivade @dmitshur @hullarb @johanbrandhorst @gopherbot