From d5b9b56834e1849f2ffcf8e3ca20ab6299e6f6c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Mon, 12 Aug 2024 11:04:19 +0200 Subject: [PATCH 1/4] fix: Track push errors and do not reset error state if push fails but sync passes 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 | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/components/Editor.vue b/src/components/Editor.vue index e3147161a59..0174f9d3493 100644 --- a/src/components/Editor.vue +++ b/src/components/Editor.vue @@ -566,7 +566,7 @@ export default { }, 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 index 4e7e74acc1f..2fa7ac317cb 100644 --- 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 } @@ -166,6 +168,7 @@ class SyncService { } return this.#connection.push(data) .then((response) => { + this.pushError = 0 this.sending = false this.emit('sync', { steps: [], @@ -175,6 +178,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: {} }) } From 3d3246ead010e321b6dcbea60cc1b82f4753a97c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Mon, 12 Aug 2024 11:05:52 +0200 Subject: [PATCH 2/4] fix: Do not reset read only state if sync errors occurred 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 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/Editor.vue b/src/components/Editor.vue index 0174f9d3493..3fe40f4da2c 100644 --- a/src/components/Editor.vue +++ b/src/components/Editor.vue @@ -559,7 +559,7 @@ 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) } From f8b87a7b9aa28df95ceb940e5baea02e03e9d6d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Mon, 12 Aug 2024 11:09:41 +0200 Subject: [PATCH 3/4] fix: emit onerror for websocket and reopen connection on reconnect attempts of y-websocket MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- src/services/SyncService.js | 8 ++++++-- src/services/WebSocketPolyfill.js | 8 +++++++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/services/SyncService.js b/src/services/SyncService.js index 2fa7ac317cb..cb4b4eb4d81 100644 --- a/src/services/SyncService.js +++ b/src/services/SyncService.js @@ -90,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 } @@ -305,7 +309,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 df5a28c25b6..474ecc89d41 100644 --- 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() { From e8ec6ea7ab33c1969c97de5d8a8d6a206bf144bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Mon, 12 Aug 2024 11:10:23 +0200 Subject: [PATCH 4/4] fix: Emit error if push fails in other unhandled cases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- src/services/SyncService.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/services/SyncService.js b/src/services/SyncService.js index cb4b4eb4d81..e2724807532 100644 --- a/src/services/SyncService.js +++ b/src/services/SyncService.js @@ -199,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 }) })