Jeffail / leaps

A pair programming service using operational transforms
MIT License
754 stars 55 forks source link

Local editor has lost synchronization with server #47

Closed qknight closed 6 years ago

qknight commented 6 years ago

problem description

we use leaps (0.6.0) with codemirror (with an old version but also with 5.26). it used to work months ago and then we found that opening the same document in two tabs (one in firefox, the other in chromium) leads to this error message in the javascript error console of tab A:

Local editor has lost synchronization with server

when one enters any letter in tab B (or vice versa) but, and this puzzles me, when both tabs are reloaded i can use del key to remove a letter or space and it keeps the sync.

seeking advice

now i don't know exactly where to search for a solution, any hints what could cause this would be great.

what i've done so far:

possible causes

Jeffail commented 6 years ago

Hey @qknight, are there any non-ascii characters inside the content you are editing? Errors like this usually suggest that events being fired from the editor (in this case CodeMirror) are returning unexpected character lengths for certain operations. This has happened a lot in the past when working with large non-ascii characters since support for them in these editors (and JavaScript in general) is often very poor.

qknight commented 6 years ago

ATM it seems that the leaps library on the tab A receives the updates and performs the transaction but fails to transparently modify the codemirror document.

client/leap-bind-codemirror.js

in the function: leap_bind_codemirror.prototype._apply_transform = function(transform) { i see that: this._content = this._leap_client.apply(transform, this._content); is correctly set but the this._codemirror.getDoc().getValue() afterwards returns the old value and thus the Local editor has lost synchronization with server is triggered.

conclusion

the leaps javascript client library, still based on 0.6.0, receives the event properly inside the browser, but the codemirror inside the webpage isn't updated properly.

question

what mechanism is used to update the document inside the codemirror from a transform? i see only one reference to update the whole document binder._codemirror.getDoc().setValue(doc.content); found in leap_bind_codemirror which is used initially to set the document. a call to setValue(...) will disrupt the current cursor position!

so my theory is that the codemirror document update from the leaps js library must somehow touch the internals of codemirror. this is probably what is not working.

qknight commented 6 years ago

conclusion

i'm confident that live_document.replaceRange(insert, start_position, end_position); does not work at all for some reason.

qknight commented 6 years ago

found the bug!

the code as it was:

    //if ( typeof(transform.insert) === "string" && transform.insert.length > 0 ) {

the code how it works instead:

    if ( typeof(transform.insert._str) === "string" && transform.insert._str.length > 0 ) {
            insert = transform.insert._str;

solution

typeof(transform.insert) === "string" is never true, as insert is of type object.

Jeffail commented 6 years ago

@qknight nice find! It looks as though during version 0.6.0 to 0.6.1 I was making changes to the JS client lib but only porting changes to the ACE binding. If you decide in the future to upgrade to 0.8.4 you should be okay to use the CodeMirror lib that comes with it as it looks as though I eventually updated it, but your fix should work fine for now. Although it might be safer to use transform.insert.str() instead of transform.insert._str.

I'm sorry that you were hit by this, at the time I was racing through major API changes and clearly struggled to keep all the components synced up. Glad you were able to solve it!

qknight commented 6 years ago

we'll update to 0.8.4 ASAP but we just didn't do that yet.