Fix null check issue causing unreadable documents

[tmathew0309]

We ran into a problem while using yjs where a stored document binary became unreadable. The stack trace we got is below:

org.graalvm.polyglot.PolyglotException: Uncaught TypeError: Cannot read properties of undefined (reading 'origin')
	at <js>.integrate(yjsBackendBundle.js:19628)
	at <js>.integrateStructs(yjsBackendBundle.js:11897)
	at <js>.:=>(yjsBackendBundle.js:11962)
	at <js>.transact(yjsBackendBundle.js:13628)
	at <js>.readUpdateV2(yjsBackendBundle.js:11950)
	at <js>.applyUpdateV2(yjsBackendBundle.js:12045)
	at <js>.applyUpdate(yjsBackendBundle.js:12059)

The line numbers above don't make sense because they come from our bundled application but the error originates here:

yjs/src/structs/Item.js

Line 465 in a22b3cd

if (compareIDs(this.origin, o.origin)) {

.

Don't understand the context around this code well enough but generally observe that o can be undefined and think we should guard against both null and undefined instead of the strict check against null. Let me know if this makes sense @dmonad

_{Huly®: YJS-457}

[dmonad]

Hi @tmathew0309,

I experienced an issue like this before but wasn't able to easily reproduce it. The thing is, o should never be undefined. Adding this check might fix this immediate issue, but it will not fix the underlying issue that the predecessor of o has been garbage-collected (which shouldn't happen in this case). I'm more concerned that this fix will lead to issues that are harder to debug.

I'm currently working on a major refactor of how we interact with list-items. Hopefully, it will fix the issue. It doesn't make sense to fix this issue first.

In the meantime, if you could find a way to easily reproduce this issue, that would be very helpful. I would add it as a test to yjs and ensure that it doesn't happen again after the refactor lands (1-3 weeks).

[dmonad]

I'm going to close this because it does not fix the underlying issue. But please feel free to create a ticket to track this.

[tmathew0309]

Ok that's fair @dmonad. Added an issue here to track: #417. Will look for your list-items refactor and upgrade to that.

In the meantime do you have any recommended approaches to recovering the existing document? It is currently corrupted and the user has lost what they were working on.

[dmonad]

Thank you for opening a ticket @tmathew0309!

The best approach would be to restore a backup if you have one. I could also manually filter out the corrupted operation. However, I recommend to use your fix to restore as much of the old document as possible. Export it to a different format (e.g. copy paste the prosemirror document to a different Yjs document).

[pietrodn]

I was able to spot a few instances of this bug on our backend as well. The errors seems to be very rare and I have (for now) what idea about the conditions that may trigger it.

[dmonad]

I fixed this issue in the last release. If it still persists, please open a new ticket