fix(sync): sending steps with interval in wrapper function

This way the calling function can catch all network errors and make sure to preserve the unsent steps in the queue. Signed-off-by: Max <[email protected]>
nextcloud · Jul 11, 2023 · 0329f46 · 0329f46
1 parent 155bbb5
commit 0329f46
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 11 deletions.
diff --git a/src/services/SyncService.js b/src/services/SyncService.js
@@ -66,6 +66,8 @@ const ERROR_TYPE = {
 
 class SyncService {
 
+	#sendIntervalId
+
 	constructor({ serialize, getDocumentState, ...options }) {
 		/** @type {import('mitt').Emitter<import('./SyncService').EventTypes>} _bus */
 		this._bus = mitt()
@@ -84,6 +86,7 @@ class SyncService {
 
 		this.version = null
 		this.sending = false
+		this.#sendIntervalId = null
 
 		this.autosave = debounce(this._autosave.bind(this), AUTOSAVE_INTERVAL)
 
@@ -147,12 +150,26 @@ class SyncService {
 	}
 
 	sendSteps(getSendable) {
-		if (!this.connection || this.sending) {
-			setTimeout(() => {
-				this.sendSteps(getSendable)
-			}, 200)
+		// If already retrying, do nothing.
+		if (this.#sendIntervalId) {
 			return
 		}
+		if (this.connection && !this.sending) {
+			return this._sendSteps(getSendable)
+		}
+		// If already sending, retry every 200ms.
+		return new Promise((resolve, reject) => {
+			this.#sendIntervalId = setInterval(() => {
+				if (this.connection && !this.sending) {
+					clearInterval(this.#sendIntervalId)
+					this.#sendIntervalId = null
+					this._sendSteps(getSendable).then(resolve).catch(reject)
+				}
+			}, 200)
+		})
+	}
+
+	_sendSteps(getSendable) {
 		this.sending = true
 		const data = getSendable()
 		if (data.steps.length > 0) {
@@ -166,7 +183,8 @@ class SyncService {
 					document: this.connection.document,
 					version: this.version,
 				})
-			}).catch(({ response, code }) => {
+			}).catch(err => {
+				const { response, code } = err
 				this.sending = false
 				if (!response || code === 'ECONNABORTED') {
 					this.emit('error', { type: ERROR_TYPE.CONNECTION_FAILED, data: {} })
@@ -182,8 +200,7 @@ class SyncService {
 						OC.Notification.showTemporary('Changes could not be sent yet')
 					}
 				}
-				logger.error('Failed to apply steps, retrying...')
-				throw('retry')
+				throw new Error('Failed to apply steps. Retry!', { cause: err })
 			})
 	}
 
@@ -194,7 +211,7 @@ class SyncService {
 			.map(s => {
 				return { step: s.lastAwarenessMessage, clientId: s.clientId }
 			})
-		const newSteps = awareness
+		const newSteps = [...awareness]
 		this.steps = [...this.steps, ...awareness.map(s => s.step)]
 		for (let i = 0; i < steps.length; i++) {
 			const singleSteps = steps[i].data

diff --git a/src/services/WebSocketPolyfill.js b/src/services/WebSocketPolyfill.js
@@ -81,9 +81,9 @@ export default function initWebSocketPolyfill(syncService, fileId, initialSessio
 
 		send(...data) {
 			this.#queue.push(...data)
-			let outbox
+			let outbox = []
 			syncService.sendSteps(() => {
-				outbox = this.#queue
+				outbox = [...this.#queue]
 				const data = {
 					steps: this.#steps,
 					awareness: this.#awareness,
@@ -92,7 +92,8 @@ export default function initWebSocketPolyfill(syncService, fileId, initialSessio
 				this.#queue = []
 				logger.debug('sending steps ', data)
 				return data
-			})?.catch(() => {
+			})?.catch(err => {
+				logger.error(err)
 				// try to send the steps again
 				this.#queue = [...outbox, ...this.#queue]
 			})