From 2c7679f4d380deb05785476e925147bed032a1a9 Mon Sep 17 00:00:00 2001 From: "Justin M. Keyes" Date: Fri, 24 Apr 2026 07:40:33 -0400 Subject: [PATCH] fix(lsp): more info in error msg, deduplicate test #39359 --- runtime/lua/vim/lsp/rpc.lua | 14 ++- test/functional/plugin/lsp_spec.lua | 142 ++++++++++------------------ 2 files changed, 63 insertions(+), 93 deletions(-) diff --git a/runtime/lua/vim/lsp/rpc.lua b/runtime/lua/vim/lsp/rpc.lua index 234f895f2b..7d85be89c9 100644 --- a/runtime/lua/vim/lsp/rpc.lua +++ b/runtime/lua/vim/lsp/rpc.lua @@ -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. diff --git a/test/functional/plugin/lsp_spec.lua b/test/functional/plugin/lsp_spec.lua index cd6629b378..adb679e393 100644 --- a/test/functional/plugin/lsp_spec.lua +++ b/test/functional/plugin/lsp_spec.lua @@ -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)