From 7cdb0e7352382b649b0f6f3fac05ca31d2cab1e4 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Tue, 18 Oct 2022 12:06:21 -0400 Subject: [PATCH] internal/jsonrpc2_v2: rename Serve to NewServer and eliminate its error return Serve had a misleading name and signature: it did not actually block on serving the connection, and never returned a non-nil error. Updates golang/go#56281. Change-Id: Ia6df0ba20066811b0551df3b3267dff2fffd7881 Reviewed-on: https://go-review.googlesource.com/c/tools/+/443678 Reviewed-by: Alan Donovan gopls-CI: kokoro TryBot-Result: Gopher Robot Auto-Submit: Bryan Mills Run-TryBot: Bryan Mills --- gopls/internal/lsp/lsprpc/binder_test.go | 6 +----- internal/jsonrpc2_v2/jsonrpc2_test.go | 5 +---- internal/jsonrpc2_v2/serve.go | 6 +++--- internal/jsonrpc2_v2/serve_test.go | 16 +++------------- 4 files changed, 8 insertions(+), 25 deletions(-) diff --git a/gopls/internal/lsp/lsprpc/binder_test.go b/gopls/internal/lsp/lsprpc/binder_test.go index 9e9fa3c5ea..3315c3eb77 100644 --- a/gopls/internal/lsp/lsprpc/binder_test.go +++ b/gopls/internal/lsp/lsprpc/binder_test.go @@ -43,11 +43,7 @@ func (e *TestEnv) serve(ctx context.Context, t *testing.T, server jsonrpc2_v2.Bi if err != nil { t.Fatal(err) } - s, err := jsonrpc2_v2.Serve(ctx, l, server) - if err != nil { - l.Close() - t.Fatal(err) - } + s := jsonrpc2_v2.NewServer(ctx, l, server) e.Servers = append(e.Servers, s) return l, s } diff --git a/internal/jsonrpc2_v2/jsonrpc2_test.go b/internal/jsonrpc2_v2/jsonrpc2_test.go index 40c2dd3826..dd8d09c887 100644 --- a/internal/jsonrpc2_v2/jsonrpc2_test.go +++ b/internal/jsonrpc2_v2/jsonrpc2_test.go @@ -136,10 +136,7 @@ func testConnection(t *testing.T, framer jsonrpc2.Framer) { if err != nil { t.Fatal(err) } - server, err := jsonrpc2.Serve(ctx, listener, binder{framer, nil}) - if err != nil { - t.Fatal(err) - } + server := jsonrpc2.NewServer(ctx, listener, binder{framer, nil}) defer func() { listener.Close() server.Wait() diff --git a/internal/jsonrpc2_v2/serve.go b/internal/jsonrpc2_v2/serve.go index 7235be91da..7df785655d 100644 --- a/internal/jsonrpc2_v2/serve.go +++ b/internal/jsonrpc2_v2/serve.go @@ -63,21 +63,21 @@ func Dial(ctx context.Context, dialer Dialer, binder Binder) (*Connection, error return newConnection(ctx, rwc, binder, nil), nil } -// Serve starts a new server listening for incoming connections and returns +// NewServer starts a new server listening for incoming connections and returns // it. // This returns a fully running and connected server, it does not block on // the listener. // You can call Wait to block on the server, or Shutdown to get the sever to // terminate gracefully. // To notice incoming connections, use an intercepting Binder. -func Serve(ctx context.Context, listener Listener, binder Binder) (*Server, error) { +func NewServer(ctx context.Context, listener Listener, binder Binder) *Server { server := &Server{ listener: listener, binder: binder, async: newAsync(), } go server.run(ctx) - return server, nil + return server } // Wait returns only when the server has shut down. diff --git a/internal/jsonrpc2_v2/serve_test.go b/internal/jsonrpc2_v2/serve_test.go index 4901a069ab..21bf0bbd46 100644 --- a/internal/jsonrpc2_v2/serve_test.go +++ b/internal/jsonrpc2_v2/serve_test.go @@ -41,10 +41,7 @@ func TestIdleTimeout(t *testing.T) { listener = jsonrpc2.NewIdleListener(d, listener) defer listener.Close() - server, err := jsonrpc2.Serve(ctx, listener, jsonrpc2.ConnectionOptions{}) - if err != nil { - t.Fatal(err) - } + server := jsonrpc2.NewServer(ctx, listener, jsonrpc2.ConnectionOptions{}) // Exercise some connection/disconnection patterns, and then assert that when // our timer fires, the server exits. @@ -187,12 +184,9 @@ func TestServe(t *testing.T) { } func newFake(t *testing.T, ctx context.Context, l jsonrpc2.Listener) (*jsonrpc2.Connection, func(), error) { - server, err := jsonrpc2.Serve(ctx, l, jsonrpc2.ConnectionOptions{ + server := jsonrpc2.NewServer(ctx, l, jsonrpc2.ConnectionOptions{ Handler: fakeHandler{}, }) - if err != nil { - return nil, nil, err - } client, err := jsonrpc2.Dial(ctx, l.Dialer(), @@ -288,7 +282,7 @@ func TestCloseCallRace(t *testing.T) { pokec := make(chan *jsonrpc2.AsyncCall, 1) - s, err := jsonrpc2.Serve(ctx, listener, jsonrpc2.BinderFunc(func(_ context.Context, srvConn *jsonrpc2.Connection) jsonrpc2.ConnectionOptions { + s := jsonrpc2.NewServer(ctx, listener, jsonrpc2.BinderFunc(func(_ context.Context, srvConn *jsonrpc2.Connection) jsonrpc2.ConnectionOptions { h := jsonrpc2.HandlerFunc(func(ctx context.Context, _ *jsonrpc2.Request) (interface{}, error) { // Start a concurrent call from the server to the client. // The point of this test is to ensure this doesn't deadlock @@ -305,10 +299,6 @@ func TestCloseCallRace(t *testing.T) { }) return jsonrpc2.ConnectionOptions{Handler: h} })) - if err != nil { - listener.Close() - t.Fatal(err) - } dialConn, err := jsonrpc2.Dial(ctx, listener.Dialer(), jsonrpc2.ConnectionOptions{}) if err != nil {