fix: error not being set if get failed without retry

LegendApp · Sep 20, 2024 · 2d21a36 · 2d21a36
1 parent 6c3104e
commit 2d21a36
Show file tree

Hide file tree

Showing 3 changed files with 80 additions and 40 deletions.
diff --git a/src/sync/retry.ts b/src/sync/retry.ts
@@ -28,46 +28,50 @@ export function runWithRetry<T>(
     fn: (params: OnErrorRetryParams) => T | Promise<T>,
     onError: (error: Error, retryParams: OnErrorRetryParams) => void,
 ): T | Promise<T> {
-    let value = fn(state);
+    try {
+        let value = fn(state);
 
-    if (isPromise(value) && retryOptions) {
-        let timeoutRetry: number;
-        if (mapRetryTimeouts.has(state.node)) {
-            clearTimeout(mapRetryTimeouts.get(state.node));
-        }
-        return new Promise<any>((resolve, reject) => {
-            const run = () => {
-                (value as Promise<any>)
-                    .then((val: any) => {
-                        resolve(val);
-                    })
-                    .catch((error: Error) => {
-                        state.retryNum++;
-                        if (timeoutRetry) {
-                            clearTimeout(timeoutRetry);
-                        }
-                        if (onError) {
-                            onError(error, state);
-                        }
-                        if (!state.cancelRetry) {
-                            const timeout = createRetryTimeout(retryOptions, state.retryNum, () => {
-                                value = fn(state);
-                                run();
-                            });
+        if (isPromise(value) && retryOptions) {
+            let timeoutRetry: number;
+            if (mapRetryTimeouts.has(state.node)) {
+                clearTimeout(mapRetryTimeouts.get(state.node));
+            }
+            return new Promise<any>((resolve, reject) => {
+                const run = () => {
+                    (value as Promise<any>)
+                        .then((val: any) => {
+                            resolve(val);
+                        })
+                        .catch((error: Error) => {
+                            state.retryNum++;
+                            if (timeoutRetry) {
+                                clearTimeout(timeoutRetry);
+                            }
+                            if (onError) {
+                                onError(error, state);
+                            }
+                            if (!state.cancelRetry) {
+                                const timeout = createRetryTimeout(retryOptions, state.retryNum, () => {
+                                    value = fn(state);
+                                    run();
+                                });
 
-                            if (timeout === false) {
-                                state.cancelRetry = true;
-                                reject(error);
-                            } else {
-                                mapRetryTimeouts.set(state.node, timeout);
-                                timeoutRetry = timeout;
+                                if (timeout === false) {
+                                    state.cancelRetry = true;
+                                    reject(error);
+                                } else {
+                                    mapRetryTimeouts.set(state.node, timeout);
+                                    timeoutRetry = timeout;
+                                }
                             }
-                        }
-                    });
-            };
-            run();
-        });
-    }
+                        });
+                };
+                run();
+            });
+        }
 
-    return value;
+        return value;
+    } catch (error) {
+        return Promise.reject(error);
+    }
 }
diff --git a/src/sync/syncObservable.ts b/src/sync/syncObservable.ts
@@ -1126,7 +1126,9 @@ export function syncObservable<T>(
                     }
                     const existingValue = getNodeValue(node);
 
-                    const onError = (error: Error) => onGetError(error, { getParams, source: 'get' });
+                    const onError = (error: Error) => {
+                        onGetError(error, { getParams, source: 'get' });
+                    };
 
                     const getParams: SyncedGetParams<T> = {
                         node,
@@ -1211,7 +1213,7 @@ export function syncObservable<T>(
                             });
                         };
                         if (isPromise(got)) {
-                            got.then(handle);
+                            got.then(handle).catch(onError);
                         } else {
                             handle(got);
                         }

diff --git a/tests/crud.test.ts b/tests/crud.test.ts
@@ -2540,3 +2540,37 @@ describe('waitForSet', () => {
         expect(typeAtWaitForSet).toEqual('create');
     });
 });
+describe('Error is set', () => {
+    test('error is set if get fails', async () => {
+        const obs$ = observable<BasicValue>(
+            syncedCrud({
+                get: () => {
+                    // await promiseTimeout(1);
+                    throw new Error('test');
+                },
+            }),
+        );
+
+        obs$.get();
+
+        await promiseTimeout(1);
+
+        expect(syncState(obs$).error.get()).toEqual(new Error('test'));
+    });
+    test('error is set if get fails async', async () => {
+        const obs$ = observable<BasicValue>(
+            syncedCrud({
+                get: async () => {
+                    await promiseTimeout(1);
+                    throw new Error('test');
+                },
+            }),
+        );
+
+        obs$.get();
+
+        await promiseTimeout(2);
+
+        expect(syncState(obs$).error.get()).toEqual(new Error('test'));
+    });
+});