diff --git a/cypress/e2e/sync.spec.js b/cypress/e2e/sync.spec.js index 5c58ffb2905..d2a4315685d 100644 --- a/cypress/e2e/sync.spec.js +++ b/cypress/e2e/sync.spec.js @@ -19,10 +19,10 @@ describe('Sync', () => { cy.intercept({ method: 'POST', url: '**/apps/text/session/*/sync' }).as('sync') cy.intercept({ method: 'POST', url: '**/apps/text/session/*/save' }).as('save') cy.openTestFile() - cy.wait('@sync') + cy.wait('@sync', { timeout: 10000 }) cy.getContent().find('h2').should('contain', 'Hello world') cy.getContent().type('{moveToEnd}* Saving the doc saves the doc state{enter}') - cy.wait('@sync') + cy.wait('@sync', { timeout: 10000 }) }) it('saves the actual file and document state', () => { @@ -47,22 +47,11 @@ describe('Sync', () => { }) it('recovers from a short lost connection', () => { - let reconnect = false - cy.intercept('**/apps/text/session/*/*', (req) => { - if (reconnect) { - req.continue() - req.alias = 'alive' - } else { - req.destroy() - req.alias = 'dead' - } - }).as('sessionRequests') + cy.intercept('**/apps/text/session/*/*', req => req.destroy()).as('dead') cy.wait('@dead', { timeout: 30000 }) cy.get('#editor-container .document-status', { timeout: 30000 }) .should('contain', 'Document could not be loaded.') - .then(() => { - reconnect = true - }) + cy.intercept('**/apps/text/session/*/*', req => req.continue()).as('alive') cy.wait('@alive', { timeout: 30000 }) cy.intercept({ method: 'POST', url: '**/apps/text/session/*/sync' }) .as('syncAfterRecovery') @@ -80,6 +69,29 @@ describe('Sync', () => { .should('include', 'after the lost connection') }) + it('reconnects via button after a short lost connection', () => { + cy.intercept('**/apps/text/session/*/*', req => req.destroy()).as('dead') + cy.wait('@dead', { timeout: 30000 }) + cy.get('#editor-container .document-status', { timeout: 30000 }) + .should('contain', 'Document could not be loaded.') + cy.get('#editor-container .document-status') + .find('.button.primary').click() + cy.get('.toastify').should('contain', 'Connection failed.') + cy.get('.toastify', { timeout: 30000 }).should('not.exist') + cy.get('#editor-container .document-status', { timeout: 30000 }) + .should('contain', 'Document could not be loaded.') + // bring back the network connection + cy.intercept('**/apps/text/session/*/*', req => { req.continue() }).as('alive') + cy.intercept('**/apps/text/session/*/create').as('create') + cy.get('#editor-container .document-status') + .find('.button.primary').click() + cy.wait('@alive', { timeout: 30000 }) + cy.wait('@create', { timeout: 10000 }) + .its('request.body') + .should('have.property', 'baseVersionEtag') + .should('not.be.empty') + }) + it('recovers from a lost and closed connection', () => { let reconnect = false cy.intercept('**/apps/text/session/*/*', (req) => { @@ -111,7 +123,7 @@ describe('Sync', () => { .should('include', 'after the lost connection') }) - it('shows warning when document session got cleaned up', () => { + it('asks to reload page when document session got cleaned up', () => { cy.get('.save-status button') .click() cy.wait('@save') diff --git a/src/components/Editor.vue b/src/components/Editor.vue index 0217e4babc1..b15a15b0920 100644 --- a/src/components/Editor.vue +++ b/src/components/Editor.vue @@ -354,12 +354,12 @@ export default { } unsubscribe('text:image-node:add', this.onAddImageNode) unsubscribe('text:image-node:delete', this.onDeleteImageNode) + unsubscribe('text:translate-modal:show', this.showTranslateModal) if (this.dirty) { const timeout = new Promise((resolve) => setTimeout(resolve, 2000)) await Promise.any([timeout, this.$syncService.save()]) } - this.$providers.forEach(p => p.destroy()) - unsubscribe('text:translate-modal:show', this.showTranslateModal) + this.close() }, methods: { initSession() { @@ -373,7 +373,7 @@ export default { guestName, shareToken: this.shareToken, filePath: this.relativePath, - baseVersionEtag: this.$syncService?.baseVersionEtag, + baseVersionEtag: this.$baseVersionEtag, forceRecreate: this.forceRecreate, serialize: this.isRichEditor ? (content) => createMarkdownSerializer(this.$editor.schema).serialize(content ?? this.$editor.state.doc) @@ -383,8 +383,6 @@ export default { this.listenSyncServiceEvents() - this.$providers.forEach(p => p?.destroy()) - this.$providers = [] const syncServiceProvider = createSyncServiceProvider({ ydoc: this.$ydoc, syncService: this.$syncService, @@ -432,7 +430,7 @@ export default { reconnect() { this.contentLoaded = false this.hasConnectionIssue = false - this.close().then(this.initSession) + this.disconnect().then(this.initSession) this.idle = false }, @@ -487,7 +485,7 @@ export default { }) }, - onLoaded({ documentSource, documentState }) { + onLoaded({ document, documentSource, documentState }) { if (documentState) { applyDocumentState(this.$ydoc, documentState, this.$providers[0]) // distribute additional state that may exist locally @@ -500,6 +498,7 @@ export default { this.setInitialYjsState(documentSource, { isRichEditor: this.isRichEditor }) } + this.$baseVersionEtag = document.baseVersionEtag this.hasConnectionIssue = false const language = extensionHighlight[this.fileExtension] || this.fileExtension; @@ -661,17 +660,19 @@ export default { await this.$syncService.save() }, + 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 + }, + async close() { - if (this.currentSession && this.$syncService) { - try { - await this.$syncService.close() - this.unlistenSyncServiceEvents() - this.currentSession = null - this.$syncService = null - } catch (e) { - // Ignore issues closing the session since those might happen due to network issues - } - } + await this.$syncService.sendRemainingSteps(this.$queue) + await this.disconnect() if (this.$editor) { try { this.unlistenEditorEvents() diff --git a/src/components/Editor/GuestNameDialog.vue b/src/components/Editor/GuestNameDialog.vue index cdc4f1ff735..61ae9e64133 100644 --- a/src/components/Editor/GuestNameDialog.vue +++ b/src/components/Editor/GuestNameDialog.vue @@ -55,12 +55,12 @@ export default { }, }, beforeMount() { - this.guestName = this.$syncService.connection.session.guestName + this.guestName = this.$syncService.guestName this.updateBufferedGuestName() }, methods: { setGuestName() { - const previousGuestName = this.$syncService.connection.session.guestName + const previousGuestName = this.$syncService.guestName this.$syncService.updateSession(this.guestName).then(() => { localStorage.setItem('nick', this.guestName) this.updateBufferedGuestName() diff --git a/src/helpers/yjs.js b/src/helpers/yjs.js index c0d696a93d2..da202ca2d9c 100644 --- a/src/helpers/yjs.js +++ b/src/helpers/yjs.js @@ -80,6 +80,26 @@ export function applyUpdateMessage(ydoc, updateMessage, origin = 'origin') { ) } +/** + * Get the steps for sending to the server + * + * @param {object[]} queue - queue for the outgoing steps + */ +export function getSteps(queue) { + return queue.map(s => encodeArrayBuffer(s)) + .filter(s => s < 'AQ') +} + +/** + * Encode the latest awareness message for sending + * + * @param {object[]} queue - queue for the outgoing steps + */ +export function getAwareness(queue) { + return queue.map(s => encodeArrayBuffer(s)) + .findLast(s => s > 'AQ') || '' +} + /** * Log y.js messages with their type and initiator call stack * diff --git a/src/services/PollingBackend.js b/src/services/PollingBackend.js index 739da92feed..2c704ac1d25 100644 --- a/src/services/PollingBackend.js +++ b/src/services/PollingBackend.js @@ -126,7 +126,7 @@ class PollingBackend { } const disconnect = Date.now() - COLLABORATOR_DISCONNECT_TIME const alive = sessions.filter((s) => s.lastContact * 1000 > disconnect) - if (this.#syncService.connection.state.document.readOnly) { + if (this.#syncService.isReadOnly) { this.maximumReadOnlyTimer() } else if (alive.length < 2) { this.maximumRefetchTimer() diff --git a/src/services/SessionApi.js b/src/services/SessionApi.js index 3bf753d38cb..11f6fb3fb5d 100644 --- a/src/services/SessionApi.js +++ b/src/services/SessionApi.js @@ -84,6 +84,10 @@ export class Connection { } } + get isClosed() { + return this.closed + } + get #defaultParams() { return { documentId: this.#document.id, @@ -161,9 +165,12 @@ export class Connection { } close() { - const promise = this.#post(this.#url(`session/${this.#document.id}/close`), this.#defaultParams) - this.closed = true - return promise + return this.#post( + this.#url(`session/${this.#document.id}/close`), + this.#defaultParams, + ).then(() => { + this.closed = true + }) } // To be used in Cypress tests only diff --git a/src/services/SyncService.js b/src/services/SyncService.js index 349dd75f0d0..648bc75d335 100644 --- a/src/services/SyncService.js +++ b/src/services/SyncService.js @@ -10,6 +10,7 @@ import debounce from 'debounce' import PollingBackend from './PollingBackend.js' import SessionApi, { Connection } from './SessionApi.js' +import { getSteps, getAwareness } from '../helpers/yjs.js' import { logger } from '../helpers/logger.js' /** @@ -52,6 +53,7 @@ const ERROR_TYPE = { class SyncService { #sendIntervalId + #connection constructor({ baseVersionEtag, serialize, getDocumentState, ...options }) { /** @type {import('mitt').Emitter} _bus */ @@ -60,7 +62,7 @@ class SyncService { this.serialize = serialize this.getDocumentState = getDocumentState this._api = new SessionApi(options) - this.connection = null + this.#connection = null this.stepClientIDs = [] @@ -76,6 +78,14 @@ class SyncService { return this } + get isReadOnly() { + return this.#connection.state.document.readOnly + } + + get guestName() { + return this.#connection.session.guestName + } + async open({ fileId, initialSession }) { const connect = initialSession @@ -83,20 +93,20 @@ class SyncService { : this._api.open({ fileId, baseVersionEtag: this.baseVersionEtag }) .catch(error => this._emitError(error)) - this.connection = await connect - if (!this.connection) { + this.#connection = await connect + if (!this.#connection) { // Error was already emitted in connect return } - this.backend = new PollingBackend(this, this.connection) - this.version = this.connection.docStateVersion - this.baseVersionEtag = this.connection.document.baseVersionEtag + this.backend = new PollingBackend(this, this.#connection) + this.version = this.#connection.docStateVersion + this.baseVersionEtag = this.#connection.document.baseVersionEtag this.emit('opened', { - ...this.connection.state, + ...this.#connection.state, version: this.version, }) this.emit('loaded', { - ...this.connection.state, + ...this.#connection.state, version: this.version, }) } @@ -118,10 +128,10 @@ class SyncService { } updateSession(guestName) { - if (!this.connection.isPublic) { + if (!this.#connection.isPublic) { return Promise.reject(new Error()) } - return this.connection.update(guestName) + return this.#connection.update(guestName) .catch((error) => { logger.error('Failed to update the session', { error }) return Promise.reject(error) @@ -135,7 +145,7 @@ class SyncService { } return new Promise((resolve, reject) => { this.#sendIntervalId = setInterval(() => { - if (this.connection && !this.sending) { + if (this.#connection && !this.sending) { this.sendStepsNow(getSendable).then(resolve).catch(reject) } }, 200) @@ -150,12 +160,12 @@ class SyncService { if (data.steps.length > 0) { this.emit('stateChange', { dirty: true }) } - return this.connection.push(data) + return this.#connection.push(data) .then((response) => { this.sending = false this.emit('sync', { steps: [], - document: this.connection.document, + document: this.#connection.document, version: this.version, }) }).catch(err => { @@ -210,7 +220,7 @@ class SyncService { this.emit('sync', { steps: newSteps, // TODO: do we actually need to dig into the connection here? - document: this.connection.document, + document: this.#connection.document, version: this.version, }) } @@ -232,7 +242,7 @@ class SyncService { async save({ force = false, manualSave = true } = {}) { logger.debug('[SyncService] saving', arguments[0]) try { - const response = await this.connection.save({ + const response = await this.#connection.save({ version: this.version, autosaveContent: this._getContent(), documentState: this.getDocumentState(), @@ -240,7 +250,7 @@ class SyncService { manualSave, }) this.emit('stateChange', { dirty: false }) - this.connection.document.lastSavedVersionTime = Date.now() / 1000 + this.#connection.document.lastSavedVersionTime = Date.now() / 1000 logger.debug('[SyncService] saved', response) const { document, sessions } = response.data this.emit('save', { document, sessions }) @@ -261,27 +271,47 @@ class SyncService { }) } + async sendRemainingSteps(queue) { + if (queue.length === 0) { + return + } + let outbox = [] + const steps = getSteps(queue) + const awareness = getAwareness(queue) + return this.sendStepsNow(() => { + const data = { steps, awareness, version: this.version } + outbox = [...queue] + logger.debug('sending final steps ', data) + return data + })?.then(() => { + // only keep the steps that were not send yet + queue.splice(0, + queue.length, + ...queue.filter(s => !outbox.includes(s)), + ) + }, err => logger.error(err)) + } + async close() { // Make sure to leave no pending requests behind. this.autosave.clear() this.backend?.disconnect() - return this._close() - } - - _close() { - if (this.connection === null) { - return Promise.resolve() + if (!this.#connection || this.#connection.isClosed) { + return } - this.backend.disconnect() - return this.connection.close() + return this.#connection.close() + // Log and ignore possible network issues. + .catch(e => { + logger.info('Failed to close connection.', { e }) + }) } uploadAttachment(file) { - return this.connection.uploadAttachment(file) + return this.#connection.uploadAttachment(file) } insertAttachmentFile(filePath) { - return this.connection.insertAttachmentFile(filePath) + return this.#connection.insertAttachmentFile(filePath) } on(event, callback) { diff --git a/src/services/WebSocketPolyfill.js b/src/services/WebSocketPolyfill.js index 32eb0b96f68..b06ba0d2631 100644 --- a/src/services/WebSocketPolyfill.js +++ b/src/services/WebSocketPolyfill.js @@ -4,7 +4,8 @@ */ import { logger } from '../helpers/logger.js' -import { encodeArrayBuffer, decodeArrayBuffer } from '../helpers/base64.ts' +import { decodeArrayBuffer } from '../helpers/base64.ts' +import { getSteps, getAwareness } from '../helpers/yjs.js' /** * @@ -69,8 +70,8 @@ export default function initWebSocketPolyfill(syncService, fileId, initialSessio let outbox = [] return syncService.sendSteps(() => { const data = { - steps: this.#steps, - awareness: this.#awareness, + steps: getSteps(queue), + awareness: getAwareness(queue), version: this.#version, } outbox = [...queue] @@ -86,48 +87,13 @@ export default function initWebSocketPolyfill(syncService, fileId, initialSessio }, err => logger.error(err)) } - get #steps() { - return queue.map(s => encodeArrayBuffer(s)) - .filter(s => s < 'AQ') - } - - get #awareness() { - return queue.map(s => encodeArrayBuffer(s)) - .findLast(s => s > 'AQ') || '' - } - async close() { - await this.#sendRemainingSteps() Object.entries(this.#handlers) .forEach(([key, value]) => syncService.off(key, value)) this.#handlers = [] - syncService.close().then(() => { - this.onclose() - }) + this.onclose() logger.debug('Websocket closed') } - #sendRemainingSteps() { - if (queue.length) { - let outbox = [] - return syncService.sendStepsNow(() => { - const data = { - steps: this.#steps, - awareness: this.#awareness, - version: this.#version, - } - outbox = [...queue] - logger.debug('sending final steps ', data) - return data - })?.then(() => { - // only keep the steps that were not send yet - queue.splice(0, - queue.length, - ...queue.filter(s => !outbox.includes(s)), - ) - }, err => logger.error(err)) - } - } - } }