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

model.set() type mismatch #165

Closed
ile opened this issue Oct 27, 2013 · 4 comments
Closed

model.set() type mismatch #165

ile opened this issue Oct 27, 2013 · 4 comments

Comments

@ile
Copy link

ile commented Oct 27, 2013

Here's something that should be easy to fix, but will mess up the whole derby application/state when this happens:

Errors that will be shown:

Uncaught Error: Referenced element not an object (it was [null,"hello 1"]) json0.js:101
Uncaught Error: Referenced element not a list json0.js:96

Code: https://gist.github.com/ile/7177459

explanation

If a model property is set to something that is an object and after that one tries to set a value into it where the key looks like an array key (numerical), an error will be thrown. The same when doing this vice versa (i.e. first set a property that makes it a list, and then setting a value that looks like an object key).

After that error, derby seems to go into a strange state and behave unexpectedly - for example page rendering gets "messed up" so that going to a new page will have a long delay. Also this error will be thrown at some point:

Uncaught Error: Cannot call submitOp from inside an 'op' event handler events.js:27

(actually that strange behavior will happen after that error)

@vmakhaev
Copy link
Contributor

vmakhaev commented Dec 3, 2013

For proper OT implementation on json data type ShareJS should know exactly if this is Array or Object. Because OT is different for them.

Actually error is thrown in ottypes: https://github.com/share/ottypes/blob/master/lib/json0.js#L92

Just treat it like ShareJS limitation. You can not use numbers as Object keys.

@ile
Copy link
Author

ile commented Dec 4, 2013

Yes, probably true, but this will mess up the internal state of Derby as well, and that's something that could be prevented.

That is to say: the behavior of Derby changes completely after this, this is not just a single error but will "break the whole thing" after this has happened.

@vmakhaev
Copy link
Contributor

vmakhaev commented Dec 4, 2013

When error thrown by ottype, it break execution of ShareJS's doc._otApply function at this place: https://github.com/share/ShareJS/blob/master/lib/client/doc.js#L660
Before it can set this.locked to false: https://github.com/share/ShareJS/blob/master/lib/client/doc.js#L675
So Doc stays locked for any other changes and Uncaught Error: Cannot call submitOp from inside an 'op' event handler events.js:27 is thrown during any further mutations.

How this can be solved? Do we need to wrap type.apply to try/catch block and emit error on catch?

@nateps
Copy link
Contributor

nateps commented Apr 10, 2014

For now, this is intended behavior.

@nateps nateps closed this as completed Apr 10, 2014
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