net, net/http, net/http/httptest: HTTP GET on local HTTP server fails with "fetch failed" on js/wasm with Node.js 18

[dmitshur]

Some Go packages use a local HTTP server in examples. To make these work, the js/wasm port includes:

go/src/net/net_fake.go

Line 5 in bae7d77

// Fake networking for js/wasm. It is intended to allow tests of other package to pass.

It works okay with Node.js 14, but fails with Node.js 18. Without it working, tests/examples in packages like compress/gzip and various others fail. This is the tracking issue this problem.

Tested at tip (79cdecc) with local patches to work around #56860 and #57516. Those issues will need to resolved first; I'm just reporting this finding earlier since I came across it while looking briefly into what's needed to make all.bash pass with Node 18.

It can be reproduced with GOOS=js GOARCH=wasm ./all.bash, or GOOS=js GOARCH=wasm go test -run='Example_compressingReader' compress/gzip, or with this more standalone program:

package main

import (
	"flag"
	"fmt"
	"io"
	"log"
	"net/http"
	"net/http/httptest"
	"os"
)

func main() {
	flag.Parse()

	err := run()
	if err != nil {
		log.Fatalln(err)
	}
}

func run() error {
	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
		fmt.Fprintln(w, "ok from httptest")
	}))
	defer ts.Close()
	fmt.Println("started a test HTTP server at:", ts.URL)

	resp, err := http.Get(ts.URL)
	if err != nil {
		return err
	}
	defer resp.Body.Close()
	fmt.Println(resp.Status)
	_, err = io.Copy(os.Stdout, resp.Body)
	return err

	// Output with Node.js 14:
	// 200 OK
	// ok from httptest

	// Output with Node.js 18:
	// started a test HTTP server at: http://127.0.0.1:1
	// (node:52134) ExperimentalWarning: The Fetch API is an experimental feature. This feature could change at any time
	// (Use `node --trace-warnings ...` to show where the warning was created)
	// 2023/01/04 16:40:18 Get "http://127.0.0.1:1": net/http: fetch() failed: fetch failed
	// exit status 1
}

CC @golang/js, @golang/wasm, @johanbrandhorst.

[dmitshur]

Also CC @Pryz, @evanphx who've recently joined as Wasm maintainers in #57968. (Thanks!)
Investigating and resolving this issue will help unblock progress on #57614.

[johanbrandhorst]

This is probably related to the new "experimental" fetch functionality in Node 18: https://nodejs.org/de/blog/announcements/v18-release-announce/#fetch-experimental.

[johanbrandhorst]

OK I've confirmed that this is due to the introduction of the fetch API in Node 18. In Node 14, fetch was not defined

go/src/net/http/roundtrip_js.go

Line 45 in 1e12c63

var jsFetchMissing = js.Global().Get("fetch").IsUndefined()

so we would short circuit the Fetch API roundtripper

go/src/net/http/roundtrip_js.go

Lines 55 to 57 in 1e12c63

	if t.Dial != nil || t.DialContext != nil || t.DialTLS != nil || t.DialTLSContext != nil || jsFetchMissing {
	return t.roundTrip(req)
	}

This falls back to the fake network implementation in net_fake.go.

However, with Node 18, all of a sudden fetch is defined, so while on the server side we set up a fake in-memory listener, on the client side we try to make a request to the real local address. I see a few ways forward:

1. Add a special case to disable the fetch API on NodeJS and reintroduce the roundtripper short circuit. We could use the new osinfo.Version() to determine the runtime environment (or just call to process directly). This would be unfortunate because it cripples Go wasm on NodeJS just so we can run the tests with a fake network.
2. Implement a real NodeJS based socket syscall interface (based on https://nodejs.org/api/net.html presumably) so we can run proper networking tests for webassembly architectures (when run under NodeJS). This would probably be a lot of work. It's also not great because Go wasm on NodeJS is not supposed to be special, and we'd have to introduce runtime checks through the codebase to decide whether to lean on NodeJS APIs or return unimplemented errors. It's almost a js+node/wasm at that point.

I'm leaning towards option 1 for now as it means we can migrate to Node 18 with few changes and no net new functionality. At a later stage we could introduce a NodeJS based socket interface if we wanted to, and then reenable fetch for NodeJS. I will prepare a CL for option 1.

Would love @neelance's thoughts on a NodeJS based socket syscall interface, I assume the reason we didn't introduce one originally was because there was no fetch in NodeJS at the time.

Another note I want to quickly make here is that the NodeJS fetch implementation disallows the use of certain ports directly on the client (per the Fetch API spec). If we introduce fetch for NodeJS we have to ensure the server side tests don't use one of the blocked ports.

[gopherbot]

Change https://go.dev/cl/463976 mentions this issue: net/http: disable fetch on NodeJS

[dmitshur]

Thanks for investigating this and figuring out the problem.

Would love @​neelance's thoughts on a NodeJS based socket syscall interface, I assume the reason we didn't introduce one originally was because there was no fetch in NodeJS at the time.

I asked Richard about that fairly recently, and he confirmed that the main reason the fake socket path was used is because that was the easiest path forward at the time. Back then, the NaCL port had the fake socket code implemented, and Node didn't support fetch. By now, with NaCL port being long gone and Node supporting fetch, it might be net simpler to start using fetch. But that is fine to investigate and consider doing if desired later.

Just disabling fetch during tests with Node 18 as done in CL 463976 is good to resolve the immediate issue. Thanks.