Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc.submitOp() called from server #162

Closed
nickasd opened this issue Jun 27, 2017 · 11 comments
Closed

doc.submitOp() called from server #162

nickasd opened this issue Jun 27, 2017 · 11 comments

Comments

@nickasd
Copy link

nickasd commented Jun 27, 2017

What is the correct way (if there is any) of applying a document operation from the server side? I noticed that when applying an invalid delete operation (one that tries to delete some text at a position where the text is different), an exception is thrown. I tried:

var doc = this.connection.get('files', '0');
doc.create({content: 'asdf'}, (error) => {
    var ops = [{p: 0, d: '0'}];
    var op = [{p: ['content'], t: 'text0', o: ops}];
    doc.submitOp(op, (error) => {
        if (error) {
            console.error(error);
        } else {
            console.log('success');
        }
    });
});

which raises the following exception:

Error: Delete component '0' does not match deleted text 'a'
    at Object.text.apply (ot-json0/lib/text0.js:71:15)
    at Object.json.apply (ot-json0/lib/json0.js:171:33)
    at Doc._otApply (sharedb/lib/client/doc.js:552:27)
    at Doc._submit (sharedb/lib/client/doc.js:650:8)
    at Doc.submitOp (sharedb/lib/client/doc.js:748:8)

and I tried also:

var doc = this.connection.get('files', '0');
doc.create({content: 'asdf'}, (error) => {
    var ops = [{p: 0, d: '0'}];
    var op = [{p: ['content'], t: 'text0', o: ops}];
    try {
        doc.submitOp(op);
        console.log('success');
    } catch (e) {
        console.error(e.message);
    }
});

but here I get even 2 exceptions thrown outside of the catch block:

Error: Delete component '0' does not match deleted text 'a'
    at Object.text.apply (ot-json0/lib/text0.js:71:15)
    at Object.json.apply (ot-json0/lib/json0.js:171:33)
    at applyOpEdit (sharedb/lib/ot.js:110:26)
    at Object.exports.apply (sharedb/lib/ot.js:92:15)
    at sharedb/lib/submit-request.js:131:14
    at Backend.trigger (sharedb/lib/backend.js:137:20)
    at SubmitRequest.apply (sharedb/lib/submit-request.js:127:16)
    at sharedb/lib/submit-request.js:83:22
    at sharedb/lib/db/memory.js:64:5
    at _combinedTickCallback (internal/process/next_tick.js:95:7)
events.js:182
      throw er; // Unhandled 'error' event
      ^

and

Error: Delete component '0' does not match deleted text 'a'
    at Connection.handleMessage (sharedb/lib/client/connection.js:164:11)
    at StreamSocket.socket.onmessage (sharedb/lib/client/connection.js:117:18)
    at sharedb/lib/stream-socket.js:59:12
    at _combinedTickCallback (internal/process/next_tick.js:95:7)
    at process._tickCallback (internal/process/next_tick.js:161:9)

On the client I can simply use the first code without any problem.

@nickasd
Copy link
Author

nickasd commented Feb 14, 2019

Also looking for a way to do this.

Any ideas someone? Did you solve it @nickasd?

No, I haven't looked into this for some time now, but it would be nice to get some help from someone.

@adelriosantiago
Copy link

adelriosantiago commented Feb 14, 2019

Wow, quick response! Thanks @nickasd!

But you won't believe this: I just tested the first code that you wrote and it just works. Maybe a bug was fixed in recent versions or the number is somehow intepreted as an integer, non-text.

I am getting a "success" trying to delete an "a" from "abc123" with var ops = [{p: 0, d: 'a'}];

@alecgibson
Copy link
Collaborator

Regarding the failed submission, I suspect I've recently fixed that with this PR: #259

If it all seems in order, should I close this issue?

@adelriosantiago
Copy link

@alecgibson Kudos to you, thanks! I think we can close this issue.

@nickasd
Copy link
Author

nickasd commented Feb 24, 2019

