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
Open

net/http: roundtrip_js.go unusable in Node.js tests #31559

fabioberger opened this issue Apr 18, 2019 · 3 comments
Labels
arch-wasm WebAssembly issues NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@fabioberger
Copy link

fabioberger 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
Copy link
Contributor

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
Copy link
Member

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 the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 22, 2019
@julieqiu julieqiu added this to the Go1.13 milestone Apr 22, 2019
@fabioberger
Copy link
Author

@johanbrandhorst is spot on, thanks for chiming in.

@dmitshur dmitshur added the arch-wasm WebAssembly issues label Apr 29, 2019
@andybons andybons modified the milestones: Go1.13, Go1.14 Jul 8, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
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.
Projects
None yet
Development

No branches or pull requests

7 participants