Skip to content

Commit

Permalink
internal/lsp/protocol: ignore reply values with non-nil errors in jso…
Browse files Browse the repository at this point in the history
…nrpc2_v2 adapters

The documentation for jsonrpc2.Replier states that
“[i]f err is set then result will be ignored.”

jsonrpc2_v2 documents no such affordance, and the JSON-RPC 2.0
protocol explicitly requires that the result “MUST NOT exist if there
was an error invoking the method.”

Although CL 388600 already avoids returning values in case of error
(which may also help with escape analysis and/or allocation
efficiency), it seems simplest and least confusing to make the
semantic difference between the jsonrpc2.Handler and
jsonrpc2_v2.Handler explicit in the code.

For golang/go#46520.

Change-Id: If13eb842505d42cbc51c01f5f5e699a549a3a28b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/400054
Run-TryBot: Bryan Mills <[email protected]>
Auto-Submit: Bryan Mills <[email protected]>
gopls-CI: kokoro <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
  • Loading branch information
Bryan C. Mills authored and gopherbot committed Apr 12, 2022
1 parent d5f48fc commit a220087
Showing 1 changed file with 10 additions and 2 deletions.
12 changes: 10 additions & 2 deletions internal/lsp/protocol/protocol.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,11 @@ func ClientHandlerV2(client Client) jsonrpc2_v2.Handler {
resErr error
)
replier := func(_ context.Context, res interface{}, err error) error {
result, resErr = res, err
if err != nil {
resErr = err
return nil
}
result = res
return nil
}
_, err := clientDispatch(ctx, client, replier, req1)
Expand Down Expand Up @@ -179,7 +183,11 @@ func ServerHandlerV2(server Server) jsonrpc2_v2.Handler {
resErr error
)
replier := func(_ context.Context, res interface{}, err error) error {
result, resErr = res, err
if err != nil {
resErr = err
return nil
}
result = res
return nil
}
_, err := serverDispatch(ctx, server, replier, req1)
Expand Down

0 comments on commit a220087

Please sign in to comment.