From 6b5213e7f3c29ded95eb63c07ed1e56ed32c3a99 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Wed, 23 Dec 2020 06:38:58 -1000 Subject: [PATCH 1/8] Fix various post sign out requests + bugs --- src/libs/API.js | 6 ++++-- src/libs/Network.js | 8 ++++++++ src/libs/NetworkConnection.js | 2 ++ src/libs/actions/Report.js | 2 +- src/libs/actions/Session.js | 1 + src/pages/home/report/ReportActionsView.js | 7 +++++-- 6 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/libs/API.js b/src/libs/API.js index aeab7db04d120..db650d9e73588 100644 --- a/src/libs/API.js +++ b/src/libs/API.js @@ -53,9 +53,11 @@ function addAuthTokenToParameters(command, parameters) { // cancel this and all other requests and set isAuthenticating to false. if (!authToken) { console.debug('A request was made without an authToken', {command, parameters}); - Network.unpauseRequestQueue(); + Network.clearRequestQueue(); redirectToSignIn(); - return; + + // Throw so processRequestQueue stops making requests + throw new Error('A request was made without an authToken'); } finalParameters.authToken = authToken; diff --git a/src/libs/Network.js b/src/libs/Network.js index 0f56958771de2..c61e8e199e6b2 100644 --- a/src/libs/Network.js +++ b/src/libs/Network.js @@ -114,9 +114,17 @@ function registerParameterEnhancer(callback) { enhanceParameters = callback; } +/** + * Clears the request queue + */ +function clearRequestQueue() { + networkRequestQueue = []; +} + export { post, pauseRequestQueue, unpauseRequestQueue, registerParameterEnhancer, + clearRequestQueue, }; diff --git a/src/libs/NetworkConnection.js b/src/libs/NetworkConnection.js index a2c26fb772894..2b57342b6b5c9 100644 --- a/src/libs/NetworkConnection.js +++ b/src/libs/NetworkConnection.js @@ -97,9 +97,11 @@ function stopListeningForReconnect() { clearInterval(sleepTimer); if (unsubscribeFromNetInfo) { unsubscribeFromNetInfo(); + unsubscribeFromNetInfo = undefined; } if (isListeningToAppStateChanges) { AppState.removeEventListener('change', setAppState); + isListeningToAppStateChanges = false; } } diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 78d9bceff5867..d4db7092e432d 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -610,7 +610,7 @@ function handleReportChanged(report) { // A report can be missing a name if a comment is received via pusher event // and the report does not yet exist in Onyx (eg. a new DM created with the logged in person) - if (report.reportName === undefined) { + if (report.reportID && report.reportName === undefined) { fetchChatReportsByIDs([report.reportID]); } diff --git a/src/libs/actions/Session.js b/src/libs/actions/Session.js index f853f87205002..149dfa6e1bae9 100644 --- a/src/libs/actions/Session.js +++ b/src/libs/actions/Session.js @@ -141,6 +141,7 @@ function signIn(password, exitTo, twoFactorAuthCode) { // If we have an old login for some reason, we should delete it before storing the new details if (credentials.login) { API.DeleteLogin({ + authToken: authenticateResponse.authToken, partnerUserID: credentials.login, partnerName: CONFIG.EXPENSIFY.PARTNER_NAME, partnerPassword: CONFIG.EXPENSIFY.PARTNER_PASSWORD, diff --git a/src/pages/home/report/ReportActionsView.js b/src/pages/home/report/ReportActionsView.js index ebe14e439e534..201aeb03263b2 100644 --- a/src/pages/home/report/ReportActionsView.js +++ b/src/pages/home/report/ReportActionsView.js @@ -46,6 +46,7 @@ class ReportActionsView extends React.Component { this.scrollToListBottom = this.scrollToListBottom.bind(this); this.recordMaxAction = this.recordMaxAction.bind(this); this.sortedReportActions = this.updateSortedReportActions(); + this.timers = []; this.state = { refetchNeeded: true, @@ -55,7 +56,7 @@ class ReportActionsView extends React.Component { componentDidMount() { this.visibilityChangeEvent = AppState.addEventListener('change', () => { if (this.props.isActiveReport && Visibility.isVisible()) { - setTimeout(this.recordMaxAction, 3000); + this.timers.push(setTimeout(this.recordMaxAction, 3000)); } }); if (this.props.isActiveReport) { @@ -86,7 +87,7 @@ class ReportActionsView extends React.Component { // When the number of actions change, wait three seconds, then record the max action // This will make the unread indicator go away if you receive comments in the same chat you're looking at if (this.props.isActiveReport && Visibility.isVisible()) { - setTimeout(this.recordMaxAction, 3000); + this.timers.push(setTimeout(this.recordMaxAction, 3000)); } return; @@ -112,6 +113,8 @@ class ReportActionsView extends React.Component { if (this.visibilityChangeEvent) { this.visibilityChangeEvent.remove(); } + + _.each(this.timers, clearTimeout); } /** From 218d0c21302757b7d44e904331b951a26b16f0f4 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Wed, 23 Dec 2020 06:49:57 -1000 Subject: [PATCH 2/8] clear timers --- src/libs/Network.js | 14 +++++++++++--- src/pages/home/report/ReportActionsView.js | 3 ++- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/libs/Network.js b/src/libs/Network.js index c61e8e199e6b2..5766060108c53 100644 --- a/src/libs/Network.js +++ b/src/libs/Network.js @@ -50,9 +50,17 @@ function processNetworkRequestQueue() { return; } - const finalParameters = _.isFunction(enhanceParameters) - ? enhanceParameters(queuedRequest.command, queuedRequest.data) - : queuedRequest.data; + let finalParameters; + + try { + finalParameters = _.isFunction(enhanceParameters) + ? enhanceParameters(queuedRequest.command, queuedRequest.data) + : queuedRequest.data; + } catch (err) { + // Something went wrong when enhancing parameters assume we should not + // make this request at all. + return; + } HttpUtils.xhr(queuedRequest.command, finalParameters, queuedRequest.type) .then(queuedRequest.resolve) diff --git a/src/pages/home/report/ReportActionsView.js b/src/pages/home/report/ReportActionsView.js index 201aeb03263b2..9887d717732fc 100644 --- a/src/pages/home/report/ReportActionsView.js +++ b/src/pages/home/report/ReportActionsView.js @@ -110,11 +110,12 @@ class ReportActionsView extends React.Component { if (this.keyboardEvent) { this.keyboardEvent.remove(); } + if (this.visibilityChangeEvent) { this.visibilityChangeEvent.remove(); } - _.each(this.timers, clearTimeout); + _.each(this.timers, timer => clearTimeout(timer)); } /** From a852fef28f9fbf54c98505991374961628502363 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Wed, 23 Dec 2020 06:51:57 -1000 Subject: [PATCH 3/8] fix comment --- src/libs/API.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/API.js b/src/libs/API.js index db650d9e73588..789bb9d7b1741 100644 --- a/src/libs/API.js +++ b/src/libs/API.js @@ -56,7 +56,7 @@ function addAuthTokenToParameters(command, parameters) { Network.clearRequestQueue(); redirectToSignIn(); - // Throw so processRequestQueue stops making requests + // Throw so the request queue knows to not make this request throw new Error('A request was made without an authToken'); } From 2b2bb4a7f3775ce78034c6b47e5492c45964e1e4 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Wed, 23 Dec 2020 06:54:48 -1000 Subject: [PATCH 4/8] remove clear queue method --- src/libs/API.js | 4 ++-- src/libs/Network.js | 11 ++--------- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/src/libs/API.js b/src/libs/API.js index 789bb9d7b1741..bed496e8e8cd1 100644 --- a/src/libs/API.js +++ b/src/libs/API.js @@ -52,10 +52,10 @@ function addAuthTokenToParameters(command, parameters) { // an API request before we are signed in. In this case, we should just // cancel this and all other requests and set isAuthenticating to false. if (!authToken) { - console.debug('A request was made without an authToken', {command, parameters}); - Network.clearRequestQueue(); redirectToSignIn(); + console.debug('A request was made without an authToken', {command, parameters}); + // Throw so the request queue knows to not make this request throw new Error('A request was made without an authToken'); } diff --git a/src/libs/Network.js b/src/libs/Network.js index 5766060108c53..30dcd07688508 100644 --- a/src/libs/Network.js +++ b/src/libs/Network.js @@ -58,7 +58,8 @@ function processNetworkRequestQueue() { : queuedRequest.data; } catch (err) { // Something went wrong when enhancing parameters assume we should not - // make this request at all. + // make this request at all and clear the request queue + networkRequestQueue = []; return; } @@ -122,17 +123,9 @@ function registerParameterEnhancer(callback) { enhanceParameters = callback; } -/** - * Clears the request queue - */ -function clearRequestQueue() { - networkRequestQueue = []; -} - export { post, pauseRequestQueue, unpauseRequestQueue, registerParameterEnhancer, - clearRequestQueue, }; From ce167cbdc36046f3323e3cade378b44a12de2ba8 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Mon, 28 Dec 2020 14:38:24 -1000 Subject: [PATCH 5/8] Add clearRequestQueue() and clear the queue when there is no authToken --- src/libs/API.js | 6 +++--- src/libs/Network.js | 25 +++++++++++++++---------- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/src/libs/API.js b/src/libs/API.js index 959655123078a..f4b063d6f2277 100644 --- a/src/libs/API.js +++ b/src/libs/API.js @@ -55,9 +55,9 @@ function addAuthTokenToParameters(command, parameters) { redirectToSignIn(); console.debug('A request was made without an authToken', {command, parameters}); - - // Throw so the request queue knows to not make this request - throw new Error('A request was made without an authToken'); + Network.pauseRequestQueue(); + Network.clearRequestQueue(); + return; } finalParameters.authToken = authToken; diff --git a/src/libs/Network.js b/src/libs/Network.js index 30dcd07688508..37f6a9e6f78be 100644 --- a/src/libs/Network.js +++ b/src/libs/Network.js @@ -50,16 +50,13 @@ function processNetworkRequestQueue() { return; } - let finalParameters; - - try { - finalParameters = _.isFunction(enhanceParameters) - ? enhanceParameters(queuedRequest.command, queuedRequest.data) - : queuedRequest.data; - } catch (err) { - // Something went wrong when enhancing parameters assume we should not - // make this request at all and clear the request queue - networkRequestQueue = []; + const finalParameters = _.isFunction(enhanceParameters) + ? enhanceParameters(queuedRequest.command, queuedRequest.data) + : queuedRequest.data; + + // Check to see if the queue has paused again. It's possible that a call to enhanceParameters() + // has paused the queue and if this is the case we must return. + if (isQueuePaused) { return; } @@ -123,9 +120,17 @@ function registerParameterEnhancer(callback) { enhanceParameters = callback; } +/** + * Clear the queue so all pending requests will be cancelled + */ +function clearRequestQueue() { + networkRequestQueue = []; +} + export { post, pauseRequestQueue, unpauseRequestQueue, registerParameterEnhancer, + clearRequestQueue, }; From 82e21d0a44714d547f9ffac0d182475aab1e75f3 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Tue, 29 Dec 2020 08:50:12 -1000 Subject: [PATCH 6/8] Fix up comment --- src/libs/API.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/libs/API.js b/src/libs/API.js index f4b063d6f2277..f3be59a88ed64 100644 --- a/src/libs/API.js +++ b/src/libs/API.js @@ -48,9 +48,8 @@ function addAuthTokenToParameters(command, parameters) { const finalParameters = {...parameters}; if (isAuthTokenRequired(command) && !parameters.authToken) { - // If we end up here with no authToken it means we are trying to make - // an API request before we are signed in. In this case, we should just - // cancel this and all other requests and set isAuthenticating to false. + // If we end up here with no authToken it means we are trying to make an API request before we are signed + // in. In this case, we should cancel the current request and clear the request queue. if (!authToken) { redirectToSignIn(); From 9f08e551b57572691fe150115b4ae38e9f6ab4a7 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Tue, 29 Dec 2020 08:50:50 -1000 Subject: [PATCH 7/8] fix comment again --- src/libs/API.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/API.js b/src/libs/API.js index f3be59a88ed64..1c64993685fbc 100644 --- a/src/libs/API.js +++ b/src/libs/API.js @@ -49,7 +49,7 @@ function addAuthTokenToParameters(command, parameters) { if (isAuthTokenRequired(command) && !parameters.authToken) { // If we end up here with no authToken it means we are trying to make an API request before we are signed - // in. In this case, we should cancel the current request and clear the request queue. + // in. In this case, we should cancel the current request by pausing the queue and clear the remaining requests. if (!authToken) { redirectToSignIn(); From e97a94771388ea21e87758a6952e1e73ec5b52cb Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Tue, 29 Dec 2020 08:51:15 -1000 Subject: [PATCH 8/8] last one --- src/libs/API.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libs/API.js b/src/libs/API.js index 1c64993685fbc..f7ce7d5c3fb3f 100644 --- a/src/libs/API.js +++ b/src/libs/API.js @@ -48,8 +48,8 @@ function addAuthTokenToParameters(command, parameters) { const finalParameters = {...parameters}; if (isAuthTokenRequired(command) && !parameters.authToken) { - // If we end up here with no authToken it means we are trying to make an API request before we are signed - // in. In this case, we should cancel the current request by pausing the queue and clear the remaining requests. + // If we end up here with no authToken it means we are trying to make an API request before we are signed in. + // In this case, we should cancel the current request by pausing the queue and clearing the remaining requests. if (!authToken) { redirectToSignIn();