net/http: add js/wasm compatible DefaultTransport #25550

johanbrandhorst · 2018-05-24T21:50:55Z

Adds a new Transport type for the js/wasm target that uses the
JavaScript Fetch API for sending HTTP requests. Support for
streaming response bodies is used when available, falling back
to reading the entire response into memory at once.

Updates #25506

johanbrandhorst · 2018-05-24T21:51:58Z

I've tried to push this up sooner rather than later to start discussing the implementation. I am aware the tests probably do not pass yet, but I will work with @neelance and @shurcooL to get my local environment up to scratch.

gopherbot · 2018-05-24T21:52:19Z

This PR (HEAD: 33bfd52) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/#/c/go/+/114515 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

gopherbot · 2018-05-24T21:59:38Z

This PR (HEAD: b1f46d7) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/#/c/go/+/114515 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

gopherbot · 2018-05-24T22:07:55Z

This PR (HEAD: 1bdaafc) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/#/c/go/+/114515 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

gopherbot · 2018-05-24T22:08:11Z

Message from Gerrit Bot:

Uploaded patch set 3: Commit message was updated.

Please don’t reply on this GitHub thread. Visit golang.org/cl/114515.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-05-24T22:17:28Z

Message from Brad Fitzpatrick:

Patch Set 3: Run-TryBot+1 TryBot-Result+1

(12 comments)

Thanks!

Please don’t reply on this GitHub thread. Visit golang.org/cl/114515.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-05-24T23:05:16Z

Message from Dmitri Shuralyov:

Patch Set 3:

(1 comment)

http.Transport type is documented as:

// A Transport is a low-level primitive for making HTTP and HTTPS requests.
// For high-level functionality, such as cookies and redirects, see Client.

When I originally wrote this functionality in GopherJS, I started by doing it at the http.RoundTripper level and was hoping to implement as much of its semantics as possible. Getting response body streaming was my main motivation, and the XHR transport was already implemented at http.RoundTripper level too, so it was a quick decision.

Over time, I learned that both XHR and Fetch browser APIs are higher level and don't allow making individual HTTP requests. Due to security reasons and things like CORS, the frontend code doesn't actually ever get Redirect responses, only the final redirect destination. Fetch also deals with caching, credentials, etc.

I didn't want to touch this decision for GopherJS because it was more of a prototype, but since this is the real Go tree, perhaps we should revisit the decision of what abstraction level to implement this at. I haven't prototyped it yet, but given what I've seen so far, I suspect that implementing the HTTP client in browsers at the http.Client level may be a closer fit to what browsers allow client code to do.

Thoughts (Brad, Johan, Richard)?

Please don’t reply on this GitHub thread. Visit golang.org/cl/114515.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-05-25T09:35:46Z

This PR (HEAD: 7c13eba) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/#/c/go/+/114515 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

gopherbot · 2018-05-25T09:44:25Z

Message from Johan Brandhorst:

Patch Set 4:

(7 comments)

Redesigned following Brad's suggestions.

Please don’t reply on this GitHub thread. Visit golang.org/cl/114515.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-05-25T09:44:31Z

This PR (HEAD: f847d94) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/#/c/go/+/114515 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

gopherbot · 2018-05-25T09:57:15Z

This PR (HEAD: 4532afa) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/#/c/go/+/114515 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

gopherbot · 2018-05-25T10:10:43Z

Message from Johan Brandhorst:

Patch Set 6:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/114515.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-05-25T10:14:53Z

Message from Johan Brandhorst:

Patch Set 6:

I'm slightly confused, as with the changes applied I can't seem to run ./all.bash. I'm seeing:

johan@johan-x1 ~/gows/src/github.com/johanbrandhorst/go/src $ ./all.bash
Building Go cmd/dist using /usr/local/go/.
Building Go toolchain1 using /usr/local/go/.
Building Go bootstrap cmd/go (go_bootstrap) using Go toolchain1.
Building Go toolchain2 using go_bootstrap and Go toolchain1.
Building Go toolchain3 using go_bootstrap and Go toolchain2.
Building packages and commands for linux/amd64.

net/http

