Merge pull request #6241 from nextcloud/backport/6217/stable30

[stable30] fix: Ensure WebsocketPolyfill always has the latest session state and version

Author: juliusknorr

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')
 			})

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() {

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) {

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'