@alecgibson I don't think the issue is resolved. @adelriosantiago trying to delete an "a" from "abc123" is supposed to work, what shouldn't work is deleting an "0" from "abc123".
The stack trace I get now is:

    at Object.text.apply (ot-json0/lib/text0.js:71:15)
    at Object.json.apply (ot-json0/lib/json0.js:171:33)
    at Doc._otApply (sharedb/lib/client/doc.js:564:27)
    at Doc._submit (sharedb/lib/client/doc.js:663:10)
    at Doc.submitOp (sharedb/lib/client/doc.js:764:8)

@alecgibson
Copy link
Collaborator

@nickasd can you please put together a failing test case?

@nickasd
Copy link
Author

nickasd commented Feb 25, 2019

It seems to have been fixed indeed. I was confused by the stack trace which didn't contain the line where I log the error (obviously). But in setting up the test case I possibly found another problem: both error 1 and error 2 get logged, which looks like the callback to doc.create is getting called twice, and both times the error is Error: Delete component '0' does not match deleted text 'a'. I can open another report if necessary:

var ShareDB = require('sharedb');
var sharedb = new ShareDB();
var connection = sharedb.connect();
var doc = connection.get('files', '0');
doc.create({content: 'asdf'}, (error) => {
    if (error) {
        console.log('error 1');
        console.error(error);
    } else {
        var ops = [{p: 0, d: '0'}];
        var op = [{p: ['content'], t: 'text0', o: ops}];
        doc.submitOp(op, (error) => {
            if (error) {
                console.log('error 2');
                console.error(error);
            } else {
                console.log('success');
            }
        });
    }
});

@alecgibson
Copy link
Collaborator

@nickasd this looks like a separate issue. I've had a very quick dig, and it looks like it has to do with the combination of _clearInflightOp and _hardRollback.

_clearInflightOp tries to call each of the callbacks before clearing the inflightOp. In this case, that includes calling a callback which will produce an error. The _hardRollback then also calls the create callback, which - as you correctly point out - means that we call the creation op callback twice.

We've got something like this happening:

  • submit creation
  • set creation to inflight op
  • receive acknowledgement of creation
  • call the creation op callback
  • creation op callback includes a bad op submission
  • bad op submission errors
  • we attempt to hard rollback
  • hard rollback tries to call the create callback again

I don't have a lot of time to think about how best to handle this right now - may be best to chat with @nateps and @ericyhwang in Wednesday's meeting. Potentially best to also move this to a new issue.

My first instinct is that when we invoke a callback, we should first dissociate it from the op it was attached to, so that we don't get this repeat call (that or mark it as called, so we don't try to call it again), but I don't know what the ramifications of that behaviour might be.

Actually, now thinking about it a bit more, perhaps we should be moving more towards having a light Callback class, which remembers if it was called or not, because we're already trying to keep track of that sort of thing anyway.

@nickasd
Copy link
Author

nickasd commented Feb 25, 2019

@alecgibson Thanks for your help! I opened a new report #272.

@adelriosantiago
Copy link

adelriosantiago commented Feb 25, 2019

I've found out that calling submitOp() called from the server is not very reliable. Or at least I couldn't make it work as I expected. Even if the operation is correctly submitted, clients are not refreshed with the new information and as soon as a client overwrites the content the operation is completely lost unless the client refreshes first. I couldn't find how to force a client refresh from the server.

But I found a workaround to -more or less- solve the issue of writing from the server. It working good so far by creating a "fake" client running on a Chrome headless browser. It is not the best solution but it works well and all clients are refreshed as expected. In case someone is interested in this approach, the steps to create this "fake" client are:

  • Install puppetter module
  • const puppeteer = require("puppeteer");
  • Start a session and begin writing (here I'm starting the session with headless: false for practicality)
    (async () => {
      const browser = await puppeteer.launch({ headless: false });
      monitor = await browser.newPage();
      await monitor.goto(`localhost:3000/`);
      //Begin writing
      await monitor.focus('#input');
      await monitor.keyboard.type('writing from server');
    })();

@nickasd
Copy link
Author

nickasd commented Feb 25, 2019

@adelriosantiago That's a nice workaround, but let's hope that the issue will be fixed nonetheless.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants