diff --git a/internal/jsonrpc2/handler.go b/internal/jsonrpc2/handler.go index d50419765e..ad7fb9c4a7 100644 --- a/internal/jsonrpc2/handler.go +++ b/internal/jsonrpc2/handler.go @@ -6,6 +6,7 @@ package jsonrpc2 import ( "context" + "fmt" ) // Handler is invoked to handle incoming requests. @@ -65,8 +66,17 @@ func (d Direction) String() string { // standard method not found response. // This should normally be the final handler in a chain. func MethodNotFound(ctx context.Context, r *Request) error { - if !r.IsNotify() { - return r.Reply(ctx, nil, NewErrorf(CodeMethodNotFound, "method %q not found", r.Method)) - } - return nil + return r.Reply(ctx, nil, NewErrorf(CodeMethodNotFound, "method %q not found", r.Method)) +} + +// MustReply creates a Handler that panics if the wrapped handler does +// not call Reply for every request that it is passed. +func MustReply(handler Handler) Handler { + return func(ctx context.Context, req *Request) error { + err := handler(ctx, req) + if req.state < requestReplied { + panic(fmt.Errorf("request %q was never replied to", req.Method)) + } + return err + } } diff --git a/internal/jsonrpc2/jsonrpc2.go b/internal/jsonrpc2/jsonrpc2.go index 245ce030c1..441fe3f46c 100644 --- a/internal/jsonrpc2/jsonrpc2.go +++ b/internal/jsonrpc2/jsonrpc2.go @@ -296,6 +296,7 @@ type combined struct { // It must be called exactly once for each Conn. // It returns only when the reader is closed or there is an error in the stream. func (c *Conn) Run(runCtx context.Context, handler Handler) error { + handler = MustReply(handler) // we need to make the next request "lock" in an unlocked state to allow // the first incoming request to proceed. All later requests are unlocked // by the preceding request going to parallel mode. @@ -353,9 +354,6 @@ func (c *Conn) Run(runCtx context.Context, handler Handler) error { req.state = requestSerial defer func() { setHandling(req, false) - if req.state < requestReplied { - req.Reply(reqCtx, nil, nil) - } done() cancelReq() }() diff --git a/internal/lsp/lsprpc/lsprpc.go b/internal/lsp/lsprpc/lsprpc.go index 16a1300ea7..e36684e5bd 100644 --- a/internal/lsp/lsprpc/lsprpc.go +++ b/internal/lsp/lsprpc/lsprpc.go @@ -418,10 +418,10 @@ func forwarderHandler(handler jsonrpc2.Handler) jsonrpc2.Handler { // once that process exists. if r.Method == "exit" { ForwarderExitFunc(0) - // return nil here to consume the message: in + // reply nil here to consume the message: in // tests, ForwarderExitFunc may be overridden to something that doesn't // exit the process. - return nil + return r.Reply(ctx, nil, nil) } return handler(ctx, r) } @@ -505,10 +505,7 @@ func handshaker(client *debugClient, goplsPath string, handler jsonrpc2.Handler) resp.DebugAddr = di.ListenedDebugAddress } - if err := r.Reply(ctx, resp, nil); err != nil { - event.Error(ctx, "replying to handshake", err) - } - return nil + return r.Reply(ctx, resp, nil) case sessionsMethod: resp := ServerState{ GoplsPath: goplsPath, @@ -526,10 +523,7 @@ func handshaker(client *debugClient, goplsPath string, handler jsonrpc2.Handler) }) } } - if err := r.Reply(ctx, resp, nil); err != nil { - event.Error(ctx, "replying to sessions request", err) - } - return nil + return r.Reply(ctx, resp, nil) } return handler(ctx, r) } diff --git a/internal/lsp/protocol/protocol.go b/internal/lsp/protocol/protocol.go index 135d946b35..8f2208bbdb 100644 --- a/internal/lsp/protocol/protocol.go +++ b/internal/lsp/protocol/protocol.go @@ -69,7 +69,11 @@ func (Canceller) Cancel(ctx context.Context, conn *jsonrpc2.Conn, id jsonrpc2.ID func (Canceller) Deliver(ctx context.Context, r *jsonrpc2.Request, delivered bool) bool { // Hide cancellations from downstream handlers. - return r.Method == "$/cancelRequest" + if r.Method != "$/cancelRequest" { + return false + } + r.Reply(ctx, nil, nil) + return true } func sendParseError(ctx context.Context, req *jsonrpc2.Request, err error) error { diff --git a/internal/lsp/protocol/tsclient.go b/internal/lsp/protocol/tsclient.go index 9be54a3919..2cf5af0474 100644 --- a/internal/lsp/protocol/tsclient.go +++ b/internal/lsp/protocol/tsclient.go @@ -42,31 +42,36 @@ func ClientHandler(client Client, handler jsonrpc2.Handler) jsonrpc2.Handler { if err := json.Unmarshal(*r.Params, ¶ms); err != nil { return sendParseError(ctx, r, err) } - return client.ShowMessage(ctx, ¶ms) + err := client.ShowMessage(ctx, ¶ms) + return r.Reply(ctx, nil, err) case "window/logMessage": // notif var params LogMessageParams if err := json.Unmarshal(*r.Params, ¶ms); err != nil { return sendParseError(ctx, r, err) } - return client.LogMessage(ctx, ¶ms) + err := client.LogMessage(ctx, ¶ms) + return r.Reply(ctx, nil, err) case "telemetry/event": // notif var params interface{} if err := json.Unmarshal(*r.Params, ¶ms); err != nil { return sendParseError(ctx, r, err) } - return client.Event(ctx, ¶ms) + err := client.Event(ctx, ¶ms) + return r.Reply(ctx, nil, err) case "textDocument/publishDiagnostics": // notif var params PublishDiagnosticsParams if err := json.Unmarshal(*r.Params, ¶ms); err != nil { return sendParseError(ctx, r, err) } - return client.PublishDiagnostics(ctx, ¶ms) + err := client.PublishDiagnostics(ctx, ¶ms) + return r.Reply(ctx, nil, err) case "$/progress": // notif var params ProgressParams if err := json.Unmarshal(*r.Params, ¶ms); err != nil { return sendParseError(ctx, r, err) } - return client.Progress(ctx, ¶ms) + err := client.Progress(ctx, ¶ms) + return r.Reply(ctx, nil, err) case "workspace/workspaceFolders": // req if r.Params != nil { return r.Reply(ctx, nil, jsonrpc2.NewErrorf(jsonrpc2.CodeInvalidParams, "Expected no params")) diff --git a/internal/lsp/protocol/tsserver.go b/internal/lsp/protocol/tsserver.go index acde0cfe5a..fc675d897c 100644 --- a/internal/lsp/protocol/tsserver.go +++ b/internal/lsp/protocol/tsserver.go @@ -80,19 +80,22 @@ func ServerHandler(server Server, handler jsonrpc2.Handler) jsonrpc2.Handler { if err := json.Unmarshal(*r.Params, ¶ms); err != nil { return sendParseError(ctx, r, err) } - return server.DidChangeWorkspaceFolders(ctx, ¶ms) + err := server.DidChangeWorkspaceFolders(ctx, ¶ms) + return r.Reply(ctx, nil, err) case "window/workDoneProgress/cancel": // notif var params WorkDoneProgressCancelParams if err := json.Unmarshal(*r.Params, ¶ms); err != nil { return sendParseError(ctx, r, err) } - return server.WorkDoneProgressCancel(ctx, ¶ms) + err := server.WorkDoneProgressCancel(ctx, ¶ms) + return r.Reply(ctx, nil, err) case "initialized": // notif var params InitializedParams if err := json.Unmarshal(*r.Params, ¶ms); err != nil { return sendParseError(ctx, r, err) } - return server.Initialized(ctx, ¶ms) + err := server.Initialized(ctx, ¶ms) + return r.Reply(ctx, nil, err) case "exit": // notif return server.Exit(ctx) case "workspace/didChangeConfiguration": // notif @@ -100,55 +103,64 @@ func ServerHandler(server Server, handler jsonrpc2.Handler) jsonrpc2.Handler { if err := json.Unmarshal(*r.Params, ¶ms); err != nil { return sendParseError(ctx, r, err) } - return server.DidChangeConfiguration(ctx, ¶ms) + err := server.DidChangeConfiguration(ctx, ¶ms) + return r.Reply(ctx, nil, err) case "textDocument/didOpen": // notif var params DidOpenTextDocumentParams if err := json.Unmarshal(*r.Params, ¶ms); err != nil { return sendParseError(ctx, r, err) } - return server.DidOpen(ctx, ¶ms) + err := server.DidOpen(ctx, ¶ms) + return r.Reply(ctx, nil, err) case "textDocument/didChange": // notif var params DidChangeTextDocumentParams if err := json.Unmarshal(*r.Params, ¶ms); err != nil { return sendParseError(ctx, r, err) } - return server.DidChange(ctx, ¶ms) + err := server.DidChange(ctx, ¶ms) + return r.Reply(ctx, nil, err) case "textDocument/didClose": // notif var params DidCloseTextDocumentParams if err := json.Unmarshal(*r.Params, ¶ms); err != nil { return sendParseError(ctx, r, err) } - return server.DidClose(ctx, ¶ms) + err := server.DidClose(ctx, ¶ms) + return r.Reply(ctx, nil, err) case "textDocument/didSave": // notif var params DidSaveTextDocumentParams if err := json.Unmarshal(*r.Params, ¶ms); err != nil { return sendParseError(ctx, r, err) } - return server.DidSave(ctx, ¶ms) + err := server.DidSave(ctx, ¶ms) + return r.Reply(ctx, nil, err) case "textDocument/willSave": // notif var params WillSaveTextDocumentParams if err := json.Unmarshal(*r.Params, ¶ms); err != nil { return sendParseError(ctx, r, err) } - return server.WillSave(ctx, ¶ms) + err := server.WillSave(ctx, ¶ms) + return r.Reply(ctx, nil, err) case "workspace/didChangeWatchedFiles": // notif var params DidChangeWatchedFilesParams if err := json.Unmarshal(*r.Params, ¶ms); err != nil { return sendParseError(ctx, r, err) } - return server.DidChangeWatchedFiles(ctx, ¶ms) + err := server.DidChangeWatchedFiles(ctx, ¶ms) + return r.Reply(ctx, nil, err) case "$/setTraceNotification": // notif var params SetTraceParams if err := json.Unmarshal(*r.Params, ¶ms); err != nil { return sendParseError(ctx, r, err) } - return server.SetTraceNotification(ctx, ¶ms) + err := server.SetTraceNotification(ctx, ¶ms) + return r.Reply(ctx, nil, err) case "$/logTraceNotification": // notif var params LogTraceParams if err := json.Unmarshal(*r.Params, ¶ms); err != nil { return sendParseError(ctx, r, err) } - return server.LogTraceNotification(ctx, ¶ms) + err := server.LogTraceNotification(ctx, ¶ms) + return r.Reply(ctx, nil, err) case "textDocument/implementation": // req var params ImplementationParams if err := json.Unmarshal(*r.Params, ¶ms); err != nil { diff --git a/internal/lsp/protocol/typescript/code.ts b/internal/lsp/protocol/typescript/code.ts index 708c3846a1..a006acb4c8 100644 --- a/internal/lsp/protocol/typescript/code.ts +++ b/internal/lsp/protocol/typescript/code.ts @@ -926,7 +926,8 @@ function goNot(side: side, m: string) { if err := json.Unmarshal(*r.Params, ¶ms); err != nil { return sendParseError(ctx, r, err) } - return ${side.name}.${nm}(ctx, ¶ms)` + err:= ${side.name}.${nm}(ctx, ¶ms) + return r.Reply(ctx, nil, err)` } else { case1 = `return ${side.name}.${nm}(ctx)`; }