From 90afcd3420a911856bde794f2b4dc1380a1a0751 Mon Sep 17 00:00:00 2001 From: Henry Jameson Date: Mon, 8 Mar 2021 22:24:39 +0200 Subject: [PATCH 1/2] WIP some work on making errors less spammy --- src/i18n/en.json | 3 +- src/modules/api.js | 44 +++++++++++-------- .../notifications_fetcher.service.js | 14 +++--- .../timeline_fetcher.service.js | 14 +++--- 4 files changed, 43 insertions(+), 32 deletions(-) diff --git a/src/i18n/en.json b/src/i18n/en.json index 217ac7c85..635bfd565 100644 --- a/src/i18n/en.json +++ b/src/i18n/en.json @@ -664,8 +664,7 @@ "no_more_statuses": "No more statuses", "no_statuses": "No statuses", "socket_reconnected": "Realtime connection established", - "socket_broke": "Realtime connection lost: CloseEvent code {0}", - "socket_closed": "No realtime connection, updates can arrive with a delaye until connection is re-established" + "socket_broke": "Realtime connection lost: CloseEvent code {0}" }, "status": { "favorites": "Favorites", diff --git a/src/modules/api.js b/src/modules/api.js index b482637d6..e392d5b0b 100644 --- a/src/modules/api.js +++ b/src/modules/api.js @@ -5,6 +5,8 @@ import { Socket } from 'phoenix' const api = { state: { + connectionBroken: false, + retryMultiplier: 1, backendInteractor: backendInteractorService(), fetchers: {}, socket: null, @@ -34,12 +36,18 @@ const api = { }, setMastoUserSocketStatus (state, value) { state.mastoUserSocketStatus = value + }, + recoverConnection (state) { + state.connectionBroken = false + }, + breakConnection (state) { + state.connectionBroken = true } }, actions: { // Global MastoAPI socket control, in future should disable ALL sockets/(re)start relevant sockets enableMastoSockets (store) { - const { state, dispatch } = store + const { state, dispatch, commit } = store // Do not initialize unless nonexistent or closed if ( state.mastoUserSocket && @@ -50,11 +58,13 @@ const api = { ) { return } + commit('recoverConnection') return dispatch('startMastoUserSocket') }, disableMastoSockets (store) { - const { state, dispatch } = store + const { state, dispatch, commit } = store if (!state.mastoUserSocket) return + commit('recoverConnection') return dispatch('stopMastoUserSocket') }, @@ -102,6 +112,7 @@ const api = { state.mastoUserSocket.addEventListener('open', () => { // Do not show notification when we just opened up the page if (state.mastoUserSocketStatus !== null) { + commit('recoverConnection') dispatch('pushGlobalNotice', { level: 'success', messageKey: 'timeline.socket_reconnected', @@ -114,15 +125,6 @@ const api = { console.error('Error in MastoAPI websocket:', error) commit('setMastoUserSocketStatus', WSConnectionStatus.ERROR) dispatch('clearOpenedChats') - /* Since data in WS event for error is useless it's better to show - * generic warning instead of in "close" which actually has some - * useful data - */ - dispatch('pushGlobalNotice', { - level: 'error', - messageKey: 'timeline.socket_closed', - timeout: 5000 - }) }) state.mastoUserSocket.addEventListener('close', ({ detail: closeEvent }) => { const ignoreCodes = new Set([ @@ -137,13 +139,19 @@ const api = { dispatch('startFetchingTimeline', { timeline: 'friends' }) dispatch('startFetchingNotifications') dispatch('startFetchingChats') - dispatch('restartMastoUserSocket') - dispatch('pushGlobalNotice', { - level: 'error', - messageKey: 'timeline.socket_broke', - messageArgs: [code], - timeout: 5000 - }) + setTimeout(() => { + console.log('TEST') + dispatch('restartMastoUserSocket') + }, 1000) + if (!state.connectionBroken) { + dispatch('pushGlobalNotice', { + level: 'error', + messageKey: 'timeline.socket_broke', + messageArgs: [code], + timeout: 5000 + }) + } + commit('breakConnection') } commit('setMastoUserSocketStatus', WSConnectionStatus.CLOSED) dispatch('clearOpenedChats') diff --git a/src/services/notifications_fetcher/notifications_fetcher.service.js b/src/services/notifications_fetcher/notifications_fetcher.service.js index f83f871e0..fc5e76ba2 100644 --- a/src/services/notifications_fetcher/notifications_fetcher.service.js +++ b/src/services/notifications_fetcher/notifications_fetcher.service.js @@ -57,12 +57,14 @@ const fetchNotifications = ({ store, args, older }) => { return notifications }) .catch((error) => { - store.dispatch('pushGlobalNotice', { - level: 'error', - messageKey: 'notifications.error', - messageArgs: [error.message], - timeout: 5000 - }) + if (!store.rootState.api.connectionBroken) { + store.dispatch('pushGlobalNotice', { + level: 'error', + messageKey: 'notifications.error', + messageArgs: [error.message], + timeout: 5000 + }) + } }) } diff --git a/src/services/timeline_fetcher/timeline_fetcher.service.js b/src/services/timeline_fetcher/timeline_fetcher.service.js index 46bba41a0..eb4a3e0da 100644 --- a/src/services/timeline_fetcher/timeline_fetcher.service.js +++ b/src/services/timeline_fetcher/timeline_fetcher.service.js @@ -66,12 +66,14 @@ const fetchAndUpdate = ({ return { statuses, pagination } }) .catch((error) => { - store.dispatch('pushGlobalNotice', { - level: 'error', - messageKey: 'timeline.error', - messageArgs: [error.message], - timeout: 5000 - }) + if (!store.rootState.api.connectionBroken) { + store.dispatch('pushGlobalNotice', { + level: 'error', + messageKey: 'timeline.error', + messageArgs: [error.message], + timeout: 5000 + }) + } }) } From a8967d85bd7ae1541f1d4e41e95fc8fa111f2360 Mon Sep 17 00:00:00 2001 From: Henry Jameson Date: Tue, 9 Mar 2021 02:38:10 +0200 Subject: [PATCH 2/2] streamlined WS flow, reduced spam amount related to WS reconnections --- src/modules/api.js | 69 +++++++++++-------- src/modules/users.js | 3 +- src/services/api/api.service.js | 5 +- .../notifications_fetcher.service.js | 14 ++-- .../timeline_fetcher.service.js | 14 ++-- 5 files changed, 56 insertions(+), 49 deletions(-) diff --git a/src/modules/api.js b/src/modules/api.js index e392d5b0b..8654b90a5 100644 --- a/src/modules/api.js +++ b/src/modules/api.js @@ -3,9 +3,10 @@ import { WSConnectionStatus } from '../services/api/api.service.js' import { maybeShowChatNotification } from '../services/chat_utils/chat_utils.js' import { Socket } from 'phoenix' +const retryTimeout = (multiplier) => 1000 * multiplier + const api = { state: { - connectionBroken: false, retryMultiplier: 1, backendInteractor: backendInteractorService(), fetchers: {}, @@ -37,16 +38,20 @@ const api = { setMastoUserSocketStatus (state, value) { state.mastoUserSocketStatus = value }, - recoverConnection (state) { - state.connectionBroken = false + incrementRetryMultiplier (state) { + state.retryMultiplier = Math.max(++state.retryMultiplier, 3) }, - breakConnection (state) { - state.connectionBroken = true + resetRetryMultiplier (state) { + state.retryMultiplier = 1 } }, actions: { - // Global MastoAPI socket control, in future should disable ALL sockets/(re)start relevant sockets - enableMastoSockets (store) { + /** + * Global MastoAPI socket control, in future should disable ALL sockets/(re)start relevant sockets + * + * @param {Boolean} [initial] - whether this enabling happened at boot time or not + */ + enableMastoSockets (store, initial) { const { state, dispatch, commit } = store // Do not initialize unless nonexistent or closed if ( @@ -58,13 +63,17 @@ const api = { ) { return } - commit('recoverConnection') + if (initial) { + commit('setMastoUserSocketStatus', WSConnectionStatus.STARTING_INITIAL) + } else { + commit('setMastoUserSocketStatus', WSConnectionStatus.STARTING) + } return dispatch('startMastoUserSocket') }, disableMastoSockets (store) { const { state, dispatch, commit } = store if (!state.mastoUserSocket) return - commit('recoverConnection') + commit('setMastoUserSocketStatus', WSConnectionStatus.DISABLED) return dispatch('stopMastoUserSocket') }, @@ -111,19 +120,28 @@ const api = { ) state.mastoUserSocket.addEventListener('open', () => { // Do not show notification when we just opened up the page - if (state.mastoUserSocketStatus !== null) { - commit('recoverConnection') + if (state.mastoUserSocketStatus !== WSConnectionStatus.STARTING_INITIAL) { dispatch('pushGlobalNotice', { level: 'success', messageKey: 'timeline.socket_reconnected', timeout: 5000 }) } + // Stop polling if we were errored or disabled + if (new Set([ + WSConnectionStatus.ERROR, + WSConnectionStatus.DISABLED + ]).has(state.mastoUserSocketStatus)) { + dispatch('stopFetchingTimeline', { timeline: 'friends' }) + dispatch('stopFetchingNotifications') + dispatch('stopFetchingChats') + } + commit('resetRetryMultiplier') commit('setMastoUserSocketStatus', WSConnectionStatus.JOINED) }) state.mastoUserSocket.addEventListener('error', ({ detail: error }) => { console.error('Error in MastoAPI websocket:', error) - commit('setMastoUserSocketStatus', WSConnectionStatus.ERROR) + // TODO is this needed? dispatch('clearOpenedChats') }) state.mastoUserSocket.addEventListener('close', ({ detail: closeEvent }) => { @@ -134,16 +152,17 @@ const api = { const { code } = closeEvent if (ignoreCodes.has(code)) { console.debug(`Not restarting socket becasue of closure code ${code} is in ignore list`) + commit('setMastoUserSocketStatus', WSConnectionStatus.CLOSED) } else { console.warn(`MastoAPI websocket disconnected, restarting. CloseEvent code: ${code}`) - dispatch('startFetchingTimeline', { timeline: 'friends' }) - dispatch('startFetchingNotifications') - dispatch('startFetchingChats') setTimeout(() => { - console.log('TEST') - dispatch('restartMastoUserSocket') - }, 1000) - if (!state.connectionBroken) { + dispatch('startMastoUserSocket') + }, retryTimeout(state.retryMultiplier)) + commit('incrementRetryMultiplier') + if (state.mastoUserSocketStatus !== WSConnectionStatus.ERROR) { + dispatch('startFetchingTimeline', { timeline: 'friends' }) + dispatch('startFetchingNotifications') + dispatch('startFetchingChats') dispatch('pushGlobalNotice', { level: 'error', messageKey: 'timeline.socket_broke', @@ -151,9 +170,8 @@ const api = { timeout: 5000 }) } - commit('breakConnection') + commit('setMastoUserSocketStatus', WSConnectionStatus.ERROR) } - commit('setMastoUserSocketStatus', WSConnectionStatus.CLOSED) dispatch('clearOpenedChats') }) resolve() @@ -162,15 +180,6 @@ const api = { } }) }, - restartMastoUserSocket ({ dispatch }) { - // This basically starts MastoAPI user socket and stops conventional - // fetchers when connection reestablished - return dispatch('startMastoUserSocket').then(() => { - dispatch('stopFetchingTimeline', { timeline: 'friends' }) - dispatch('stopFetchingNotifications') - dispatch('stopFetchingChats') - }) - }, stopMastoUserSocket ({ state, dispatch }) { dispatch('startFetchingTimeline', { timeline: 'friends' }) dispatch('startFetchingNotifications') diff --git a/src/modules/users.js b/src/modules/users.js index ac52be1fe..8a764a160 100644 --- a/src/modules/users.js +++ b/src/modules/users.js @@ -549,9 +549,8 @@ const users = { if (store.getters.mergedConfig.useStreamingApi) { store.dispatch('fetchTimeline', 'friends', { since: null }) store.dispatch('fetchNotifications', { since: null }) - store.dispatch('enableMastoSockets').catch((error) => { + store.dispatch('enableMastoSockets', true).catch((error) => { console.error('Failed initializing MastoAPI Streaming socket', error) - startPolling() }).then(() => { store.dispatch('fetchChats', { latest: true }) setTimeout(() => store.dispatch('setNotificationsSilence', false), 10000) diff --git a/src/services/api/api.service.js b/src/services/api/api.service.js index d3d5c68d9..436b8b0a0 100644 --- a/src/services/api/api.service.js +++ b/src/services/api/api.service.js @@ -1184,7 +1184,10 @@ export const handleMastoWS = (wsEvent) => { export const WSConnectionStatus = Object.freeze({ 'JOINED': 1, 'CLOSED': 2, - 'ERROR': 3 + 'ERROR': 3, + 'DISABLED': 4, + 'STARTING': 5, + 'STARTING_INITIAL': 6 }) const chats = ({ credentials }) => { diff --git a/src/services/notifications_fetcher/notifications_fetcher.service.js b/src/services/notifications_fetcher/notifications_fetcher.service.js index fc5e76ba2..f83f871e0 100644 --- a/src/services/notifications_fetcher/notifications_fetcher.service.js +++ b/src/services/notifications_fetcher/notifications_fetcher.service.js @@ -57,14 +57,12 @@ const fetchNotifications = ({ store, args, older }) => { return notifications }) .catch((error) => { - if (!store.rootState.api.connectionBroken) { - store.dispatch('pushGlobalNotice', { - level: 'error', - messageKey: 'notifications.error', - messageArgs: [error.message], - timeout: 5000 - }) - } + store.dispatch('pushGlobalNotice', { + level: 'error', + messageKey: 'notifications.error', + messageArgs: [error.message], + timeout: 5000 + }) }) } diff --git a/src/services/timeline_fetcher/timeline_fetcher.service.js b/src/services/timeline_fetcher/timeline_fetcher.service.js index eb4a3e0da..46bba41a0 100644 --- a/src/services/timeline_fetcher/timeline_fetcher.service.js +++ b/src/services/timeline_fetcher/timeline_fetcher.service.js @@ -66,14 +66,12 @@ const fetchAndUpdate = ({ return { statuses, pagination } }) .catch((error) => { - if (!store.rootState.api.connectionBroken) { - store.dispatch('pushGlobalNotice', { - level: 'error', - messageKey: 'timeline.error', - messageArgs: [error.message], - timeout: 5000 - }) - } + store.dispatch('pushGlobalNotice', { + level: 'error', + messageKey: 'timeline.error', + messageArgs: [error.message], + timeout: 5000 + }) }) }