LSP response processing broken when responses span messages

[muirdm]

Thank you for the bug report

• [X] I am using the latest version of lsp-mode related packages.
• [X] I checked FAQ and Troubleshooting sections
• [ ] You may also try reproduce the issue using clean environment using the following command

  emacs -q -l lsp-start-plain.el

  where lsp-start-plain.el can be downloaded here. Alternatively, it will be great if you can reproduce the issue using lsp-docker which provides the minimal configurations for lsp-mode and ships with most of the language servers.

Bug description

Handling in lsp--create-filter-function is not correct for certain cases where response messages are split across messages. The underlying issue is leftovers is set to nil when a message is parsed and chunk is overwritten as well (so nothing contains the leftovers).

To fix I think we can just get rid of leftovers variable and just not set chunk to nil in the call to lsp--parser-on-message.

Steps to reproduce

I experience this using the hack server over tramp (which probably affects the message size somehow). Here are the two messages that broke the parseing:

Content-Length: 834

{"jsonrpc":"2.0","id":47,"result":{"capabilities":{"textDocumentSync":{"openClose":true,"change":2,"willSave":false,"willSaveWaitUntil":true,"save":{"includeText":false}},"hoverProvider":true,"completionProvider":{"resolveProvider":true,"triggerCharacters":["$",">","\\",":","<","[","'","\"","{","#"]},"signatureHelpProvider":{"triggerCharacters":["(",","]},"definitionProvider":true,"typeDefinitionProvider":true,"referencesProvider":true,"documentHighlightProvider":true,"documentSymbolProvider":true,"workspaceSymbolProvider":true,"codeActionProvider":false,"documentFormattingProvider":true,"documentRangeFormattingProvider":true,"documentOnTypeFormattingProvider":{"firstTriggerCharacter":";","moreTriggerCharacter":["}"]},"renameProvider":true,"implementationProvider":true,"typeCoverageProvider":true,"rageProvider":true}}}

Content-Length: 279

{"jsonrpc":"2.0","id":1,"method":"client/registerCapability","params":{"registrations":[{"id":"did-change-watched-files","method":"workspace/did

ChangeWatchedFiles","registerOptions":{"watchers":[{"globPattern":"**/*.{php,phpt,hack,hackpartial,hck,hh,hhi,xhp}","kind":7}]}}]}}

Content-Length: 118

{"jsonrpc":"2.0","method":"telemetry/event","params":{"type":4,"message":"Version in hhconfig and switch=5.12.0"}}

Expected behavior

Messages are parsed and processed properly.

Which Language Server did you use?

hack

OS

MacOS

Error callstack

No response

Anything else?

No response

[yyoncho]

The proposed fix makes sense. Willing to make a PR? (FTR we have a test suite for io handling, it will be nice if you add test for that scenario).

[muirdm]

I just switched jobs and don't have permission yet.

[yyoncho]

ok, thanks, I will handle it.

[yyoncho]

I tested it out and it seems like parsing function can handle that input: (lsp--create-process-message is defined lsp-io-test.el)

(let* ((fn (lsp--create-process-message)))
  (--map (lsp--read-json it)
         (nconc (funcall fn "Content-Length: 834\r\n\r\n{\"jsonrpc\":\"2.0\",\"id\":47,\"result\":{\"capabilities\":{\"textDocumentSync\":{\"openClose\":true,\"change\":2,\"willSave\":false,\"willSaveWaitUntil\":true,\"save\":{\"includeText\":false}},\"hoverProvider\":true,\"completionProvider\":{\"resolveProvider\":true,\"triggerCharacters\":[\"$\",\">\",\"\\\\\",\":\",\"<\",\"[\",\"'\",\"\\\"\",\"{\",\"#\"]},\"signatureHelpProvider\":{\"triggerCharacters\":[\"(\",\",\"]},\"definitionProvider\":true,\"typeDefinitionProvider\":true,\"referencesProvider\":true,\"documentHighlightProvider\":true,\"documentSymbolProvider\":true,\"workspaceSymbolProvider\":true,\"codeActionProvider\":false,\"documentFormattingProvider\":true,\"documentRangeFormattingProvider\":true,\"documentOnTypeFormattingProvider\":{\"firstTriggerCharacter\":\";\",\"moreTriggerCharacter\":[\"}\"]},\"renameProvider\":true,\"implementationProvider\":true,\"typeCoverageProvider\":true,\"rageProvider\":true}}}\r\n\r\nContent-Length: 279\r\n\r\n{\"jsonrpc\":\"2.0\",\"id\":1,\"method\":\"client/registerCapability\",\"params\":{\"registrations\":[{\"id\":\"did-change-watched-files\",\"method\":\"workspace/did")
                (funcall fn "ChangeWatchedFiles\",\"registerOptions\":{\"watchers\":[{\"globPattern\":\"**/*.{php,phpt,hack,hackpartial,hck,hh,hhi,xhp}\",\"kind\":7}]}}]}}\r\n\r\nContent-Length: 118\r\n\r\n{\"jsonrpc\":\"2.0\",\"method\":\"telemetry/event\",\"params\":{\"type\":4,\"message\":\"Version in hhconfig and switch=5.12.0\"}}\r\n\r\n"))))

[muirdm]

Yes, I still don't quite seem to know what is wrong. Let me look some more.

[yyoncho]

There are issues and pr about tramp and line endings, maybe your issue is related

[muirdm]

The filter function seems to be called re-entrantly in my real scenario. It looks like it gets called recursively via lsp--parser-on-message somehow, but I'm not sure. The recursive call appears to come from the textDocument/didOpen for the file I am looking at.

[yyoncho]

@muirdm that makes sense. watches usually do a lot of work with might allow another cycle of reading messages. I will change the function to allow reentrance and ask you to test.

[yyoncho]

PR at #2941

[muirdm]

Seems to work, thanks!

Probable unrelated, but I see this sometimes when shutting down the workspace:

Remote file error: Forbidden reentrant call of Tramp
error in process sentinel: tramp-error: Forbidden reentrant call of Tramp
error in process sentinel: Forbidden reentrant call of Tramp

[yyoncho]

Probable unrelated, but I see this sometimes when shutting down the workspace:

there is a separate issue for sending being not reentrant