From cf9ad39267d9dc5f15ff0e4c6eda236b2c7e39fe Mon Sep 17 00:00:00 2001 From: Yi Ming Date: Wed, 3 Jun 2026 05:39:56 +0800 Subject: [PATCH] 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`.) --- runtime/lua/vim/lsp/rpc.lua | 37 ++++++++++++++++++----------- test/functional/plugin/lsp_spec.lua | 37 ++++++++++++----------------- 2 files changed, 38 insertions(+), 36 deletions(-) diff --git a/runtime/lua/vim/lsp/rpc.lua b/runtime/lua/vim/lsp/rpc.lua index f76793c286..5888afcefd 100644 --- a/runtime/lua/vim/lsp/rpc.lua +++ b/runtime/lua/vim/lsp/rpc.lua @@ -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, diff --git a/test/functional/plugin/lsp_spec.lua b/test/functional/plugin/lsp_spec.lua index ed120cb69e..db6f1e1481 100644 --- a/test/functional/plugin/lsp_spec.lua +++ b/test/functional/plugin/lsp_spec.lua @@ -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)