mirror of
https://github.com/neovim/neovim.git
synced 2026-06-18 17:51:18 +00:00
fix(lsp): handle null id in JSON-RPC responses
Problem: LSP spec allows response message to have a null request-id. This may happen when for example client sends unparseable request. https://github.com/microsoft/language-server-protocol/issues/196 Solution: Guard the server response branches against id=vim.NIL (json null), and handle error responses with null id by logging a warning and dispatching on error. Problem: CI (ubuntu asan, ubuntu tsan, windows) reports `uv_loop_close() hang?` from the two new null-id response tests. The leaked handle is the server-side accepted TCP socket created inside `server:listen` callback. The tests closed only the listener but not the accepted socket, so libuv could not finish shutting down the loop and each test session took ~2s extra to exit. Solution: Hoist the accepted socket to the outer `exec_lua` scope and close it at teardown before closing the listener. The close runs synchronously inside `exec_lua`, so the loop has time to dispose the handle before the session exits. * test(lsp): close accepted socket on read-loop exit/error Match the precedent in the handler test ("handler can return false as response") and the shared `_create_tcp_server` helper in `test/functional/plugin/lsp/testutil.lua`: close the accepted socket from inside the `create_read_loop` exit/error callbacks. The teardown close added in the previous commit remains as belt-and-suspenders, so the socket is disposed whether the server goes away first or the client does.
This commit is contained in:
@@ -380,7 +380,9 @@ function Client:handle_body(body)
|
||||
|
||||
if type(decoded) ~= 'table' then
|
||||
self:on_error(M.client_errors.INVALID_SERVER_MESSAGE, decoded)
|
||||
elseif type(decoded.method) == 'string' and decoded.id then
|
||||
elseif type(decoded.method) == 'string' and decoded.id and decoded.id ~= vim.NIL then
|
||||
local id_type = type(decoded.id)
|
||||
assert(id_type == 'number' or id_type == 'string', 'Request id must be a number or a string')
|
||||
local err --- @type lsp.ResponseError?
|
||||
-- Schedule here so that the users functions don't trigger an error and
|
||||
-- we can still use the result.
|
||||
@@ -426,6 +428,7 @@ function Client:handle_body(body)
|
||||
-- - If 'result' is nil, then 'error' must be present (and not vim.NIL).
|
||||
elseif
|
||||
decoded.id
|
||||
and decoded.id ~= vim.NIL
|
||||
and (
|
||||
(decoded.error == nil and decoded.result ~= nil)
|
||||
or (decoded.result == nil and decoded.error ~= nil and decoded.error ~= vim.NIL)
|
||||
@@ -477,6 +480,9 @@ function Client:handle_body(body)
|
||||
self:on_error(M.client_errors.NO_RESULT_CALLBACK_FOUND, decoded)
|
||||
log.error('No callback found for server response id ' .. result_id)
|
||||
end
|
||||
elseif decoded.id == vim.NIL then
|
||||
log.warn('Server sent response with null id', decoded.error)
|
||||
self:on_error(M.client_errors.INVALID_SERVER_MESSAGE, decoded)
|
||||
elseif type(decoded.method) == 'string' then
|
||||
-- Notification
|
||||
self:try_call(
|
||||
|
||||
Reference in New Issue
Block a user