Skip to content

Commit

Permalink
fix(sync): reuse open connection
Browse files Browse the repository at this point in the history
Do not attempt to create a new connection
if there already is one and it is not closed.

If no messages are received for 30 seconds
yjs will open a new websocket.

Since we do not close the connection anymore from the websocket polyfill
we also do not need to open it.

If the network connection has gone down
creating a new connection will fail anyway.

Once it comes back we will know if the session is still valid.
Then we can either continue using it or reconnect.

This is part of #6050.

Signed-off-by: Max <max@nextcloud.com>
  • Loading branch information
max-nextcloud committed Jul 19, 2024
1 parent 6256dc2 commit 0ec22ea
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 2 deletions.
6 changes: 6 additions & 0 deletions cypress/e2e/sync.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ describe('Sync', () => {

it('recovers from a lost and closed connection', () => {
let reconnect = false
// block all requests until the session is closed and reopened
cy.intercept('**/apps/text/session/*/*', (req) => {
if (req.url.includes('close') || req.url.includes('create') || reconnect) {
req.continue()
Expand All @@ -125,6 +126,11 @@ describe('Sync', () => {
cy.get('#editor-container .document-status', { timeout: 30000 })
.should('contain', 'Document could not be loaded.')

// Reconnect button works - it closes and reopens the session
cy.get('#editor-container .document-status a.button')
.contains('Reconnect')
.click()

cy.wait('@syncAfterRecovery', { timeout: 60000 })

cy.get('#editor-container .document-status', { timeout: 30000 })
Expand Down
6 changes: 4 additions & 2 deletions src/services/SyncService.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,12 +104,14 @@ class SyncService {
}

async open({ fileId, initialSession }) {

if (this.#connection && !this.#connection.isClosed) {
// We're already connected.
return
}
const connect = initialSession
? Promise.resolve(new Connection({ data: initialSession }, {}))
: this._api.open({ fileId, baseVersionEtag: this.baseVersionEtag })
.catch(error => this._emitError(error))

this.#connection = await connect
if (!this.#connection) {
// Error was already emitted in connect
Expand Down

0 comments on commit 0ec22ea

Please sign in to comment.