artempyanykh / marksman

Write Markdown with code assist and intelligence in the comfort of your favourite editor.
MIT License
1.98k stars 35 forks source link

A link to the root page crashes Marksman #275

Closed TheBlob42 closed 10 months ago

TheBlob42 commented 10 months ago

I just recently worked on a docusaurus documentation with a file that contained a link to the root page [<text>](/) which crashes Marksman with the following error message:

"[09:23:33 INF] <LSP Entry> Starting Marksman LSP server: {}\n"
"[09:23:50 INF] <LSP Entry> Starting Marksman LSP server: {}\n"
"---------------------------------------------------------------------------"
"\n"
"Marksman encountered a fatal error\nPlease, report the error at https://github.com/artempyanykh/marksman/issues\n"
"---------------------------------------------------------------------------\nMarksman version: "
"1.0.0.0\n"
"OS: "
"Linux 5.15.0-89-generic #99~20.04.1-Ubuntu SMP Thu Nov 2 15:16:47 UTC 2023\nArch: "
"X64\n---------------------------------------------------------------------------"
"\n"
"String  couldn't be converted to a path\n"
"   at Marksman.Paths.LocalPathModule.ofSystem@177-1.Invoke(Unit unitVar0) in /home/runner/work/marksman/marksman/Marksman/Paths.fs:line 177\n   at Marksman.Names.InternNameModule.tryAsPath$cont@112(DocId src, String name, Unit unitVar) in /home/runner/work/marksman/marksman/Marksman/Names.fs:line 113\n   at Marksman.Names.InternNameModule.tryAsPath(InternName _arg1) in /home/runner/work/marksman/marksman/Marksman/Names.fs:line 111\n   at Marksman.Folder.Oracle.filterDocsByName(FolderData data, FolderLookup lookup, InternName name) in /home/runner/work/marksman/marksman/Marksman/Folder.fs:line 189\n   at Marksman.Folder.Oracle.resolveToDoc(FolderData data, FolderLookup lookup, DocId fromDoc, Ref ref) in /home/runner/work/marksman/marksman/Marksman/Folder.fs:line 200\n   at Marksman.Folder.Oracle.oracle@232.Invoke(Scope scope, Ref ref) in /home/runner/work/marksman/marksman/Marksman/Folder.fs:line 233\n   at Marksman.Conn.ConnModule.updateAux(Oracle oracle, Difference`1 _arg1, Conn conn) in /home/runner/work/marksman/marksman/Marksman/Conn.fs:line 390\n   at Marksman.Conn.ConnModule.update(Oracle oracle, Difference`1 diff, Conn conn) in /home/runner/work/marksman/marksman/Marksman/Conn.fs:line 432\n   at Marksman.Conn.ConnModule.mk(Oracle oracle, MMap`2 symMap) in /home/runner/work/marksman/marksman/Marksman/Conn.fs:line 445\n   at Marksman.Folder.FolderModule.mk(FolderData data) in /home/runner/work/marksman/marksman/Marksman/Folder.fs:line 459\n   at Marksman.Folder.FolderModule.multiFile(String name, UriWith`1 root, IEnumerable`1 docs, FSharpOption`1 config) in /home/runner/work/marksman/marksman/Marksman/Folder.fs:line 477\n   at Marksman.Folder.FolderModule.tryLoad(FSharpOption`1 userConfig, String name, UriWith`1 folderId) in /home/runner/work/marksman/marksman/Marksman/Folder.fs:line 507\n   at Marksman.Server.ServerUtil.readWorkspace(FSharpOption`1 userConfig, FSharpMap`2 roots) in /home/runner/work/marksman/marksman/Marksman/Server.fs:line 88\n   at Marksman.Server.MarksmanServer.Initialize(InitializeParams par) in /home/runner/work/marksman/marksman/Marksman/Server.fs:line 533\n"

Reproducing

Just create a markdown file with the following content to reproduce the crash:

[](/)

Expected Result

I totally understand that such a link does not work for the "goto definition" or reference functionality, but it should not crash the whole server and make it unusable for the rest of the project. Also the error message in this case is not entirely clear. I got a bit lucky to find the root cause :sweat_smile:

Seems like the parsing here can not detect a "root path" correctly. Haven't had any more detailed look into this yet.

artempyanykh commented 10 months ago

Thanks for reporting @TheBlob42! Crash is almost always a bug, and in this case it definitely is! Patches are welcome 🙂

Also the error message in this case is not entirely clear. I got a bit lucky to find the root cause 😅

I'd be happy to make error messages clearer! Is there something in particular that was confusing?

TheBlob42 commented 10 months ago

Well I should have specified it a little better. The error message String <string> couldn't be converted to a path\n was absolutely clear. The problem was the empty string being inserted into it. This led me believe I need to search for an "empty" link in the (quite big) documentation project. But the "real" reason was the / path which I guess is somehow converted to an empty string here for this error message.

This threw me off. Never the less I was able to figure it out :slightly_smiling_face:

It would be awesome to include the file, line and column of a particular problematic element into the error message (in case of a parsing issue), but I am not sure how easy such a change would be :thinking:

TheBlob42 commented 10 months ago

After some binary search I found this commit: https://github.com/artempyanykh/marksman/commit/e4366cde7acf8d0c015c5b19fb31d2e43abb7ee5 to be the one introducing the issue (reverting to the one before it resolves the crash). A little strange for me since nothing changed in the path parsing, but maybe you have a good idea what could be the issue here