src/net/http/client.go:307:3: impossible type switch case: rt (type RoundTripper) cannot have dynamic type *Transport (wrong type for RoundTrip method)
have roundTrip(*Request) (*Response, error)
want RoundTrip(*Request) (*Response, error)
src/net/http/transport.go:39:5: cannot use Transport literal (type *Transport) as type RoundTripper in assignment:
*Transport does not implement RoundTripper (missing RoundTrip method)
have roundTrip(*Request) (*Response, error)
want RoundTrip(*Request) (*Response, error)
go tool dist: FAILED: /home/johan/gows/src/github.com/johanbrandhorst/go/pkg/tool/linux_amd64/go_bootstrap install -gcflags=all= -ldflags=all= std cmd: exit status 2

Now I think I have correctly defined Transport both in the case of js,wasm and !js,wasm. Am I missing anything obvious?

Please don’t reply on this GitHub thread. Visit golang.org/cl/114515.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-05-25T10:14:53Z

Message from Johan Brandhorst:

Patch Set 6:

Patch Set 3:

(1 comment)

http.Transport type is documented as:
// A Transport is a low-level primitive for making HTTP and HTTPS requests.
// For high-level functionality, such as cookies and redirects, see Client.
When I originally wrote this functionality in GopherJS, I started by doing it at the http.RoundTripper level and was hoping to implement as much of its semantics as possible. Getting response body streaming was my main motivation, and the XHR transport was already implemented at http.RoundTripper level too, so it was a quick decision.

Over time, I learned that both XHR and Fetch browser APIs are higher level and don't allow making individual HTTP requests. Due to security reasons and things like CORS, the frontend code doesn't actually ever get Redirect responses, only the final redirect destination. Fetch also deals with caching, credentials, etc.

I didn't want to touch this decision for GopherJS because it was more of a prototype, but since this is the real Go tree, perhaps we should revisit the decision of what abstraction level to implement this at. I haven't prototyped it yet, but given what I've seen so far, I suspect that implementing the HTTP client in browsers at the http.Client level may be a closer fit to what browsers allow client code to do.

Thoughts (Brad, Johan, Richard)?

I'm not sure I understand how this would apply here - wouldn't we still need a custom Transport implementation? If we defined a js,wasm specific client presumably it'd still need to have all the same fields as the normal client.

Please don’t reply on this GitHub thread. Visit golang.org/cl/114515.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-05-25T19:55:29Z

Message from Dmitri Shuralyov:

Patch Set 6:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/114515.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-05-25T21:24:47Z

This PR (HEAD: dd97b7f) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/#/c/go/+/114515 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

gopherbot · 2018-05-25T21:26:18Z

Message from Johan Brandhorst:

Patch Set 7:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/114515.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-05-25T21:28:53Z

Message from Johan Brandhorst:

Patch Set 7:

(6 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/114515.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-05-25T21:30:50Z

This PR (HEAD: 254d2df) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/#/c/go/+/114515 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

gopherbot · 2018-05-27T15:24:42Z

Message from Johan Brandhorst:

Patch Set 8:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/114515.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-05-28T05:50:58Z

Message from Agniva De Sarker:

Patch Set 8:

(3 comments)

A minor review from a wasm enthusiast. Thanks for your work. :)

Please don’t reply on this GitHub thread. Visit golang.org/cl/114515.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-05-28T08:19:05Z

This PR (HEAD: a3dde72) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/#/c/go/+/114515 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

gopherbot · 2018-05-28T08:19:31Z

Message from Johan Brandhorst:

Patch Set 8:

(3 comments)

Patch Set 8:

(3 comments)

A minor review from a wasm enthusiast. Thanks for your work. :)

Thanks, great points all!

Please don’t reply on this GitHub thread. Visit golang.org/cl/114515.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-05-28T08:21:21Z

Message from Johan Brandhorst:

Patch Set 9:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/114515.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-05-28T08:23:34Z

Message from Johan Brandhorst:

Patch Set 9:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/114515.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-05-28T13:03:26Z

Message from Agniva De Sarker:

Patch Set 9:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/114515.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-05-28T15:04:02Z

Message from Johan Brandhorst:

Patch Set 9:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/114515.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-05-29T20:50:51Z

This PR (HEAD: c068b7f) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/#/c/go/+/114515 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

gopherbot · 2018-05-29T21:03:23Z

Message from Brad Fitzpatrick:

Patch Set 10: Run-TryBot+1 Code-Review+1

