mirror of
https://github.com/neovim/neovim.git
synced 2026-07-16 06:10:39 +00:00
fix(lsp): handle requests with null id #40073
Problem: PR #38340 prevented messages we receive with id:null from being incorrectly classified as notifications, but caused us to ignore all messages with id:null, including requests. Solution: Handle requests with id:null. When we receive a request, we only need to respond based on the `method` and `param`. (The original so-called `notification_received` in the test was actually semantically `request_or_notification_received`.)
This commit is contained in:
@@ -461,9 +461,11 @@ function Client:handle_body(body)
|
||||
|
||||
log.debug('rpc.receive', decoded)
|
||||
|
||||
-- Received a request.
|
||||
if type(decoded.method) == 'string' and decoded.id and decoded.id ~= vim.NIL then
|
||||
if type(decoded.id) ~= 'number' and type(decoded.id) ~= 'string' then
|
||||
if
|
||||
-- Received a request.
|
||||
type(decoded.method) == 'string' and decoded.id
|
||||
then
|
||||
if type(decoded.id) ~= 'number' and type(decoded.id) ~= 'string' and decoded.id ~= vim.NIL then
|
||||
log.error(
|
||||
'Server request id must be a number or string, got ' .. type(decoded.id),
|
||||
decoded.method,
|
||||
@@ -515,17 +517,25 @@ function Client:handle_body(body)
|
||||
end))
|
||||
elseif
|
||||
-- Received a response to a request we sent.
|
||||
decoded.id
|
||||
then
|
||||
-- If there was an error in detecting the id in the Request object
|
||||
-- (e.g. Parse error/Invalid Request), it must be Null.
|
||||
if decoded.id == vim.NIL then
|
||||
log.warn('Server sent response with null id', decoded)
|
||||
self:on_error(M.client_errors.INVALID_SERVER_MESSAGE, decoded)
|
||||
return
|
||||
end
|
||||
-- Proceed only if exactly one of 'result' or 'error' is present,
|
||||
-- as required by the JSON-RPC spec:
|
||||
-- * 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)
|
||||
)
|
||||
then
|
||||
if (decoded.error == nil or decoded.error == vim.NIL) and decoded.result == nil then
|
||||
log.error('Server respond empty result and error', decoded)
|
||||
self:on_error(M.client_errors.INVALID_SERVER_MESSAGE, decoded)
|
||||
return
|
||||
end
|
||||
|
||||
-- We sent a number, so we expect a number.
|
||||
local result_id = vim._assert_integer(decoded.id)
|
||||
|
||||
@@ -569,11 +579,10 @@ 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.method, decoded.error)
|
||||
self:on_error(M.client_errors.INVALID_SERVER_MESSAGE, decoded)
|
||||
elseif type(decoded.method) == 'string' then
|
||||
elseif
|
||||
-- Received a notification.
|
||||
type(decoded.method) == 'string'
|
||||
then
|
||||
self:try_call(
|
||||
M.client_errors.NOTIFICATION_HANDLER_ERROR,
|
||||
self.dispatchers.notification,
|
||||
|
||||
@@ -3048,13 +3048,11 @@ describe('LSP', function()
|
||||
end)
|
||||
|
||||
--- 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.
|
||||
--- "initialized" 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)
|
||||
local function test_null_id_response(null_id_payload)
|
||||
return exec_lua(function()
|
||||
local server = assert(vim.uv.new_tcp())
|
||||
local accepted
|
||||
@@ -3102,22 +3100,21 @@ 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-test',
|
||||
cmd = vim.lsp.rpc.connect('127.0.0.1', port),
|
||||
cmd = function(dispatchers)
|
||||
local notification = dispatchers.notification
|
||||
dispatchers.notification = function(method, params)
|
||||
notification_received = true
|
||||
if notification then
|
||||
notification(method, params)
|
||||
end
|
||||
end
|
||||
return vim.lsp.rpc.connect('127.0.0.1', port)(dispatchers)
|
||||
end,
|
||||
on_error = function(_code, _err)
|
||||
on_error_called = true
|
||||
end,
|
||||
handlers = handlers,
|
||||
}))
|
||||
vim.lsp.get_client_by_id(client_id)
|
||||
vim.wait(1000, function()
|
||||
@@ -3147,19 +3144,15 @@ describe('LSP', function()
|
||||
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'
|
||||
'{"jsonrpc":"2.0","method":"workspace/configuration","params":{"items":[]}}'
|
||||
)
|
||||
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'
|
||||
'{"jsonrpc":"2.0","method":"workspace/configuration","params":{"items":[]},"id":null}'
|
||||
)
|
||||
-- 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.
|
||||
-- Null id must be dispatched as a request, not as a notification.
|
||||
eq(false, result.notification_received)
|
||||
end)
|
||||
end)
|
||||
|
||||
Reference in New Issue
Block a user