From 98765fc2051ed2130dac3e5e8eb1257bed40273d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Fri, 16 Aug 2024 15:06:56 +0200 Subject: [PATCH] fix: Ensure WebsocketPolyfill always has the latest session state and version MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- src/components/Editor.vue | 2 +- src/services/SyncService.js | 27 ++++++++++++-------- src/services/WebSocketPolyfill.js | 17 +++++++----- src/tests/services/WebsocketPolyfill.spec.js | 17 +++++++----- 4 files changed, 38 insertions(+), 25 deletions(-) diff --git a/src/components/Editor.vue b/src/components/Editor.vue index 3fe40f4da2..21ef52c8f6 100644 --- a/src/components/Editor.vue +++ b/src/components/Editor.vue @@ -566,7 +566,7 @@ export default { }, onSync({ steps, document }) { - this.hasConnectionIssue = !this.$providers[0].wsconnected || this.$syncService.pushError > 0 + 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 e272480753..92fa361b12 100644 --- a/src/services/SyncService.js +++ b/src/services/SyncService.js @@ -94,10 +94,19 @@ class SyncService { 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.hasActiveConnection) { - // We're already connected. - return + return this.connectionState } const connect = initialSession ? Promise.resolve(new Connection({ data: initialSession }, {})) @@ -106,19 +115,15 @@ class SyncService { this.#connection = await connect if (!this.#connection) { // 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() { diff --git a/src/services/WebSocketPolyfill.js b/src/services/WebSocketPolyfill.js index 474ecc89d4..0fb7b5477e 100644 --- a/src/services/WebSocketPolyfill.js +++ b/src/services/WebSocketPolyfill.js @@ -34,15 +34,11 @@ export default function initWebSocketPolyfill(syncService, fileId, initialSessio this.#notifyPushBus?.on('notify_push', this.#onNotifyPush.bind(this)) this.url = url logger.debug('WebSocketPolyfill#constructor', { url, fileId, initialSession }) - if (syncService.hasActiveConnection) { - setTimeout(() => this.onopen?.(), 0) - } 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 }) @@ -60,7 +56,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) { diff --git a/src/tests/services/WebsocketPolyfill.spec.js b/src/tests/services/WebsocketPolyfill.spec.js index 331f56b915..6eb4ad72f3 100644 --- a/src/tests/services/WebsocketPolyfill.spec.js +++ b/src/tests/services/WebsocketPolyfill.spec.js @@ -8,21 +8,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) @@ -33,7 +33,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' ] @@ -51,9 +51,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' ] @@ -69,9 +70,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' @@ -90,9 +92,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'