(6 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/114515.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-05-29T21:03:23Z

Message from Gobot Gobot:

Patch Set 10:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=509e3082

Please don’t reply on this GitHub thread. Visit golang.org/cl/114515.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-05-29T21:05:13Z

Message from Brad Fitzpatrick:

Patch Set 10:

(2 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/114515.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-05-29T21:05:50Z

Message from Brad Fitzpatrick:

Patch Set 10:

Patch Set 3:

(1 comment)

http.Transport type is documented as:
// A Transport is a low-level primitive for making HTTP and HTTPS requests.
// For high-level functionality, such as cookies and redirects, see Client.
When I originally wrote this functionality in GopherJS, I started by doing it at the http.RoundTripper level and was hoping to implement as much of its semantics as possible. Getting response body streaming was my main motivation, and the XHR transport was already implemented at http.RoundTripper level too, so it was a quick decision.

Over time, I learned that both XHR and Fetch browser APIs are higher level and don't allow making individual HTTP requests. Due to security reasons and things like CORS, the frontend code doesn't actually ever get Redirect responses, only the final redirect destination. Fetch also deals with caching, credentials, etc.

I didn't want to touch this decision for GopherJS because it was more of a prototype, but since this is the real Go tree, perhaps we should revisit the decision of what abstraction level to implement this at. I haven't prototyped it yet, but given what I've seen so far, I suspect that implementing the HTTP client in browsers at the http.Client level may be a closer fit to what browsers allow client code to do.

Thoughts (Brad, Johan, Richard)?

Good point, but I'm more concerned with more code working by default without changes, so I think this is okay to violate for now in Go 1.11. We can revisit in Go 1.12 & later.

Please don’t reply on this GitHub thread. Visit golang.org/cl/114515.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-05-29T21:10:44Z

Message from Gobot Gobot:

Patch Set 10:

Build is still in progress...
This change failed on linux-amd64:
See https://storage.googleapis.com/go-build-log/509e3082/linux-amd64_79524419.log

Consult https://build.golang.org/ to see whether it's a new failure. Other builds still in progress; subsequent failure notices suppressed until final report.

Please don’t reply on this GitHub thread. Visit golang.org/cl/114515.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-05-29T21:10:58Z

Message from Gobot Gobot:

Patch Set 10: TryBot-Result-1

7 of 17 TryBots failed:
Failed on linux-amd64: https://storage.googleapis.com/go-build-log/509e3082/linux-amd64_79524419.log
Failed on linux-386: https://storage.googleapis.com/go-build-log/509e3082/linux-386_89d4285c.log
Failed on freebsd-amd64-11_1: https://storage.googleapis.com/go-build-log/509e3082/freebsd-amd64-11_1_7f0993ef.log
Failed on windows-386-2008: https://storage.googleapis.com/go-build-log/509e3082/windows-386-2008_62d01cea.log
Failed on linux-amd64-race: https://storage.googleapis.com/go-build-log/509e3082/linux-amd64-race_4c8daf21.log
Failed on openbsd-amd64-62: https://storage.googleapis.com/go-build-log/509e3082/openbsd-amd64-62_34429510.log
Failed on windows-amd64-2016: https://storage.googleapis.com/go-build-log/509e3082/windows-amd64-2016_6dc87715.log

Consult https://build.golang.org/ to see whether they are new failures.

Please don’t reply on this GitHub thread. Visit golang.org/cl/114515.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-05-30T08:03:26Z

This PR (HEAD: 630957c) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/#/c/go/+/114515 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

gopherbot · 2018-05-30T08:03:50Z

Message from Johan Brandhorst:

Patch Set 10:

(8 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/114515.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-05-30T08:05:58Z

This PR (HEAD: 49b5acc) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/#/c/go/+/114515 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

gopherbot · 2018-05-30T08:09:41Z

This PR (HEAD: a710973) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/#/c/go/+/114515 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

gopherbot · 2018-05-30T16:53:40Z

Message from Brad Fitzpatrick:

Patch Set 13: Run-TryBot+1

Please don’t reply on this GitHub thread. Visit golang.org/cl/114515.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-05-30T16:53:52Z

Message from Gobot Gobot:

Patch Set 13:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=80f0fd2f

Please don’t reply on this GitHub thread. Visit golang.org/cl/114515.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-05-30T17:03:18Z

Message from Gobot Gobot:

Patch Set 13: TryBot-Result+1

TryBots are happy.

Please don’t reply on this GitHub thread. Visit golang.org/cl/114515.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-05-30T17:07:26Z

Message from Brad Fitzpatrick:

Patch Set 13: Code-Review+1

(2 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/114515.
After addressing review feedback, remember to publish your drafts!

Adds a new Transport type for the js/wasm target that uses the JavaScript Fetch API for sending HTTP requests. Support for streaming response bodies is used when available, falling back to reading the entire response into memory at once.

gopherbot · 2018-05-30T17:11:34Z

This PR (HEAD: 6df6467) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/#/c/go/+/114515 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

gopherbot · 2018-05-30T17:12:10Z

Message from Johan Brandhorst:

Patch Set 13:

(2 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/114515.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-05-30T17:24:45Z

Message from Brad Fitzpatrick:

Patch Set 14: Code-Review+2

Please don’t reply on this GitHub thread. Visit golang.org/cl/114515.
After addressing review feedback, remember to publish your drafts!

Adds a new Transport type for the js/wasm target that uses the JavaScript Fetch API for sending HTTP requests. Support for streaming response bodies is used when available, falling back to reading the entire response into memory at once. Updates #25506 Change-Id: Ie9ea433a1a2ed2f65b03c6cc84a16e70c06fcf5c GitHub-Last-Rev: 6df6467 GitHub-Pull-Request: #25550 Reviewed-on: https://go-review.googlesource.com/114515 Reviewed-by: Brad Fitzpatrick <[email protected]>

gopherbot · 2018-05-30T17:30:04Z

This PR is being closed because golang.org/cl/114515 has been merged.

Adds a new Transport type for the js/wasm target that uses the JavaScript Fetch API for sending HTTP requests. Support for streaming response bodies is used when available, falling back to reading the entire response into memory at once. Updates #25506 Change-Id: Ie9ea433a1a2ed2f65b03c6cc84a16e70c06fcf5c GitHub-Last-Rev: 6df646745b8e0474781f4b1a3084536e573e8e8c GitHub-Pull-Request: golang/go#25550 Reviewed-on: https://go-review.googlesource.com/114515 Reviewed-by: Brad Fitzpatrick <[email protected]>

googlebot added the cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change. label May 24, 2018

gopherbot closed this May 30, 2018

johanbrandhorst mentioned this pull request Jun 25, 2018

Experimental: Add grpc-web HTTP client grpc/grpc-go#2174

Closed

sekulicd mentioned this pull request Aug 28, 2024

net/http: js-wasm in nodejs HTTP requests fail #69106

Open

net/http: add js/wasm compatible DefaultTransport #25550

net/http: add js/wasm compatible DefaultTransport #25550

Conversation

johanbrandhorst commented May 24, 2018

johanbrandhorst commented May 24, 2018

gopherbot commented May 24, 2018

gopherbot commented May 24, 2018

gopherbot commented May 24, 2018

gopherbot commented May 24, 2018

gopherbot commented May 24, 2018

gopherbot commented May 24, 2018

gopherbot commented May 25, 2018

gopherbot commented May 25, 2018

gopherbot commented May 25, 2018

gopherbot commented May 25, 2018

gopherbot commented May 25, 2018

gopherbot commented May 25, 2018

net/http

gopherbot commented May 25, 2018

gopherbot commented May 25, 2018

gopherbot commented May 25, 2018

gopherbot commented May 25, 2018

gopherbot commented May 25, 2018

gopherbot commented May 25, 2018

gopherbot commented May 27, 2018

gopherbot commented May 28, 2018

gopherbot commented May 28, 2018

gopherbot commented May 28, 2018

gopherbot commented May 28, 2018

gopherbot commented May 28, 2018

gopherbot commented May 28, 2018

gopherbot commented May 28, 2018

gopherbot commented May 29, 2018

gopherbot commented May 29, 2018

gopherbot commented May 29, 2018

gopherbot commented May 29, 2018

gopherbot commented May 29, 2018

gopherbot commented May 29, 2018

gopherbot commented May 29, 2018

gopherbot commented May 30, 2018

gopherbot commented May 30, 2018

gopherbot commented May 30, 2018

gopherbot commented May 30, 2018

gopherbot commented May 30, 2018

gopherbot commented May 30, 2018

gopherbot commented May 30, 2018

gopherbot commented May 30, 2018

gopherbot commented May 30, 2018

gopherbot commented May 30, 2018

gopherbot commented May 30, 2018

gopherbot commented May 30, 2018