Feel-ix-343 / markdown-oxide

Robust, Minimalist, Unbundled PKM for your favorite text-editor through the LSP
https://oxide.md
GNU General Public License v3.0
899 stars 14 forks source link

LSP[markdown_oxide]: Error INVALID_SERVER_MESSAGE: -32700 Parse error #156

Closed powerman closed 5 days ago

powerman commented 1 month ago

Often when I paste large chunk of text into current .md file I got this:

LSP[markdown_oxide]: Error INVALID_SERVER_MESSAGE: {                                        
  error = {                                                                                         
    code = -32700,                                                                                  
    message = "Parse error"                                                                         
  },                                                                                                
  jsonrpc = "2.0"                                                                                   
}

This happens only with markdown-oxide (I've also efm LSP connected to run markdownlint), so I don't really believe Neovim actually sends an invalid JSON request. All other kinds of "markdown parse errors" shouldn't use that code and shouldn't crash the server.

Another issue is LSP doesn't restart automatically after that error, so I need to manually do :LspStart markdown_oxide - but this is probably a general Neovim issue.

Let me know if you need LSP logs with request/response traces, I'll try to reproduce this issue with enabled logging.

Feel-ix-343 commented 1 month ago

Thank you for reporting! Sorry you are experiencing this.

More LSP logs may help if you don't mind. Possibly relevant neovim configuration too.

powerman commented 1 month ago

It turns out to be racy! I repeat the same simple repro sequence a lot of times:

  1. vi test.md (non-existing file)
  2. A<Enter><Enter><Esc>p (I don't think A<Enter><Enter><Esc> part is needed, but as it's racy I repeat the one I know for sure may trigger the issue and don't waste more time experimenting here).

and it triggers the issue about 1/10 times.

I've temporary switched off markdownlint to avoid unrelated messages in a log. markdown-oxide.lsp1.log

powerman commented 1 month ago

Mason says I'm using markdown-oxide v0.23.1.

Feel-ix-343 commented 1 month ago

hmmmm thank you

I wonder if it's related to the non-ascii characters

Feel-ix-343 commented 1 month ago

Also, this LS does not specify any "Parse error", so I wonder of its related to the LSP library I'm using: tower-lsp

powerman commented 1 month ago

I wonder if it's related to the non-ascii characters

Me too. Tried to repro using english, but as it's racy I give up after some fair amount of attempts.

Feel-ix-343 commented 1 month ago

Which LSP client are you using?

Feel-ix-343 commented 1 month ago

I'm finding that this is an error in parsing the JSON of the RPC

Feel-ix-343 commented 1 month ago

"the message from the server cannot be parsed"

powerman commented 1 month ago

I'm finding that this is an error in parsing the JSON of the RPC

Yeah. At least it shouldn't be used for other reasons: https://www.jsonrpc.org/specification#error_object

Which LSP client are you using?

Just Neovim (I'm using neovim/nvim-lspconfig).

Feel-ix-343 commented 1 month ago

Hm. So no nvim-cmp?

powerman commented 1 month ago

I've nvim-cmp, but it has nothing with this. It doesn't affect existing LSP requests and I didn't used completion keymap so it shouldn't send it's own LSP requests too.

powerman commented 1 month ago

Anyway, it shouldn't and doesn't matter which plugins I've in Neovim. JSON-RPC 2 is a network protocol, and you see all network messages in a log, so if sent from Neovim messages are valid then it doesn't matter who sends them.

Feel-ix-343 commented 1 month ago

This is related: https://github.com/ebkalderon/tower-lsp/issues/267 (but of course out of date)

Feel-ix-343 commented 1 month ago

I don't believe I am able to reproduce this.

Could you send a neovim configuration and version information with your issue?

powerman commented 1 month ago

Still trying to produce a minimal repro.lua, no luck so far. But I noticed something interesting! This is request which result in an error:

[DEBUG][2024-07-26 20:35:52] .../vim/lsp/rpc.lua:286    "rpc.send"  {
  jsonrpc = "2.0",
  method = "textDocument/didChange",
  params = {
    contentChanges = { {
        text = "\n\n  - п╢п╬п╠п╟п╡п╦я┌я▄ PostgreSQL\n  - п╢п╬п╠п╟п╡п╦я┌я▄ HSTS, CSP п╦ п©я─п╬я┤п╦п╣ /etc/nginx/conf.d/headers.conf\n  - п╢п╬п╠п╟п╡п╦я┌я▄ LPC я┤п╣я─п╣п╥ gRPC\n    - https://github.com/fullstorydev/grpchan\n      - https://github.com/apssouza22/grpc-production-go я┌я┐я┌ п╣я│я┌я▄ п©я─п╦п╪п╣я─\n    - п╨п╟п╨ п╢п╣п╩п╟я┌я▄ discovery/mock?\n  - п╢п╬п©п╦я│п╟я┌я▄ ms/example:\n    - п╠п╬п╩я▄я┬п╣ API (п╦ я│п╪я▀я│п╩п╟)\n    - п╠п╬п╩я▄я┬п╣ я┌п╣я│я┌п╬п╡\n    - п╟п╢п╟п©я┌п╣я─ п╨ ms/auth\n  - п╢п╬п╠п╟п╡п╦я┌я▄ п╡п╟я─п╦п╟п╫я┌ я│ legacy-п╨п╬п╢п╬п╪ п╡ old/ я┐ п╬п╢п╫п╬пЁп╬ п╦п╥ example?\n  - п©п╬п©я─п╬п╠п╬п╡п╟я┌я▄ consul п╡п╪п╣я│я┌п╬ env.sh\n    - п╠п╬п╩я▄я┬п╦п╫я│я┌п╡п╬ п╫п╟я│я┌я─п╬п╣п╨ п╦п╪п╣я▌я┌ defaults\n    - п©я─п╦ п╥п╟п©я┐я│п╨п╣ п╬я┌я│я┐я┌я│я┌п╡я┐я▌я┴п╦п╣ п╫п╟я│я┌я─п╬п╧п╨п╦ я│п╬п╥п╢п╟я▌я┌я│я▐ я│ default-п╥п╫п╟я┤п╣п╫п╦я▐п╪п╦\n    - п╨п╟п╨ я─п╣я┬п╦я┌я▄ host-specific п╫п╟я│я┌я─п╬п╧п╨п╦?\n    - п╨п╟п╨ п╦п╥п╠п╟п╡п╩я▐я┌я▄я│я▐ п╬я┌ п╫п╣п╦я│п©п╬п╩я▄п╥я┐п╣п╪я▀я┘ п╫п╟я│я┌я─п╬п╣п╨?\n      - я│ я┐я┤я▒я┌п╬п╪ я┌п╬пЁп╬, я┤я┌п╬ пЁп╢п╣-я┌п╬ п╪п╬п╤п╣я┌ я─п╟п\n"
      } },
    textDocument = {
      uri = "file:///home/powerman/doc/Main/test.md",
      version = 6
    }
  }
}

and this is same request with about same content which does not result in error:

[DEBUG][2024-07-26 23:01:02] .../vim/lsp/rpc.lua:286    "rpc.send"  {
  jsonrpc = "2.0",
  method = "textDocument/didChange",
  params = {
    contentChanges = { {
        text = "\n\n\n  - добавить PostgreSQL\n  - добавить HSTS, CSP и прочие /etc/nginx/conf.d/headers.conf\n  - добавить LPC через gRPC\n    - https://github.com/fullstorydev/grpchan\n      - https://github.com/apssouza22/grpc-production-go тут есть пример\n    - как делать discovery/mock?\n  - дописать ms/example:\n    - больше API (и смысла)\n    - больше тестов\n    - адаптер к ms/auth\n  - добавить вариант с legacy-кодом в old/ у одного из example?\n  - попробовать consul вместо env.sh\n    - большинство настроек имеют defaults\n    - при запуске отсутствующие настройки создаются с default-значениями\n    - как решить host-specific настройки?\n    - как избавляться от неиспользуемых настроек?\n      - с учётом того, что где-то может работать инстанс старой версии,\n        ещё использующий эти настройки? или он может в этот момент\n        перезапускаться…\n"
      } },
    textDocument = {
      uri = "file:///home/powerman/doc/Main/test.md",
      version = 6
    }
  }
}

It turns out both are the same! No matter they didn't look the same in Vim and here on Github - they're mostly the same, or, more precisely, they begins with same bytes but ends differently.

The problem is first one ends with incomplete Unicode byte sequence. Because of this it detected in different way by file tool (and by Vim):

$ file  /tmp/lsp-encoding.txt  /tmp/lsp-encoding-fixed.txt
/tmp/lsp-encoding.txt:       Non-ISO extended-ASCII text, with LF, NEL line terminators
/tmp/lsp-encoding-fixed.txt: Unicode text, UTF-8 text

The second "-fixed" file is same as first but with last (incomplete UTF-8) symbol removed from the file.

So, it looks like Neovim sends incomplete UTF-8 byte sequence just before closing ", which breaks parsing JSON in that library (I suppose it eats " byte as part of previous UTF-8 sequence and then fails to find closing ").

powerman commented 1 month ago

Resume. Sending incomplete UTF-8 sequence is a Neovim bug for sure. Still, LSP servers shouldn't crash on such an input.

Feel-ix-343 commented 1 month ago

:exploding_head:

That's quite the bug! Your commentary is exceptionally quality.

tower-lsp returns the Parse error right here as a result of serde_json not being able to parse. The library author chose to make the LS crash for this input.

I think from a UX perspective, it would be better if we log an error instead. I can modify the library.

NOTE: the project seems to be dead, and I have been using my own fork for a while

powerman commented 1 month ago

https://github.com/neovim/neovim/issues/29898

Feel-ix-343 commented 5 days ago

This is not an issue with oxide, so closing (cleaning)