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 unusable in Node.js tests #31559

Open
fabioberger opened this issue Apr 18, 2019 · 3 comments

Comments

@fabioberger
Copy link

commented Apr 18, 2019

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

$ go version
go version go1.12.3 darwin/amd64

Does this issue reproduce with the latest release? Yes.

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

go env
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/fabioberger/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/fabioberger/Documents/projects/0x_project/go"
GOPROXY=""
GORACE=""
GOROOT="/Users/fabioberger/.go"
GOTMPDIR=""
GOTOOLDIR="/Users/fabioberger/.go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/91/m4w_p4fj06vc8vdzwqw7cqfc0000gn/T/go-build121993023=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

I wrote a test that requires a network request to localhost using the net/http library, compiled it to WASM and attempted to run it using Node.js. In order to get it to work in a Node.js environment, I add a shim for fetch by requiring the isomorphic-fetch NPM package.

package main

import (
	"fmt"
	"net/http"
)

func TestRequest(t *testing.T) {

	client := &http.Client{}

	req, err := http.NewRequest("GET", "http://localhost:8545", nil)
	if err != nil {
		t.Fatal(err)
	}

	_, err = client.Do(req)
	if err != nil {
		t.Fatal(err)
	}

}

What did you expect to see?

I expected the request to complete without an error.

What did you see instead?

Post http://localhost:8545: dial tcp: Protocol not available

Digging into roundtrip_js.go, I found this check:

if useFakeNetwork() {
	return t.roundTrip(req)
}

Where useFakeNetwork is defined as:

// useFakeNetwork is used to determine whether the request is made
// by a test and should be made to use the fake in-memory network.
func useFakeNetwork() bool {
	return len(os.Args) > 0 && strings.HasSuffix(os.Args[0], ".test")
}

This logic means that any test making a network request that gets compiled to WASM is re-routed to a fake in-memory network. Non-WASM tests do not exhibit this behavior.

Is there any chance the core devs would consider making this behavior optional? Perhaps via an environment variable?

@agnivade

This comment has been minimized.

Copy link
Member

commented Apr 19, 2019

In order to get it to work in a Node.js environment, I add a shim for fetch by requiring the isomorphic-fetch NPM package.

That is the catch here.

@johanbrandhorst @neelance

@johanbrandhorst

This comment has been minimized.

Copy link
Member

commented Apr 19, 2019

In order to get it to work in a Node.js environment, I add a shim for fetch by requiring the isomorphic-fetch NPM package.

That is the catch here.

@johanbrandhorst @neelance

I could be wrong but my impression is that this was just an assertion, not a question. I think the request here is that we make it possible to use the fetch wrapper while running a test.

Unfortunately, before we can do that, we need #26051 finished, so we can reliably run the existing test under js/wasm. The short circuit is there to allow them to pass without relying on a custom runner type. I think we could consider this a dupe of #26358.

@julieqiu julieqiu added this to the Go1.13 milestone Apr 22, 2019

@fabioberger

This comment has been minimized.

Copy link
Author

commented Apr 23, 2019

@johanbrandhorst is spot on, thanks for chiming in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.