diff --git a/src/components/Editor.vue b/src/components/Editor.vue index d3779093277..4ed6d9c5385 100644 --- a/src/components/Editor.vue +++ b/src/components/Editor.vue @@ -562,14 +562,14 @@ export default { this.document = document this.syncError = null - const editable = !this.readOnly + const editable = !this.readOnly && !this.hasConnectionIssue if (this.$editor.isEditable !== editable) { this.$editor.setEditable(editable) } }, onSync({ steps, document }) { - this.hasConnectionIssue = false + this.hasConnectionIssue = this.$syncService.backend.fetcher === 0 || !this.$providers[0].wsconnected || this.$syncService.pushError > 0 this.$nextTick(() => { this.emit('sync-service:sync') }) diff --git a/src/services/SyncService.js b/src/services/SyncService.js index c13481d0b72..f0128c57757 100644 --- a/src/services/SyncService.js +++ b/src/services/SyncService.js @@ -95,6 +95,8 @@ class SyncService { this.autosave = debounce(this._autosave.bind(this), AUTOSAVE_INTERVAL) + this.pushError = 0 + return this } @@ -106,10 +108,23 @@ class SyncService { return this.#connection.session.guestName } + get hasActiveConnection() { + return this.#connection && !this.#connection.isClosed + } + + get connectionState() { + if (!this.#connection || this.version === undefined) { + return null + } + return { + ...this.#connection.state, + version: this.version, + } + } + async open({ fileId, initialSession }) { - if (this.#connection && !this.#connection.isClosed) { - // We're already connected. - return + if (this.hasActiveConnection) { + return this.connectionState } const onChange = ({ sessions }) => { this.sessions = sessions @@ -124,19 +139,15 @@ class SyncService { if (!this.#connection) { this.off('change', onChange) // Error was already emitted in connect - return + return null } this.backend = new PollingBackend(this, this.#connection) this.version = this.#connection.docStateVersion this.baseVersionEtag = this.#connection.document.baseVersionEtag - this.emit('opened', { - ...this.#connection.state, - version: this.version, - }) - this.emit('loaded', { - ...this.#connection.state, - version: this.version, - }) + this.emit('opened', this.connectionState) + this.emit('loaded', this.connectionState) + + return this.connectionState } startSync() { @@ -190,6 +201,7 @@ class SyncService { } return this.#connection.push(data) .then((response) => { + this.pushError = 0 this.sending = false this.emit('sync', { steps: [], @@ -199,6 +211,7 @@ class SyncService { }).catch(err => { const { response, code } = err this.sending = false + this.pushError++ if (!response || code === 'ECONNABORTED') { this.emit('error', { type: ERROR_TYPE.CONNECTION_FAILED, data: {} }) } @@ -215,6 +228,8 @@ class SyncService { this.emit('error', { type: ERROR_TYPE.PUSH_FAILURE, data: {} }) OC.Notification.showTemporary('Changes could not be sent yet') } + } else { + this.emit('error', { type: ERROR_TYPE.PUSH_FAILURE, data: {} }) } throw new Error('Failed to apply steps. Retry!', { cause: err }) }) @@ -325,7 +340,7 @@ class SyncService { // Make sure to leave no pending requests behind. this.autosave.clear() this.backend?.disconnect() - if (!this.#connection || this.#connection.isClosed) { + if (!this.hasActiveConnection) { return } return this.#connection.close() diff --git a/src/services/WebSocketPolyfill.js b/src/services/WebSocketPolyfill.js index ed5f743c198..861204dfc9d 100644 --- a/src/services/WebSocketPolyfill.js +++ b/src/services/WebSocketPolyfill.js @@ -49,10 +49,9 @@ export default function initWebSocketPolyfill(syncService, fileId, initialSessio logger.debug('WebSocketPolyfill#constructor', { url, fileId, initialSession }) this.#registerHandlers({ opened: ({ version, session }) => { - this.#version = version logger.debug('opened ', { version, session }) + this.#version = version this.#session = session - this.onopen?.() }, loaded: ({ version, session, content }) => { logger.debug('loaded ', { version, session }) @@ -70,7 +69,16 @@ export default function initWebSocketPolyfill(syncService, fileId, initialSessio } }, }) - syncService.open({ fileId, initialSession }) + + syncService.open({ fileId, initialSession }).then((data) => { + if (syncService.hasActiveConnection) { + const { version, session } = data + this.#version = version + this.#session = session + + this.onopen?.() + } + }) } #registerHandlers(handlers) { @@ -101,7 +109,10 @@ export default function initWebSocketPolyfill(syncService, fileId, initialSessio ...queue.filter(s => !outbox.includes(s)), ) return ret - }, err => logger.error(err)) + }, err => { + logger.error(`Failed to push the queue with ${queue.length} steps to the server`, err) + this.onerror?.(err) + }) } async close() { diff --git a/src/tests/services/WebsocketPolyfill.spec.js b/src/tests/services/WebsocketPolyfill.spec.js index 1536d6e6992..060bab71e67 100644 --- a/src/tests/services/WebsocketPolyfill.spec.js +++ b/src/tests/services/WebsocketPolyfill.spec.js @@ -3,21 +3,21 @@ import initWebSocketPolyfill from '../../services/WebSocketPolyfill.js' describe('Init function', () => { it('returns a websocket polyfill class', () => { - const syncService = { on: jest.fn(), open: jest.fn() } + const syncService = { on: jest.fn(), open: jest.fn(() => Promise.resolve({ version: 123, session: {} })) } const Polyfill = initWebSocketPolyfill(syncService) const websocket = new Polyfill('url') expect(websocket).toBeInstanceOf(Polyfill) }) it('registers handlers', () => { - const syncService = { on: jest.fn(), open: jest.fn() } + const syncService = { on: jest.fn(), open: jest.fn(() => Promise.resolve({ version: 123, session: {} })) } const Polyfill = initWebSocketPolyfill(syncService) const websocket = new Polyfill('url') expect(syncService.on).toHaveBeenCalled() }) it('opens sync service', () => { - const syncService = { on: jest.fn(), open: jest.fn() } + const syncService = { on: jest.fn(), open: jest.fn(() => Promise.resolve({ version: 123, session: {} })) } const fileId = 123 const initialSession = { } const Polyfill = initWebSocketPolyfill(syncService, fileId, initialSession) @@ -28,7 +28,7 @@ describe('Init function', () => { it('sends steps to sync service', async () => { const syncService = { on: jest.fn(), - open: jest.fn(), + open: jest.fn(() => Promise.resolve({ version: 123, session: {} })), sendSteps: async getData => getData(), } const queue = [ 'initial' ] @@ -46,9 +46,10 @@ describe('Init function', () => { }) it('handles early reject', async () => { + jest.spyOn(console, 'error').mockImplementation(() => {}) const syncService = { on: jest.fn(), - open: jest.fn(), + open: jest.fn(() => Promise.resolve({ version: 123, session: {} })), sendSteps: jest.fn().mockRejectedValue('error before reading steps in sync service'), } const queue = [ 'initial' ] @@ -64,9 +65,10 @@ describe('Init function', () => { }) it('handles reject after reading data', async () => { + jest.spyOn(console, 'error').mockImplementation(() => {}) const syncService = { on: jest.fn(), - open: jest.fn(), + open: jest.fn(() => Promise.resolve({ version: 123, session: {} })), sendSteps: jest.fn().mockImplementation( async getData => { getData() throw 'error when sending in sync service' @@ -85,9 +87,10 @@ describe('Init function', () => { }) it('queue survives a close', async () => { + jest.spyOn(console, 'error').mockImplementation(() => {}) const syncService = { on: jest.fn(), - open: jest.fn(), + open: jest.fn(() => Promise.resolve({ version: 123, session: {} })), sendSteps: jest.fn().mockImplementation( async getData => { getData() throw 'error when sending in sync service'