From 5d35a75079e7b98ad4f79d02ac4372c1ad62334f Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Mon, 28 Feb 2022 10:55:20 -0500 Subject: [PATCH] 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 Run-TryBot: Bryan Mills Reviewed-by: Ian Cottrell gopls-CI: kokoro TryBot-Result: Gopher Robot --- internal/jsonrpc2_v2/conn.go | 11 +++--- internal/jsonrpc2_v2/jsonrpc2.go | 58 ++++++++++++++++++++++---------- internal/jsonrpc2_v2/serve.go | 10 +++--- internal/jsonrpc2_v2/wire.go | 6 ++-- 4 files changed, 55 insertions(+), 30 deletions(-) diff --git a/internal/jsonrpc2_v2/conn.go b/internal/jsonrpc2_v2/conn.go index 606c3f99c..7d492c402 100644 --- a/internal/jsonrpc2_v2/conn.go +++ b/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 }() diff --git a/internal/jsonrpc2_v2/jsonrpc2.go b/internal/jsonrpc2_v2/jsonrpc2.go index 271f42cf2..e68558442 100644 --- a/internal/jsonrpc2_v2/jsonrpc2.go +++ b/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 } diff --git a/internal/jsonrpc2_v2/serve.go b/internal/jsonrpc2_v2/serve.go index 98e8894a6..fb3516635 100644 --- a/internal/jsonrpc2_v2/serve.go +++ b/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 } diff --git a/internal/jsonrpc2_v2/wire.go b/internal/jsonrpc2_v2/wire.go index 97b1ae8d6..4da129ae6 100644 --- a/internal/jsonrpc2_v2/wire.go +++ b/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"