Refactor account data cache retry and delay logic

4 files changed, 66 insertions, 93 deletions

diff --git a/gui/src/main/account-data-cache.ts b/gui/src/main/account-data-cache.ts
index f4864743a8..ac829ca0f3 100644
--- a/gui/src/main/account-data-cache.ts
+++ b/gui/src/main/account-data-cache.ts
@@ -2,16 +2,14 @@ import log from 'electron-log';
 import moment from 'moment';
 import { AccountToken, IAccountData } from '../shared/daemon-rpc-types';
 import consumePromise from '../shared/promise';
+import { Scheduler } from '../shared/scheduler';
+import { InvalidAccountError } from './errors';
 
 const EXPIRED_ACCOUNT_REFRESH_PERIOD = 60_000;
 
-export enum AccountFetchRetryAction {
-  stop,
-  retry,
-}
 interface IAccountFetchWatcher {
   onFinish: () => void;
-  onError: (error: Error) => AccountFetchRetryAction;
+  onError: (error: Error) => void;
 }
 
 // An account data cache that helps to throttle RPC requests to get_account_data and retain the
@@ -20,8 +18,8 @@ export default class AccountDataCache {
   private currentAccount?: AccountToken;
   private expiresAt?: Date;
   private performingFetch = false;
-  private fetchAttempt = 0;
-  private fetchRetryTimeout?: NodeJS.Timeout;
+  private waitStrategy = new WaitStrategy();
+  private fetchRetryScheduler = new Scheduler();
   private watchers: IAccountFetchWatcher[] = [];
 
   constructor(
@@ -42,9 +40,9 @@ export default class AccountDataCache {
         this.watchers.push(watcher);
       }
 
-      this.clearFetchRetryTimeout();
+      this.fetchRetryScheduler.cancel();
       // If a scheduled retry is cancelled the fetchAttempt shouldn't be increased.
-      this.fetchAttempt = Math.max(0, this.fetchAttempt - 1);
+      this.waitStrategy.decrease();
 
       // Only fetch if there's no fetch for this account number in progress.
       if (!this.performingFetch) {
@@ -56,8 +54,8 @@ export default class AccountDataCache {
   }
 
   public invalidate() {
-    this.clearFetchRetryTimeout();
-    this.fetchAttempt = 0;
+    this.fetchRetryScheduler.cancel();
+    this.waitStrategy.reset();
 
     this.performingFetch = false;
     this.expiresAt = undefined;
@@ -67,13 +65,6 @@ export default class AccountDataCache {
     });
   }
 
-  private clearFetchRetryTimeout() {
-    if (this.fetchRetryTimeout) {
-      clearTimeout(this.fetchRetryTimeout);
-      this.fetchRetryTimeout = undefined;
-    }
-  }
-
   private setValue(value: IAccountData) {
     this.expiresAt = new Date(Date.now() + 60 * 1000); // 60s expiration
     this.updateHandler(value);
@@ -95,6 +86,7 @@ export default class AccountDataCache {
       if (this.currentAccount === accountToken) {
         this.setValue(accountData);
         this.scheduleRefetchIfExpired(accountToken, accountData);
+        this.waitStrategy.reset();
         this.performingFetch = false;
       }
     } catch (error) {
@@ -113,24 +105,15 @@ export default class AccountDataCache {
   }
 
   private handleFetchError(accountToken: AccountToken, error: Error) {
-    let shouldRetry = true;
-
-    this.notifyWatchers((watcher) => {
-      if (watcher.onError(error) === AccountFetchRetryAction.stop) {
-        shouldRetry = false;
-      }
-    });
-
-    if (shouldRetry) {
+    this.notifyWatchers((w) => w.onError(error));
+    if (!(error instanceof InvalidAccountError)) {
       this.scheduleRetry(accountToken);
     }
   }
 
   private scheduleRetry(accountToken: AccountToken) {
-    this.fetchAttempt += 1;
-
-    // Max delay: 2^11 = 2048
-    const delay = Math.pow(2, Math.min(this.fetchAttempt + 2, 11)) * 1000;
+    this.waitStrategy.increase();
+    const delay = this.waitStrategy.delay();
 
     log.warn(`Failed to fetch account data. Retrying in ${delay} ms`);
 
@@ -138,8 +121,7 @@ export default class AccountDataCache {
   }
 
   private scheduleFetch(accountToken: AccountToken, delay: number) {
-    this.fetchRetryTimeout = global.setTimeout(() => {
-      this.fetchRetryTimeout = undefined;
+    this.fetchRetryScheduler.schedule(() => {
       consumePromise(this.performFetch(accountToken));
     }, delay);
   }
@@ -148,3 +130,29 @@ export default class AccountDataCache {
     this.watchers.splice(0).forEach(notify);
   }
 }
+
+const MAX_ATTEMPT = 9;
+
+class WaitStrategy {
+  private counter = 0;
+
+  public increase() {
+    if (this.counter < MAX_ATTEMPT) {
+      this.counter += 1;
+    }
+  }
+  public decrease() {
+    if (this.counter > 0) {
+      this.counter -= 1;
+    }
+  }
+
+  public reset() {
+    this.counter = 0;
+  }
+
+  public delay(): number {
+    // Max delay: 2^11 = 2048
+    return Math.pow(2, this.counter + 2) * 1000;
+  }
+}
diff --git a/gui/src/main/index.ts b/gui/src/main/index.ts
index 61b689456c..6f9185688a 100644
--- a/gui/src/main/index.ts
+++ b/gui/src/main/index.ts
@@ -36,7 +36,7 @@ import {
   setupLogging,
 } from '../shared/logging';
 import consumePromise from '../shared/promise';
-import AccountDataCache, { AccountFetchRetryAction } from './account-data-cache';
+import AccountDataCache from './account-data-cache';
 import { getOpenAtLogin, setOpenAtLogin } from './autostart';
 import {
   ConnectionObserver,
@@ -1139,13 +1139,11 @@ class ApplicationMain {
       this.accountDataCache.invalidate();
       this.accountDataCache.fetch(accountToken, {
         onFinish: () => resolve({ status: 'verified' }),
-        onError: (error): AccountFetchRetryAction => {
+        onError: (error) => {
           if (error instanceof InvalidAccountError) {
             reject(error);
-            return AccountFetchRetryAction.stop;
           } else {
             resolve({ status: 'deferred', error });
-            return AccountFetchRetryAction.retry;
           }
         },
       });
diff --git a/gui/src/shared/scheduler.ts b/gui/src/shared/scheduler.ts
new file mode 100644
index 0000000000..af28a7e95b
--- /dev/null
+++ b/gui/src/shared/scheduler.ts
@@ -0,0 +1,12 @@
+export class Scheduler {
+  private timer?: number;
+
+  public schedule(action: () => void, delay: number) {
+    this.cancel();
+    this.timer = setTimeout(action, delay);
+  }
+
+  public cancel() {
+    clearTimeout(this.timer);
+  }
+}
diff --git a/gui/test/account-data-cache.spec.ts b/gui/test/account-data-cache.spec.ts
index 6839e7a1eb..bec1c6ef91 100644
--- a/gui/test/account-data-cache.spec.ts
+++ b/gui/test/account-data-cache.spec.ts
@@ -1,4 +1,4 @@
-import AccountDataCache, { AccountFetchRetryAction } from '../src/main/account-data-cache';
+import AccountDataCache from '../src/main/account-data-cache';
 import { IAccountData } from '../src/shared/daemon-rpc-types';
 import sinon from 'sinon';
 import { expect, spy } from 'chai';
@@ -28,10 +28,7 @@ describe('IAccountData cache', () => {
     const watcher = new Promise((resolve, reject) => {
       cache.fetch(dummyAccountToken, {
         onFinish: () => resolve(),
-        onError: (_error: Error) => {
-          reject();
-          return AccountFetchRetryAction.stop;
-        },
+        onError: (_error: Error) => reject(),
       });
     });
 
@@ -47,10 +44,7 @@ describe('IAccountData cache', () => {
     const watcher = new Promise((resolve, reject) => {
       cache.fetch(dummyAccountToken, {
         onFinish: (_reason?: any) => resolve(),
-        onError: (_error: Error) => {
-          reject();
-          return AccountFetchRetryAction.stop;
-        },
+        onError: (_error: Error) => reject(),
       });
     });
 
@@ -66,10 +60,7 @@ describe('IAccountData cache', () => {
 
       cache.fetch(dummyAccountToken, {
         onFinish: () => {},
-        onError: (_error: Error) => {
-          reject();
-          return AccountFetchRetryAction.stop;
-        },
+        onError: (_error: Error) => reject(),
       });
     });
 
@@ -94,34 +85,7 @@ describe('IAccountData cache', () => {
 
       cache.fetch(dummyAccountToken, {
         onFinish: () => reject(),
-        onError: (_error: Error) => AccountFetchRetryAction.retry,
-      });
-    });
-
-    return expect(update).to.eventually.be.fulfilled;
-  });
-
-  it('should not retry if told to stop', async () => {
-    const update = new Promise((resolve, reject) => {
-      let firstAttempt = true;
-      const fetch = () => {
-        if (firstAttempt) {
-          firstAttempt = false;
-          setTimeout(() => clock.tick(14000), 0);
-          return Promise.reject(new Error('First attempt fails'));
-        } else {
-          reject();
-          return Promise.resolve(dummyAccountData);
-        }
-      };
-
-      const cache = new AccountDataCache(fetch, () => resolve());
-
-      setTimeout(resolve, 12000);
-
-      cache.fetch(dummyAccountToken, {
-        onFinish: () => {},
-        onError: (_error: Error) => AccountFetchRetryAction.stop,
+        onError: (_error: Error) => {},
       });
     });
 
@@ -129,7 +93,7 @@ describe('IAccountData cache', () => {
   });
 
   it('should cancel first fetch', async () => {
-    const firstError = spy((_error: Error) => AccountFetchRetryAction.stop);
+    const firstError = spy((_error: Error) => {});
     const secondSuccess = spy();
 
     const update = new Promise<IAccountData>((resolve, reject) => {
@@ -140,10 +104,7 @@ describe('IAccountData cache', () => {
 
           cache.fetch('1231231231', {
             onFinish: secondSuccess,
-            onError: () => {
-              reject();
-              return AccountFetchRetryAction.stop;
-            },
+            onError: () => reject(),
           });
 
           return new Promise<IAccountData>((resolve) => {
@@ -199,10 +160,7 @@ describe('IAccountData cache', () => {
 
       cache.fetch(dummyAccountToken, {
         onFinish: () => {},
-        onError: (_error: Error) => {
-          reject();
-          return AccountFetchRetryAction.stop;
-        },
+        onError: (_error: Error) => reject(),
       });
     });
 
@@ -236,10 +194,7 @@ describe('IAccountData cache', () => {
 
       cache.fetch(dummyAccountToken, {
         onFinish: () => {},
-        onError: (_error: Error) => {
-          firstError();
-          return AccountFetchRetryAction.retry;
-        },
+        onError: (_error: Error) => firstError(),
       });
       setTimeout(() => {
         cache.fetch(dummyAccountToken, {
@@ -247,7 +202,7 @@ describe('IAccountData cache', () => {
             secondSuccess();
             setTimeout(resolve);
           },
-          onError: (_error: Error) => AccountFetchRetryAction.stop,
+          onError: (_error: Error) => {},
         });
       });
     });
@@ -268,7 +223,7 @@ describe('IAccountData cache', () => {
       };
 
       const cache = new AccountDataCache(fetch, () => {});
-      const onError = (_error: Error) => AccountFetchRetryAction.stop;
+      const onError = (_error: Error) => {};
       cache.fetch(dummyAccountToken, { onFinish: () => {}, onError });
       cache.fetch(dummyAccountToken, { onFinish: () => resolve(), onError });
     });