Mismatch between server and client text

[davidanthoff]

:( still not fixed...

From crash reporting.

Error message: Mismatch between server and client text for file:///XXX.jl.

Stack trace:

ErrorException:
   at error(::String) (error.jl33)
   at process(::LanguageServer.JSONRPC.Request{Val{Symbol("textDocument/didSave")},LanguageServer.DidSaveTextDocumentParams}, ::LanguageServerInstance) (./scripts/languageserver/packages/LanguageServer/src/requests/textdocument.jl79)
   at run(::LanguageServerInstance) (./scripts/languageserver/packages/LanguageServer/src/languageserverinstance.jl217)
   at top-level scope (./scripts/languageserver/main.jl28)
   at include (boot.jl328)
   at include_relative(::Module, ::String) (loading.jl1105)
   at include(::Module, ::String) (Base.jl31)
   at exec_options(::Base.JLOptions) (client.jl287)
   at _start() (client.jl460)

[davidanthoff]

I'm pretty lost about this one... Maybe we need to add some more diagnostics in this crash situation, for example whether that file was open in the editor or not, or something like that? Or maybe the version number, maybe that could tell us something interesting?

We could also turn off the incremental sync for a while and see whether we still get these kind of errors? That would maybe allow us to tell whether there is some problem in that?

[davidanthoff]

There are now many more crash reports like that. The ones I looked at all had a pretty high version field (we now get a bit more diagnostics), and they are all open in the editor... My best guess is that there is simply some error in the way we handle the didChange request?

One thing we could try is to temporarily disable incremental sync and check whether that gets rid of the error?

[davidanthoff]

I've asked upstream whether our interpretation that discrepancies here should be treated as bugs is actually correct.

[ZacLN]

What about a direct approach - we send a custom request from the server to the client on each didChange and compare the text? Then we also catch the text sync that's not being handled correctly. If we're getting enough crashes off of the insiders release we may be able to track this down quite quickly.

[davidanthoff]

I can't find any crash reports on insider right now, only in the release channel... But even there I can't find them right now because they are all lumped together with the other 1.3k thousand errors that used error ;)

But I guess we could also do this in the shipping version for a while?

[davidanthoff]

Btw., we should try to replace all calls to error with more specific exception types, makes the whole crash reporting so much easier to classify.

[davidanthoff]

Actually, I think there would be another problem: when we send our getContent msg from the server to the client, the content on the client might already have changed again. The client could have sent a new didChange request in the meantime, while we are still processing a previous didChange request on the server side...

[ZacLN]

We can compare the version number of the document to account for that

[davidanthoff]

I took another look at the text sync code itself, and I'm pretty confused right now about 0 and 1 based etc...

My sense right now is that get_offset does not return valid index values, right? Instead it returns a 0 based index into the content string? Or something else?

But then in edit_string it seems that the values that are returned by get_offset are actually used to index into the content string. So just looking at edit_string, that seems like a bug to me...

Wouldn't all of this be a lot easier if we just said that get_offset returns valid index values into the content string? And equally, if get_line_offsets also computed valid string index values?

[davidanthoff]

I'm experimenting with some code now that uses a 1 based story, will open a PR soon (hopefully).