Keep base version etag during reload#5941
Conversation
|
I'm looking into the flaky sync tests and fixing some issues with lost network connectivity on the way. |
max-nextcloud
force-pushed
the
fix/keep-base-version-etag-during-reload
branch
from
df1131e to
dab63df
Compare
juliusknorr
left a comment
juliusknorr
left a comment
There was a problem hiding this comment.
Very nice, thanks also for the nicely split commits. Makes reviewing a very straight forward ❤️
Signed-off-by: Max <max@nextcloud.com>
`this.$syncService` is cleared during the `close` method. However we need the `baseVersionEtag` to ensure the editing session on the server is still in sync with our local ydoc. Signed-off-by: Max <max@nextcloud.com>
Signed-off-by: Max <max@nextcloud.com>
Signed-off-by: Max <max@nextcloud.com>
Signed-off-by: Max <max@nextcloud.com>
* `close` is for closing the editor. It tries to save the document and clean everything up. * `disconnect` is for cleaning up the current collaboration sessions. It will not save the document and asumes the editing will be resumed after a reconnect. Move `sendRemainingSteps` out to the sync service. Also make close in the websocket polyfill sync. Just clean up the polyfills state. Signed-off-by: Max <max@nextcloud.com>
Signed-off-by: Max <max@nextcloud.com>
max-nextcloud
force-pushed
the
fix/keep-base-version-etag-during-reload
branch
from
e056263 to
13c580c
Compare
|
/backport to stable29 |
|
/backport to stable28 |
mejo-
left a comment
mejo-
left a comment
There was a problem hiding this comment.
Did some testing and seems to work well. Thanks so much for looking into it.
When cutting the network and pressing "Reconnect", we run into
sendStepsNow(): this.connection is undefined because the connection got closed but sendRemainingSteps() still tries to run sendStepsNow().
In the same scenario Error: Close has already been called on the connection is also still logged.
I tried but failed to reproduce these two. I also don't see how that would happen as reconnect should not send the remaining steps anymore. But I will take a look at the code to confirm. |
| this.close().then(this.initSession) | ||
| this.disconnect().then(this.initSession) |
There was a problem hiding this comment.
reconnect calling disconnect rather than close now.
| async disconnect() { | ||
| await this.$syncService.close() | ||
| this.unlistenSyncServiceEvents() | ||
| this.$providers.forEach(p => p?.destroy()) | ||
| this.$providers = [] | ||
| this.$syncService = null | ||
| // disallow editing while still showing the content | ||
| this.readOnly = true | ||
| }, | ||
|
|
There was a problem hiding this comment.
There's no sendRemainingSteps here.
|
Merged as i could not reproduce the remaining issue. @mejo- Please open a new issue based on the comment if this still happens for you. |
3 participants
📝 Summary
this.$syncServiceis cleared during theclosemethod.However we need the
baseVersionEtagto ensure the editing sessionon the server
is still in sync with our local ydoc.
See #5724 (comment) for a trace of the effect of this.
Also fixes #5726
🏁 Checklist
npm run lint/npm run stylelint/composer run cs:check)