Create memhttp package to debug flaky testcases (#594)

Creates two internal packages: `memhttp` and `memhttptest` based on
https://github.com/akshayjshah/memhttp . We use this for testing
connect's services.

Changes`httptest.Server` to use an in memory network. This makes testing
more robust to ephemeral port issues under load. Two flaky test cases
were now easily reproducible and fixed. Both occurred from races between
starting the network request in a go routine and checking for a http2
stream and erroring if not. This can error on Send or Receive depending
on how fast the request can complete.

On `duplexHTTPCall` I removed `SetError` to avoid overwriting the
original response error. Now `BlockUntilResponseReady` returns an error
from the response initialization.

See: golang/go#14200

Fixes:
- `TestServer/http1/*/*/cumsum_cancel_before_send`: send can error on
write message.
- `TestBidiRequiresHTTP2`: send can `io.EOF` error on write, or succeed.
- `TestBidiOverHTTP1`: same as above.
- `TestClientPeer/grpcweb`: stream request sends an invalidate payload
invoking server error, races with interceptor.

Two places where errors raced:
-
https://github.com/connectrpc/connect-go/blob/525cf76ebd3c1c54f2cfd2cb4413c44b57539be9/duplex_http_call.go#L290
-
https://github.com/connectrpc/connect-go/blob/525cf76ebd3c1c54f2cfd2cb4413c44b57539be9/handler.go#L184

.golangci.yml

@@ -123,3 +123,10 @@ issues:
     - linters: [revive]
       text: "^if-return: "
       path: error_writer.go
+    # We want to set http.Server's logger
+    - linters: [forbidigo]
+      path: internal/memhttp
+      text: "use of `log.(New|Logger|Lshortfile)` forbidden by pattern .*"
+    # We want to show examples with http.Get
+    - linters: [noctx]
+      path: internal/memhttp/memhttp_test.go

client_example_test.go

@@ -27,9 +27,8 @@ import (
 
 func Example_client() {
 	logger := log.New(os.Stdout, "" /* prefix */, 0 /* flags */)
-	// Unfortunately, pkg.go.dev can't run examples that actually use the
-	// network. To keep this example runnable, we'll use an HTTP server and
-	// client that communicate over in-memory pipes. The client is still a plain
+	// To keep this example runnable, we'll use an HTTP server and client
+	// that communicate over in-memory pipes. The client is still a plain
 	// *http.Client!
 	var httpClient *http.Client = examplePingServer.Client()
 

client_ext_test.go

@@ -18,14 +18,14 @@ import (
 	"context"
 	"errors"
 	"net/http"
-	"net/http/httptest"
 	"strings"
 	"testing"
 
 	connect "connectrpc.com/connect"
 	"connectrpc.com/connect/internal/assert"
 	pingv1 "connectrpc.com/connect/internal/gen/connect/ping/v1"
 	"connectrpc.com/connect/internal/gen/connect/ping/v1/pingv1connect"
+	"connectrpc.com/connect/internal/memhttp/memhttptest"
 )
 
 func TestNewClient_InitFailure(t *testing.T) {
@@ -75,55 +75,56 @@ func TestClientPeer(t *testing.T) {
 	t.Parallel()
 	mux := http.NewServeMux()
 	mux.Handle(pingv1connect.NewPingServiceHandler(pingServer{}))
-	server := httptest.NewUnstartedServer(mux)
-	server.EnableHTTP2 = true
-	server.StartTLS()
-	t.Cleanup(server.Close)
+	server := memhttptest.NewServer(t, mux)
 
 	run := func(t *testing.T, unaryHTTPMethod string, opts ...connect.ClientOption) {
 		t.Helper()
 		client := pingv1connect.NewPingServiceClient(
 			server.Client(),
-			server.URL,
+			server.URL(),
 			connect.WithClientOptions(opts...),
 			connect.WithInterceptors(&assertPeerInterceptor{t}),
 		)
 		ctx := context.Background()
-		// unary
-		unaryReq := connect.NewRequest[pingv1.PingRequest](nil)
-		_, err := client.Ping(ctx, unaryReq)
-		assert.Nil(t, err)
-		assert.Equal(t, unaryHTTPMethod, unaryReq.HTTPMethod())
-		text := strings.Repeat(".", 256)
-		r, err := client.Ping(ctx, connect.NewRequest(&pingv1.PingRequest{Text: text}))
-		assert.Nil(t, err)
-		assert.Equal(t, r.Msg.Text, text)
-		// client streaming
-		clientStream := client.Sum(ctx)
-		t.Cleanup(func() {
-			_, closeErr := clientStream.CloseAndReceive()
-			assert.Nil(t, closeErr)
+		t.Run("unary", func(t *testing.T) {
+			unaryReq := connect.NewRequest[pingv1.PingRequest](nil)
+			_, err := client.Ping(ctx, unaryReq)
+			assert.Nil(t, err)
+			assert.Equal(t, unaryHTTPMethod, unaryReq.HTTPMethod())
+			text := strings.Repeat(".", 256)
+			r, err := client.Ping(ctx, connect.NewRequest(&pingv1.PingRequest{Text: text}))
+			assert.Nil(t, err)
+			assert.Equal(t, r.Msg.Text, text)
 		})
-		assert.NotZero(t, clientStream.Peer().Addr)
-		assert.NotZero(t, clientStream.Peer().Protocol)
-		err = clientStream.Send(&pingv1.SumRequest{})
-		assert.Nil(t, err)
-		// server streaming
-		serverStream, err := client.CountUp(ctx, connect.NewRequest(&pingv1.CountUpRequest{}))
-		t.Cleanup(func() {
-			assert.Nil(t, serverStream.Close())
+		t.Run("client_stream", func(t *testing.T) {
+			clientStream := client.Sum(ctx)
+			t.Cleanup(func() {
+				_, closeErr := clientStream.CloseAndReceive()
+				assert.Nil(t, closeErr)
+			})
+			assert.NotZero(t, clientStream.Peer().Addr)
+			assert.NotZero(t, clientStream.Peer().Protocol)
+			err := clientStream.Send(&pingv1.SumRequest{})
+			assert.Nil(t, err)
 		})
-		assert.Nil(t, err)
-		// bidi streaming
-		bidiStream := client.CumSum(ctx)
-		t.Cleanup(func() {
-			assert.Nil(t, bidiStream.CloseRequest())
-			assert.Nil(t, bidiStream.CloseResponse())
+		t.Run("server_stream", func(t *testing.T) {
+			serverStream, err := client.CountUp(ctx, connect.NewRequest(&pingv1.CountUpRequest{}))
+			t.Cleanup(func() {
+				assert.Nil(t, serverStream.Close())
+			})
+			assert.Nil(t, err)
+		})
+		t.Run("bidi_stream", func(t *testing.T) {
+			bidiStream := client.CumSum(ctx)
+			t.Cleanup(func() {
+				assert.Nil(t, bidiStream.CloseRequest())
+				assert.Nil(t, bidiStream.CloseResponse())
+			})
+			assert.NotZero(t, bidiStream.Peer().Addr)
+			assert.NotZero(t, bidiStream.Peer().Protocol)
+			err := bidiStream.Send(&pingv1.CumSumRequest{})
+			assert.Nil(t, err)
 		})
-		assert.NotZero(t, bidiStream.Peer().Addr)
-		assert.NotZero(t, bidiStream.Peer().Protocol)
-		err = bidiStream.Send(&pingv1.CumSumRequest{})
-		assert.Nil(t, err)
 	}
 
 	t.Run("connect", func(t *testing.T) {
@@ -157,14 +158,10 @@ func TestGetNotModified(t *testing.T) {
 
 	mux := http.NewServeMux()
 	mux.Handle(pingv1connect.NewPingServiceHandler(&notModifiedPingServer{etag: etag}))
-	server := httptest.NewUnstartedServer(mux)
-	server.EnableHTTP2 = true
-	server.StartTLS()
-	t.Cleanup(server.Close)
-
+	server := memhttptest.NewServer(t, mux)
 	client := pingv1connect.NewPingServiceClient(
 		server.Client(),
-		server.URL,
+		server.URL(),
 		connect.WithHTTPGet(),
 	)
 	ctx := context.Background()

client_get_fallback_test.go

@@ -17,12 +17,12 @@ package connect
 import (
 	"context"
 	"net/http"
-	"net/http/httptest"
 	"strings"
 	"testing"
 
 	"connectrpc.com/connect/internal/assert"
 	pingv1 "connectrpc.com/connect/internal/gen/connect/ping/v1"
+	"connectrpc.com/connect/internal/memhttp/memhttptest"
 )
 
 func TestClientUnaryGetFallback(t *testing.T) {
@@ -38,14 +38,11 @@ func TestClientUnaryGetFallback(t *testing.T) {
 		},
 		WithIdempotency(IdempotencyNoSideEffects),
 	))
-	server := httptest.NewUnstartedServer(mux)
-	server.EnableHTTP2 = true
-	server.StartTLS()
-	t.Cleanup(server.Close)
+	server := memhttptest.NewServer(t, mux)
 
 	client := NewClient[pingv1.PingRequest, pingv1.PingResponse](
 		server.Client(),
-		server.URL+"/connect.ping.v1.PingService/Ping",
+		server.URL()+"/connect.ping.v1.PingService/Ping",
 		WithHTTPGet(),
 		WithHTTPGetMaxURLSize(1, true),
 		WithSendGzip(),

compression_test.go

@@ -17,10 +17,10 @@ package connect
 import (
 	"context"
 	"net/http"
-	"net/http/httptest"
 	"testing"
 
 	"connectrpc.com/connect/internal/assert"
+	"connectrpc.com/connect/internal/memhttp/memhttptest"
 	"google.golang.org/protobuf/types/known/emptypb"
 )
 
@@ -42,12 +42,10 @@ func TestAcceptEncodingOrdering(t *testing.T) {
 		w.WriteHeader(http.StatusOK)
 		called = true
 	})
-	server := httptest.NewServer(verify)
-	t.Cleanup(server.Close)
-
+	server := memhttptest.NewServer(t, verify)
 	client := NewClient[emptypb.Empty, emptypb.Empty](
 		server.Client(),
-		server.URL,
+		server.URL(),
 		withFakeBrotli,
 		withGzip(),
 	)