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

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.

Comment From: seankhliao

cc @golang/wasm @neild

Comment From: anitschke

@golang/wasm @neild This is an issue that I reported two years ago and we still run into on occasion. I ran the reproduction steps in go 1.23.0 and it seems like this is still an issue. Would you all be ok with me opening a PR as per the contribution guide with the fixed I proposed? Or is there some further investigation that needs to be done by the Go team first?

Comment From: ianlancetaylor

I don't know anything about this issue, but in general it's always OK to contribute a patch. Thanks.

Comment From: kevinbarabash

I patched my project's local copy of wasm_exec.js:

Before

_resume() {
    if (this.exited) {
        throw new Error("Go program has already exited");
    }
    this._inst.exports.resume();
    if (this.exited) {
        this._resolveExitPromise();
    }
}

After

_resume() {
    if (!this.exited) {
        this._inst.exports.resume();
    }
    if (this.exited) {
        this._resolveExitPromise();
    }
}

Not sure if this this reasonable in all situations or not.

Comment From: gopherbot

Change https://go.dev/cl/680937 mentions this issue: net/http: fix RoundTrip context cancellation for js/wasm