Merge pull request #6200 from nextcloud/fix/sync-bugs

Fix reconnecting websocket polyfill and error propagation during push
nextcloud · Aug 13, 2024 · 806bc9d · 806bc9d
2 parents 9b22e96 + e8ec6ea
commit 806bc9d
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 5 deletions.
diff --git a/src/components/Editor.vue b/src/components/Editor.vue
@@ -559,14 +559,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.$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
@@ -77,6 +77,8 @@ class SyncService {
 
 		this.autosave = debounce(this._autosave.bind(this), AUTOSAVE_INTERVAL)
 
+		this.pushError = 0
+
 		return this
 	}
 
@@ -88,8 +90,12 @@ class SyncService {
 		return this.#connection.session.guestName
 	}
 
+	get hasActiveConnection() {
+		return this.#connection && !this.#connection.isClosed
+	}
+
 	async open({ fileId, initialSession }) {
-		if (this.#connection && !this.#connection.isClosed) {
+		if (this.hasActiveConnection) {
 			// We're already connected.
 			return
 		}
@@ -166,6 +172,7 @@ class SyncService {
 		}
 		return this.#connection.push(data)
 			.then((response) => {
+				this.pushError = 0
 				this.sending = false
 				this.emit('sync', {
 					steps: [],
@@ -175,6 +182,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: {} })
 				}
@@ -191,6 +199,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 })
 			})
@@ -301,7 +311,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
@@ -34,6 +34,9 @@ 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
@@ -88,7 +91,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() {