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

net/http: roundtrip_js.go fails to handle fetch() Promise if RoundTrip context is canceled #57098

Open
anitschke opened this issue Dec 6, 2022 · 1 comment
Labels
arch-wasm WebAssembly issues NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-JS
Milestone

Comments

@anitschke
Copy link

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

$ go version
go version go1.18.1 linux/amd64

Does this issue reproduce with the latest release? Yes

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/anitschk/.cache/go-build"
GOENV="/home/anitschk/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/local-ssd/anitschk/go/pkg/mod"
GONOPROXY="github.mathworks.com"
GONOSUMDB="github.mathworks.com,golang.dhcp"
GOOS="linux"
GOPATH="/local-ssd/anitschk/go/"
GOPRIVATE="github.mathworks.com,golang.dhcp"
GOPROXY="http://iat-go-proxy-prod-01:7000/go-proxy"
GOROOT="/mathworks/hub/3rdparty/R2022b/8461447/glnxa64/golang"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/mathworks/hub/3rdparty/R2022b/8461447/glnxa64/golang/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.18.1"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build3452488547=/tmp/go-build -gno-record-gcc-switches"

What did you do?

See the following sample reproduction program

This sample program consists of two binaries:

  • cmd/wasm/main.go a simple binary that sends a request to GET http://localhost:9091, logging the result or the error and then exits. The key thing here is the GET has a context that times out after a second.
  • cmd/server/main.go which is simple web server that serves two ports:
    • :9090 serves up a simple index.html file that uses the above wasm binary
    • :9091 is "slow" and takes 10 seconds before responding with "ok" (so cmd/wasm/main.go will always timeout)

It is also worth noting that the index.html will console.log('WASM Done') once cmd/wasm/main.go has exited.

What did you expect to see?

When I run ./server and then visit http://localhost:9090 I expect the wasm binary to reach out to :9091 but timeout after 1 second. So it should show the following in the debugger console:

wasm_exec.js:22 Go Web Assembly
wasm_exec.js:22 Error Get "http://localhost:9091": context deadline exceeded (Client.Timeout exceeded while awaiting headers)
(index):8 WASM Done

What did you see instead?

Instead I see the following error get thrown to the debugger console after the WASM program exits:

wasm_exec.js:22 Go Web Assembly
wasm_exec.js:22 Error Get "http://localhost:9091": context deadline exceeded (Client.Timeout exceeded while awaiting headers)
(index):8 WASM Done
wasm_exec.js:536 Uncaught (in promise) Error: Go program has already exited
    at globalThis.Go._resume (wasm_exec.js:536:11)
    at wasm_exec.js:549:8

I can reproduce this when loading the webpage from both firefox 102.5.0esr and chrome 106.0.5249.119

Problem

The problem is when we do ac.Call("abort") it results in the fetch() promise being rejected.

See: https://developer.mozilla.org/en-US/docs/Web/API/AbortController

Note: When abort() is called, the fetch() promise rejects with a DOMException named AbortError.

So ac.Call("abort") aborts the fetch() and then Transport.RoundTrip exits imediatly. In the sample program the WASM main.go also exits right away. After it exits the fetch() promise gets a chance to be rejected which attempts to call back into the Go WASM to call the failure function. But the Go WASM has already exited, so we see the "Error: Go program has already exited"

Fix

One possible fix would be to replace
ac.Call("abort")
with

ac.Call("abort")
select {
    case <- respCh :
        resp.Body.Close()
    case <- errCh :
}

This way we wait for the fetch() promise to be rejected prior to exiting out of Transport.RoundTrip.

But why the case <- respCh part too? I am not entirely sure if this is necessary, however I could see there being a race where if ac.Call("abort") is called right when fetch() also successfully resolves the promise. If this is possible then respCh might be written to instead of errCh and if we don't also have case <- respCh then we would deadlock because errCh would never be written to.

I tried this fix out and it seems to resolve the issue for me in both firefox and chrome, not sure if there might be any other fallout though.

@anitschke anitschke changed the title net/http: roundtrip_js.go fails to handle Promise if RoundTrip context is canceled net/http: roundtrip_js.go fails to handle fetch() Promise if RoundTrip context is canceled Dec 6, 2022
@seankhliao seankhliao 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 Dec 6, 2022
@seankhliao
Copy link
Member

cc @golang/wasm @neild

@seankhliao seankhliao added this to the Unplanned milestone Dec 8, 2022
@dmitshur dmitshur added the OS-JS label Jan 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch-wasm WebAssembly issues NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-JS
Projects
None yet
Development

No branches or pull requests

3 participants