improve retry behavior, add retry on set

LegendApp · Oct 29, 2023 · 7799101 · 7799101
1 parent 08efd93
commit 7799101
Show file tree

Hide file tree

Showing 7 changed files with 79 additions and 17 deletions.
diff --git a/index.ts b/index.ts
@@ -57,6 +57,7 @@ export { ObservablePrimitiveClass } from './src/ObservablePrimitive';
 import { get, getProxy, observableFns, peek, set } from './src/ObservableObject';
 import { ensureNodeValue, findIDKey, getNode, globalState, optimized, setNodeValue, symbolDelete } from './src/globals';
 import { setAtPath } from './src/helpers';
+import { setupRetry } from './src/retry';
 
 export const internal = {
     ensureNodeValue,
@@ -71,5 +72,6 @@ export const internal = {
     set,
     setAtPath,
     setNodeValue,
+    setupRetry,
     symbolDelete,
 };
diff --git a/persist.ts b/persist.ts
@@ -25,7 +25,9 @@ declare module '@legendapp/state' {
         updateLastSync: (lastSync: number) => void;
         retry: (options?: RetryOptions) => void;
     }
-    // interface OnSetExtra {}
+    interface OnSetExtra {
+        onError: () => void;
+    }
     // interface SubscribeOptions {}
     // eslint-disable-next-line @typescript-eslint/no-empty-interface
     interface ObservableState extends ObservablePersistStateBase {}

diff --git a/src/ObservableObject.ts b/src/ObservableObject.ts
@@ -927,6 +927,7 @@ function activateNodeFunction(node: NodeValue, lazyFn: () => void) {
     let update: UpdateFn;
     let wasPromise: Promise<any> | undefined;
     let timeoutRetry: { current?: any };
+    const attemptNum = { current: 0 };
     const activator = (isFunction(node) ? node : lazyFn) as (value: ActivateProxyParams) => any;
     const refresh = () => (node.state as ObservableObject<ObservablePersistStateInternal>).refreshNum.set((v) => v + 1);
     observe(
@@ -981,7 +982,7 @@ function activateNodeFunction(node: NodeValue, lazyFn: () => void) {
                         if (timeoutRetry) {
                             clearTimeout(timeoutRetry.current);
                         }
-                        const { handleError, timeout } = setupRetry(retryOptions, refresh);
+                        const { handleError, timeout } = setupRetry(retryOptions, refresh, attemptNum);
                         onError = handleError;
                         timeoutRetry = timeout;
                     }

diff --git a/src/observableInterfaces.ts b/src/observableInterfaces.ts
@@ -550,7 +550,11 @@ export interface ActivateParams<T = any> {
 export interface ActivateProxyParams<T = any> extends ActivateParams {
     proxy: (fn: (key: string, params: ActivateParams<T>) => T | Promise<T>) => void;
 }
-export type UpdateFn = (params: ObservableOnChangeParams) => void;
+export type UpdateFn = (params: {
+    value: unknown;
+    mode?: 'assign' | 'set' | 'dateModified';
+    dateModified?: number | undefined;
+}) => void;
 export interface RetryOptions {
     infinite?: boolean;
     times?: number;

diff --git a/src/persist/persistActivateNode.ts b/src/persist/persistActivateNode.ts
@@ -9,7 +9,7 @@ import type {
 } from '@legendapp/state';
 import { internal, isPromise, mergeIntoObservable } from '@legendapp/state';
 import { persistObservable } from './persistObservable';
-const { getProxy, globalState } = internal;
+const { getProxy, globalState, setupRetry } = internal;
 
 export function persistActivateNode() {
     globalState.activateNode = function activateNodePersist(
@@ -40,16 +40,33 @@ export function persistActivateNode() {
             // TODO: Work out these types better
             pluginRemote.set = async (params: ObservablePersistRemoteSetParams<any>) => {
                 if (node.state?.isLoaded.get()) {
-                    let changes = {};
-                    let maxModified = 0;
-                    await onSetFn(params as unknown as ListenerParams, {
-                        update: (params) => {
-                            const { value, dateModified } = params;
-                            maxModified = Math.max(dateModified || 0, maxModified);
-                            changes = mergeIntoObservable(changes, value);
-                        },
+                    return new Promise((resolve) => {
+                        const attemptNum = { current: 0 };
+                        const run = async () => {
+                            let changes = {};
+                            let maxModified = 0;
+                            let didError = false;
+                            let onError: () => void;
+                            if (retryOptions) {
+                                onError = setupRetry(retryOptions, run, attemptNum).handleError;
+                            }
+                            await onSetFn(params as unknown as ListenerParams, {
+                                update: (params) => {
+                                    const { value, dateModified } = params;
+                                    maxModified = Math.max(dateModified || 0, maxModified);
+                                    changes = mergeIntoObservable(changes, value);
+                                },
+                                onError: () => {
+                                    didError = true;
+                                    onError?.();
+                                },
+                            });
+                            if (!didError) {
+                                resolve({ changes, dateModified: maxModified || undefined });
+                            }
+                        };
+                        run();
                     });
-                    return { changes, dateModified: maxModified || undefined };
                 }
             };
         }

diff --git a/src/retry.ts b/src/retry.ts
@@ -1,13 +1,13 @@
 import type { RetryOptions } from './observableInterfaces';
 
-export function setupRetry(retryOptions: RetryOptions, refresh: () => void) {
-    let attemptNum = 0;
+export function setupRetry(retryOptions: RetryOptions, refresh: () => void, attemptNum: { current: number }) {
     const timeout: { current?: any } = {};
     let didGiveUp = false;
     const { backoff, delay = 1000, infinite, times = 3, maxDelay = 30000 } = retryOptions;
     let handleError: () => void;
-    if (infinite || attemptNum++ < times) {
-        const delayTime = Math.min(delay * (backoff === 'constant' ? 1 : 2 ** attemptNum), maxDelay);
+    attemptNum.current++;
+    if (infinite || attemptNum.current < times) {
+        const delayTime = Math.min(delay * (backoff === 'constant' ? 1 : 2 ** attemptNum.current), maxDelay);
         handleError = () => {
             timeout.current = setTimeout(refresh, delayTime);
         };
@@ -23,6 +23,8 @@ export function setupRetry(retryOptions: RetryOptions, refresh: () => void) {
                     clearTimeout(timeout.current);
                     timeout.current = undefined;
                 }
+                // Restart the backoff when coming back online
+                attemptNum.current = 0;
                 didGiveUp = false;
                 refresh();
             }

diff --git a/tests/computed-persist.test.ts b/tests/computed-persist.test.ts
@@ -169,4 +169,38 @@ describe('retry', () => {
         await promiseTimeout(0);
         expect(obs$.get()).toEqual('hi');
     });
+    test('retry a set', async () => {
+        const attemptNum$ = observable(0);
+        let saved = false;
+        const obs$ = observable(async ({ retry, onSet }: ActivateParams) => {
+            retry({
+                delay: 1,
+            });
+
+            onSet(({ value }, { onError }) => {
+                return new Promise((resolve, reject) => {
+                    attemptNum$.set((v) => v + 1);
+                    if (attemptNum$.get() > 2) {
+                        saved = true;
+                    } else {
+                        onError();
+                    }
+                });
+            });
+
+            return 1;
+        });
+
+        obs$.get();
+
+        expect(attemptNum$.get()).toEqual(0);
+        obs$.set(1);
+        await when(() => attemptNum$.get() === 1);
+        expect(attemptNum$.get()).toEqual(1);
+        expect(saved).toEqual(false);
+        await when(() => attemptNum$.get() === 2);
+        expect(saved).toEqual(false);
+        await when(() => attemptNum$.get() === 3);
+        expect(saved).toEqual(true);
+    });
 });