internal/jsonrpc2_v2: clarify documentation

For golang/go#46520

Change-Id: Id9cdb539ae6f16e03d02f3b00b0b5ee06042a42f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/388594
Trust: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

internal/jsonrpc2_v2/conn.go

@@ -22,7 +22,8 @@ import (
 // ConnectionOptions itself implements Binder returning itself unmodified, to
 // allow for the simple cases where no per connection information is needed.
 type Binder interface {
-	// Bind is invoked when creating a new connection.
+	// Bind returns the ConnectionOptions to use when establishing the passed-in
+	// Connection.
 	// The connection is not ready to use when Bind is called.
 	Bind(context.Context, *Connection) (ConnectionOptions, error)
 }
@@ -234,10 +235,10 @@ func (a *AsyncCall) Await(ctx context.Context, result interface{}) error {
 	return json.Unmarshal(r.result, result)
 }
 
-// Respond deliverers a response to an incoming Call.
-// It is an error to not call this exactly once for any message for which a
-// handler has previously returned ErrAsyncResponse. It is also an error to
-// call this for any other message.
+// Respond delivers a response to an incoming Call.
+//
+// Respond must be called exactly once for any message for which a handler
+// returns ErrAsyncResponse. It must not be called for any other message.
 func (c *Connection) Respond(id ID, result interface{}, rerr error) error {
 	pending := <-c.incomingBox
 	defer func() { c.incomingBox <- pending }()

internal/jsonrpc2_v2/jsonrpc2.go

@@ -15,11 +15,19 @@ import (
 var (
 	// ErrIdleTimeout is returned when serving timed out waiting for new connections.
 	ErrIdleTimeout = errors.New("timed out waiting for new connections")
-	// ErrNotHandled is returned from a handler to indicate it did not handle the
-	// message.
+
+	// ErrNotHandled is returned from a Handler or Preempter to indicate it did
+	// not handle the request.
+	//
+	// If a Handler returns ErrNotHandled, the server replies with
+	// ErrMethodNotFound.
 	ErrNotHandled = errors.New("JSON RPC not handled")
+
 	// ErrAsyncResponse is returned from a handler to indicate it will generate a
 	// response asynchronously.
+	//
+	// ErrAsyncResponse must not be returned for notifications,
+	// which do not receive responses.
 	ErrAsyncResponse = errors.New("JSON RPC asynchronous response")
 )
 
@@ -28,17 +36,33 @@ var (
 // Primarily this is used for cancel handlers or notifications for which out of
 // order processing is not an issue.
 type Preempter interface {
-	// Preempt is invoked for each incoming request before it is queued.
-	// If the request is a call, it must return a value or an error for the reply.
-	// Preempt should not block or start any new messages on the connection.
-	Preempt(ctx context.Context, req *Request) (interface{}, error)
+	// Preempt is invoked for each incoming request before it is queued for handling.
+	//
+	// If Preempt returns ErrNotHandled, the request will be queued,
+	// and eventually passed to a Handle call.
+	//
+	// Otherwise, the result and error are processed as if returned by Handle.
+	//
+	// Preempt must not block. (The Context passed to it is for Values only.)
+	Preempt(ctx context.Context, req *Request) (result interface{}, err error)
 }
 
 // Handler handles messages on a connection.
 type Handler interface {
-	// Handle is invoked for each incoming request.
-	// If the request is a call, it must return a value or an error for the reply.
-	Handle(ctx context.Context, req *Request) (interface{}, error)
+	// Handle is invoked sequentially for each incoming request that has not
+	// already been handled by a Preempter.
+	//
+	// If the Request has a nil ID, Handle must return a nil result,
+	// and any error may be logged but will not be reported to the caller.
+	//
+	// If the Request has a non-nil ID, Handle must return either a
+	// non-nil, JSON-marshalable result, or a non-nil error.
+	//
+	// The Context passed to Handle will be canceled if the
+	// connection is broken or the request is canceled or completed.
+	// (If Handle returns ErrAsyncResponse, ctx will remain uncanceled
+	// until either Cancel or Respond is called for the request's ID.)
+	Handle(ctx context.Context, req *Request) (result interface{}, err error)
 }
 
 type defaultHandler struct{}
@@ -60,15 +84,15 @@ func (f HandlerFunc) Handle(ctx context.Context, req *Request) (interface{}, err
 // async is a small helper for operations with an asynchronous result that you
 // can wait for.
 type async struct {
-	ready  chan struct{} // signals that the operation has completed
-	errBox chan error    // guards the operation result
+	ready    chan struct{} // closed when done
+	firstErr chan error    // 1-buffered; contains either nil or the first non-nil error
 }
 
 func newAsync() *async {
 	var a async
 	a.ready = make(chan struct{})
-	a.errBox = make(chan error, 1)
-	a.errBox <- nil
+	a.firstErr = make(chan error, 1)
+	a.firstErr <- nil
 	return &a
 }
 
@@ -87,15 +111,15 @@ func (a *async) isDone() bool {
 
 func (a *async) wait() error {
 	<-a.ready
-	err := <-a.errBox
-	a.errBox <- err
+	err := <-a.firstErr
+	a.firstErr <- err
 	return err
 }
 
 func (a *async) setError(err error) {
-	storedErr := <-a.errBox
+	storedErr := <-a.firstErr
 	if storedErr == nil {
 		storedErr = err
 	}
-	a.errBox <- storedErr
+	a.firstErr <- storedErr
 }

internal/jsonrpc2_v2/serve.go

@@ -18,17 +18,17 @@ import (
 
 // Listener is implemented by protocols to accept new inbound connections.
 type Listener interface {
-	// Accept an inbound connection to a server.
-	// It must block until an inbound connection is made, or the listener is
-	// shut down.
+	// Accept accepts an inbound connection to a server.
+	// It blocks until either an inbound connection is made, or the listener is closed.
 	Accept(context.Context) (io.ReadWriteCloser, error)
 
-	// Close is used to ask a listener to stop accepting new connections.
+	// Close closes the listener.
+	// Any blocked Accept or Dial operations will unblock and return errors.
 	Close() error
 
 	// Dialer returns a dialer that can be used to connect to this listener
 	// locally.
-	// If a listener does not implement this it will return a nil.
+	// If a listener does not implement this it will return nil.
 	Dialer() Dialer
 }
 

internal/jsonrpc2_v2/wire.go

@@ -12,8 +12,6 @@ import (
 // see http://www.jsonrpc.org/specification for details
 
 var (
-	// ErrUnknown should be used for all non coded errors.
-	ErrUnknown = NewError(-32001, "JSON RPC unknown error")
 	// ErrParse is used when invalid JSON was received by the server.
 	ErrParse = NewError(-32700, "JSON RPC parse error")
 	// ErrInvalidRequest is used when the JSON sent is not a valid Request object.
@@ -28,11 +26,13 @@ var (
 	ErrInternal = NewError(-32603, "JSON RPC internal error")
 
 	// The following errors are not part of the json specification, but
-	// compliant extensions specific to this implimentation.
+	// compliant extensions specific to this implementation.
 
 	// ErrServerOverloaded is returned when a message was refused due to a
 	// server being temporarily unable to accept any new messages.
 	ErrServerOverloaded = NewError(-32000, "JSON RPC overloaded")
+	// ErrUnknown should be used for all non coded errors.
+	ErrUnknown = NewError(-32001, "JSON RPC unknown error")
 )
 
 const wireVersion = "2.0"