As documented: OnRecv causes all requests received on conn to invoke
f(req, nil) and all responses to invoke f(req, resp).
Since OnRecv is called with both *Request and *Response being non-nil
when we're handling a response, we need to check that *Response is
non-nil before we check *Request is non-nil. This change just swaps the
two cases in the switch statement to fix the issue. For consistency,
I've swapped the cases for OnSend also, even when it's not needed.
The sending mutex may be blocked due to the underlying conn blocking. If
that happens then we can't call close since close also attempts to hold
the sending mutex. Sending mutex is only used for serializing sends and
doesn't protect the fields close reads/writes. I believe we introduced
locking it so we would return ErrClosed. This commit instead introduces
a check in send which rechecks if we have since closed while attempting
to send.
Test Plan: expanded the test coverage
With this commit, the JSON encoding of `Request` always omits the params
member when calling `Conn.Call`, `Conn.DispatchCall`, or `Conn.Notify`
with the `params` argument set to `nil`. This change also removes the
`OmitNilParams` call option that was added in commit 8012d496 (#62).
As of this commit, if users desire to send a JSON-RPC request with a
`params` value of `null`, then they may do so by explicitly setting the
`params` argument of `Conn.Call`/`Conn.DispatchCall`/`Conn.Notify` to
`json.RawMessage("null")`.
The JSON-RPC 2.0 specification allows the data member of an error object
to be omitted. Before this commit, if Error.Data was nil then the JSON
encoding was "null". That means that the data member was included in
every JSON-RPC error object, even when the data member was not
explicitly set.
This commit adds the omitempty struct tag to the Error.Data field,
meaning that the data member is omitted from the JSON encoding unless
explicitly set with the Error.SetError() method. Beware that this is a
breaking change for clients that may have strict null checks on the data
member.
This merge request moves some of the contents from the jsonrpc2.go file
into their own designated file. The new files being introduced
(excluding test files) are as follows:
* conn.go
* request.go
* response.go
The motive of this change is to make it easier to navigate the code.
Without this change, the jsonrpc2.go file is 813 lines of code.
Before this commit, the underlying connection of `Conn` was not being
closed when a protocol error was encountered. This behavior contradicted
with `Conn.DisconnectNotify()` because it reported that the underlying
connection was being closed. Additionally, the underlying connection was
now orphaned because `Conn` was no longer processing any of the
subsequent requests.
With this commit, the underlying connection is now being closed when a
protocol error is encountered, matching what `Conn.DisconnectNotify()`
has already been reporting.
The CI pipeline has been broken in this project for some time because
the latest version of staticcheck is not compatible with every version
of Go. Through trial and error, it was discovered that staticcheck
v0.2.2 is the latest version that is compatible with Go 1.16.
The authors of staticcheck also recommend pinning the version in CI
pipelines to prevent unintentional breakage of the build [1].
References
[1]: https://staticcheck.io/docs/running-staticcheck/ci/github-actions/#version
The JSON-RPC 2.0 specification allows the params member of a request to
be omitted [1]. Before this commit, this library did not allow the
params member to be omitted when sending a request. When the params
argument of the Conn.Call/Conn.DispatchCall or Conn.Notify method was
set to nil, then Request.Params was set to the JSON encoding of nil
which is null.
This commit adds a CallOption named OmitNilParams. If OmitNilParams is
used when sending a request with params set to nil, then the params
member in the JSON encoding of Request is omitted. If the OmitNilParams
option is not used then the previous behavior is maintained. In other
words, the changes in this commit are backwards compatible.
References
[1]: https://www.jsonrpc.org/specification#request_object
This change fixes a bug that causes PlainObjectCodec to
lose additional messages from stream. json.Decoder has
an internal buffer that reads more than one message, but
is discarded after every use. Now PlainObjectCodec reuses
encoder and decoder within a buffered stream, however
using it directly in your code retains the old, incorrect
behaviour.
A user should now use plainObjectStream if he needs
plain JSON-RPC 2.0 stream without headers.
`NewPlainObjectStream` method has been added for this reason.
Before this commit, a custom logger could be set with the LogMessages
function. However, by using LogMessages not only is a custom logger set
but also all received and sent messages are logged. Use cases exist
where a custom logger is desired to log errors but not all messages.
This change makes the treatment of params and meta the same, by
assigning a well-known pointer at first to detect if the unmarshaling
process overwrites it with an explicit nil, or it stays the same in
which it means that it was unset from the beginning.
Before, the Call method dispatched the JSON-RPC request and waited
until completion of the request before returning. This made it difficult
to release any locks that need to be released upon dispatch.
Now, the Call method is broken into a DispatchCall and Wait pair.
DispatchCall returns a proxy to the internal call object as soon as the
request is dispatched. When appropriate, the caller can invoke the Wait
method on this proxy to block until the result is ready.
The original Call method is not changed, but now implemented by
these primitives.
The 'result' key MUST be unset according when the error key is set.
This is not what is happening right now. When the error is set,
"result":null is returned in the response payload. This patch is fixing
the issue by adding omitempty for the result field.
NOTE: This is a breaking change to the expected contract of Handler. Please update your implementation to use AsyncHandler if needs be.
We have strict ordering requirements of how we handle FileSystem requests in
LSP. As such relying on the ordering the goroutine scheduler runs requests in
leads to potential out of order mutations to the FS. As such we update the
jsonrpc2 implementation to by default block until Handle returns (note it can
still respond to the request at a later stage). For more simple use cases we
provide the AsyncHandler which will work like the previous implementation.
* Ensure handle is blocking
