fix(lsp): handle null id in JSON-RPC responses #38340

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:
atusy
2026-04-24 05:41:59 +09:00
committed by GitHub
parent 0a8218a2b4
commit 46b6859a4f
2 changed files with 161 additions and 1 deletions

View File

@@ -453,7 +453,9 @@ function Client:handle_body(body)
log.debug('rpc.receive', decoded)
-- Received a request.
if type(decoded.method) == 'string' and decoded.id then
if 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')
-- Schedule here so that the users functions don't trigger an error and
-- we can still use the result.
vim.schedule(coroutine.wrap(function()
@@ -501,6 +503,7 @@ function Client:handle_body(body)
-- * If 'error' is nil, then 'result' must be present.
-- * If 'result' is nil, then 'error' must be present (and not vim.NIL).
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)
@@ -549,6 +552,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
-- Received a notification.
self:try_call(

View File

@@ -3040,6 +3040,160 @@ describe('LSP', function()
}
eq(expected, result)
end)
it('does not crash on error response with null id (JSON-RPC 2.0 parse error)', function()
local result = exec_lua(function()
local server = assert(vim.uv.new_tcp())
local accepted
local messages = {}
server:bind('127.0.0.1', 0)
server:listen(127, function(err)
assert(not err, err)
accepted = assert(vim.uv.new_tcp())
server:accept(accepted)
accepted:read_start(require('vim.lsp.rpc').create_read_loop(function(body)
local payload = vim.json.decode(body)
if payload.method then
table.insert(messages, payload.method)
if payload.method == 'initialize' then
-- Send a valid initialize response first
local msg = vim.json.encode({
id = payload.id,
jsonrpc = '2.0',
result = {
capabilities = {},
},
})
accepted:write(
table.concat({ 'Content-Length: ', tostring(#msg), '\r\n\r\n', msg })
)
elseif payload.method == 'initialized' then
-- Then send an error response with null id (parse error per JSON-RPC 2.0 §5)
local msg =
'{"jsonrpc":"2.0","error":{"code":-32700,"message":"Parse error"},"id":null}'
accepted:write(
table.concat({ 'Content-Length: ', tostring(#msg), '\r\n\r\n', msg })
)
end
end
end, function()
if accepted and not accepted:is_closing() then
accepted:close()
end
end, function()
if accepted and not accepted:is_closing() then
accepted:close()
end
end))
end)
local port = server:getsockname().port
local on_error_called = false
local client_id = assert(vim.lsp.start({
name = 'null-id-test',
cmd = vim.lsp.rpc.connect('127.0.0.1', port),
on_error = function(_code, _err)
on_error_called = true
end,
}))
vim.lsp.get_client_by_id(client_id)
-- Wait for the initialized notification to be sent and the null-id error to be received
vim.wait(1000, function()
return #messages >= 2 and on_error_called
end)
if accepted and not accepted:is_closing() then
accepted:close()
end
server:close()
server:shutdown()
return {
messages = messages,
on_error_called = on_error_called,
}
end)
-- The key assertion: Neovim should not crash, and the error handler should be called
eq(true, result.on_error_called)
eq(true, #result.messages >= 2)
end)
it('does not misclassify server request with null id as notification', function()
local result = exec_lua(function()
local server = assert(vim.uv.new_tcp())
local accepted
local messages = {}
server:bind('127.0.0.1', 0)
server:listen(127, function(err)
assert(not err, err)
accepted = assert(vim.uv.new_tcp())
server:accept(accepted)
accepted:read_start(require('vim.lsp.rpc').create_read_loop(function(body)
local payload = vim.json.decode(body)
if payload.method then
table.insert(messages, payload.method)
if payload.method == 'initialize' then
local msg = vim.json.encode({
id = payload.id,
jsonrpc = '2.0',
result = {
capabilities = {},
},
})
accepted:write(
table.concat({ 'Content-Length: ', tostring(#msg), '\r\n\r\n', msg })
)
elseif payload.method == 'initialized' then
-- Send a server request with null id (invalid per JSON-RPC 2.0)
local msg =
'{"jsonrpc":"2.0","method":"workspace/configuration","params":{"items":[]},"id":null}'
accepted:write(
table.concat({ 'Content-Length: ', tostring(#msg), '\r\n\r\n', msg })
)
end
end
end, function()
if accepted and not accepted:is_closing() then
accepted:close()
end
end, function()
if accepted and not accepted:is_closing() then
accepted:close()
end
end))
end)
local port = server:getsockname().port
local on_error_called = false
local notification_received = false
local client_id = assert(vim.lsp.start({
name = 'null-id-request-test',
cmd = vim.lsp.rpc.connect('127.0.0.1', port),
on_error = function(_code, _err)
on_error_called = true
end,
handlers = {
['workspace/configuration'] = function()
notification_received = true
return {}
end,
},
}))
vim.lsp.get_client_by_id(client_id)
vim.wait(1000, function()
return #messages >= 2 and (on_error_called or notification_received)
end)
if accepted and not accepted:is_closing() then
accepted:close()
end
server:close()
server:shutdown()
return {
messages = messages,
on_error_called = on_error_called,
notification_received = notification_received,
}
end)
-- Should be dispatched as an error, NOT silently handled as a notification
eq(true, result.on_error_called)
eq(false, result.notification_received)
end)
end)
describe('#dynamic vim.lsp._dynamic', function()