mirror of
https://github.com/neovim/neovim.git
synced 2026-05-24 05:40:08 +00:00
fix(lsp): more info in error msg, deduplicate test #39359
This commit is contained in:
@@ -454,8 +454,16 @@ function Client:handle_body(body)
|
||||
|
||||
-- Received a request.
|
||||
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')
|
||||
if type(decoded.id) ~= 'number' and type(decoded.id) ~= 'string' then
|
||||
log.error(
|
||||
'Server request id must be a number or string, got ' .. type(decoded.id),
|
||||
decoded.method,
|
||||
decoded.id
|
||||
)
|
||||
self:on_error(M.client_errors.INVALID_SERVER_MESSAGE, decoded)
|
||||
return
|
||||
end
|
||||
|
||||
-- Schedule here so that the users functions don't trigger an error and
|
||||
-- we can still use the result.
|
||||
vim.schedule(coroutine.wrap(function()
|
||||
@@ -553,7 +561,7 @@ function Client:handle_body(body)
|
||||
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)
|
||||
log.warn('Server sent response with null id', decoded.method, decoded.error)
|
||||
self:on_error(M.client_errors.INVALID_SERVER_MESSAGE, decoded)
|
||||
elseif type(decoded.method) == 'string' then
|
||||
-- Received a notification.
|
||||
|
||||
@@ -3041,8 +3041,15 @@ 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()
|
||||
--- Starts a TCP server that completes initialization, then sends `null_id_payload` after the
|
||||
--- "initialized" notification. If `notification_method` is given, registers a handler
|
||||
--- that tracks whether it was dispatched as a notification.
|
||||
---
|
||||
--- @param null_id_payload string JSON
|
||||
--- @param notification_method? string
|
||||
--- @return { on_error_called: table, notification_received: boolean, messages: boolean }.
|
||||
local function test_null_id_response(null_id_payload, notification_method)
|
||||
return exec_lua(function()
|
||||
local server = assert(vim.uv.new_tcp())
|
||||
local accepted
|
||||
local messages = {}
|
||||
@@ -3068,85 +3075,12 @@ describe('LSP', function()
|
||||
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 })
|
||||
)
|
||||
accepted:write(table.concat({
|
||||
'Content-Length: ',
|
||||
tostring(#null_id_payload),
|
||||
'\r\n\r\n',
|
||||
null_id_payload,
|
||||
}))
|
||||
end
|
||||
end
|
||||
end, function()
|
||||
@@ -3162,18 +3096,22 @@ describe('LSP', function()
|
||||
local port = server:getsockname().port
|
||||
local on_error_called = false
|
||||
local notification_received = false
|
||||
local handlers = nil
|
||||
if notification_method then
|
||||
handlers = {
|
||||
[notification_method] = function()
|
||||
notification_received = true
|
||||
return {}
|
||||
end,
|
||||
}
|
||||
end
|
||||
local client_id = assert(vim.lsp.start({
|
||||
name = 'null-id-request-test',
|
||||
name = 'null-id-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,
|
||||
},
|
||||
handlers = handlers,
|
||||
}))
|
||||
vim.lsp.get_client_by_id(client_id)
|
||||
vim.wait(1000, function()
|
||||
@@ -3182,16 +3120,40 @@ describe('LSP', function()
|
||||
if accepted and not accepted:is_closing() then
|
||||
accepted:close()
|
||||
end
|
||||
server:close()
|
||||
server:shutdown()
|
||||
server:close()
|
||||
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
|
||||
end
|
||||
|
||||
it('null-id in response (JSON-RPC 2.0 parse error) is handled, emits error', function()
|
||||
local result = test_null_id_response(
|
||||
'{"jsonrpc":"2.0","error":{"code":-32700,"message":"Parse error"},"id":null}'
|
||||
)
|
||||
eq(true, result.on_error_called)
|
||||
eq(true, #result.messages >= 2)
|
||||
end)
|
||||
|
||||
it('null-id in response does not misclassify as a notification', function()
|
||||
-- Sanity check: a real notification (no id) dispatches the handler.
|
||||
local valid = test_null_id_response(
|
||||
'{"jsonrpc":"2.0","method":"workspace/configuration","params":{"items":[]}}',
|
||||
'workspace/configuration'
|
||||
)
|
||||
eq(true, valid.notification_received)
|
||||
|
||||
local result = test_null_id_response(
|
||||
-- Error response with null id (parse error per JSON-RPC 2.0 §5)
|
||||
'{"jsonrpc":"2.0","method":"workspace/configuration","params":{"items":[]},"id":null}',
|
||||
'workspace/configuration'
|
||||
)
|
||||
-- Should be dispatched as an error, NOT silently handled as a notification.
|
||||
eq(true, result.on_error_called)
|
||||
-- Null id must NOT be dispatched as a notification.
|
||||
eq(false, result.notification_received)
|
||||
end)
|
||||
end)
|
||||
|
||||
Reference in New Issue
Block a user