diff --git a/src/sync-plugins/crud.ts b/src/sync-plugins/crud.ts index f699dd3c..924f25dd 100644 --- a/src/sync-plugins/crud.ts +++ b/src/sync-plugins/crud.ts @@ -13,6 +13,7 @@ import { symbolDelete, } from '@legendapp/state'; import { + SyncedErrorParams, SyncedGetParams, SyncedOptions, SyncedSetParams, @@ -54,8 +55,14 @@ export interface WaitForSetCrudFnParams extends WaitForSetFnParams { type: 'create' | 'update' | 'delete'; } +export interface CrudErrorParams extends Omit { + source: 'list' | 'get' | 'create' | 'update' | 'delete'; +} + +export type CrudOnErrorFn = (error: Error, params: CrudErrorParams) => void; + export interface SyncedCrudPropsBase - extends Omit, 'get' | 'set' | 'initial' | 'subscribe' | 'waitForSet'> { + extends Omit, 'get' | 'set' | 'initial' | 'subscribe' | 'waitForSet' | 'onError'> { create?(input: TRemote, params: SyncedSetParams): Promise | null | undefined | void>; update?( input: Partial, @@ -77,6 +84,7 @@ export interface SyncedCrudPropsBase | Promise | ObservableParam | ObservableEvent; + onError?: (error: Error, params: CrudErrorParams) => void; } type InitialValue = TAsOption extends 'Map' @@ -200,16 +208,18 @@ export function syncedCrud { - return Promise.all( - data.map((value: any) => - // Skip transforming any children with symbolDelete or fieldDeleted because they'll get deleted by resultsToOutType - value[symbolDelete] || - (fieldDeleted && value[fieldDeleted]) || - (fieldDeletedList && value[fieldDeletedList]) - ? value - : transform!.load!(value, 'get'), - ), - ); + return data.length + ? Promise.all( + data.map((value: any) => + // Skip transforming any children with symbolDelete or fieldDeleted because they'll get deleted by resultsToOutType + value[symbolDelete] || + (fieldDeleted && value[fieldDeleted]) || + (fieldDeletedList && value[fieldDeletedList]) + ? value + : transform!.load!(value, 'get'), + ), + ) + : []; }; const get: undefined | ((params: SyncedGetParams) => TLocal | Promise) = @@ -231,7 +241,9 @@ export function syncedCrud 0 ? transformed[0] - : ((((isLastSyncMode && lastSync) || fieldDeleted) && value) ?? null); + : getFn + ? ((((isLastSyncMode && lastSync) || fieldDeleted) && value) ?? null) + : undefined; } else { return resultsToOutType(transformed); } @@ -583,6 +595,6 @@ export function syncedCrud extends Omit, 'list' | 'retry'>, - SyncedCrudPropsBase { + Omit, 'onError'> { refPath: (uid: string | undefined) => string; query?: (ref: DatabaseReference) => DatabaseReference | Query; fieldId?: string; fieldTransforms?: FieldTransforms; + onError?: (error: Error, params: FirebaseErrorParams) => void; // Also in global config realtime?: boolean; requireAuth?: boolean; @@ -111,6 +112,12 @@ interface FirebaseFns { generateId: () => string; } +export interface FirebaseErrorParams extends Omit { + source: 'list' | 'get' | 'create' | 'update' | 'delete'; +} + +type OnErrorFn = (error: Error, params: FirebaseErrorParams) => void; + const fns: FirebaseFns = { isInitialized: () => { try { @@ -186,7 +193,8 @@ export function syncedFirebase): Promise => { + const list = async (getParams: SyncedGetParams): Promise => { + const { lastSync, onError } = getParams; const ref = computeRef(lastSync!); return new Promise((resolve) => { @@ -209,7 +217,7 @@ export function syncedFirebase (onError as OnErrorFn)(error, { source: 'list', type: 'get', retry: getParams }), ); }); }; @@ -389,7 +397,7 @@ export function syncedFirebase({ - ...rest, + ...(rest as any), // Workaround for type errors list, subscribe, create, diff --git a/src/sync-plugins/keel.ts b/src/sync-plugins/keel.ts index b1556504..7c53ca39 100644 --- a/src/sync-plugins/keel.ts +++ b/src/sync-plugins/keel.ts @@ -161,6 +161,8 @@ export interface SyncedKeelPropsBase void; } +type OnErrorFn = (error: Error, params: KeelErrorParams) => void; + const modifiedClients = new WeakSet>(); const isAuthed$ = observable(false); const isAuthing$ = observable(false); @@ -275,7 +277,7 @@ async function getAllPages( >, params: KeelListParams, listParams: SyncedGetParams, - onError: ((error: Error, params: KeelErrorParams) => void) | undefined, + onError: (error: Error, params: KeelErrorParams) => void, ): Promise { const allData: TRemote[] = []; let pageInfo: PageInfo | undefined = undefined; @@ -302,15 +304,13 @@ async function getAllPages( if (!handled) { const err = new Error(error.message, { cause: { error } }); - // TODO - onError?.(err, { + onError(err, { getParams: listParams, type: 'get', source: 'list', action: listFn.name || listFn.toString(), + retry: listParams, }); - - throw err; } } else if (data) { pageInfo = data.pageInfo as PageInfo; @@ -356,7 +356,6 @@ export function syncedKeel< fieldDeleted, realtime, mode, - onError, requireAuth = true, ...rest } = props; @@ -385,7 +384,7 @@ export function syncedKeel< const list = listParam ? async (listParams: SyncedGetParams) => { - const { lastSync } = listParams; + const { lastSync, onError } = listParams; const queryBySync = !!lastSync && changesSince === 'last-sync'; // If querying with lastSync pass it to the "where" parameters const where = Object.assign( @@ -396,7 +395,7 @@ export function syncedKeel< realtimeState.current = {}; - const promise = getAllPages(props, listParam, params, listParams, onError); + const promise = getAllPages(props, listParam, params, listParams, onError as OnErrorFn); if (realtime) { setupSubscribe!(listParams); @@ -408,7 +407,7 @@ export function syncedKeel< const get = getParam ? async (getParams: SyncedGetParams) => { - const { refresh } = getParams; + const { refresh, onError } = getParams; realtimeState.current = {}; @@ -425,15 +424,13 @@ export function syncedKeel< if (!handled) { const err = new Error(error.message, { cause: { error } }); - // TODO - onError?.(err, { + (onError as OnErrorFn)(err, { getParams, type: 'get', source: 'get', action: getParam.name || getParam.toString(), + retry: getParams, }); - - throw err; } } else { return data as TRemote; @@ -459,7 +456,7 @@ export function syncedKeel< fn: Function, from: 'create' | 'update' | 'delete', ) => { - const { update } = params; + const { update, onError } = params; if ( from === 'create' && @@ -485,15 +482,14 @@ export function syncedKeel< if (!handled) { const err = new Error(error.message, { cause: { error } }); - onError?.(err, { + (onError as OnErrorFn)(err, { setParams: params, input, type: 'set', source: from, action: fn.name || fn.toString(), + retry: params, }); - - throw err; } } }; diff --git a/src/sync-plugins/supabase.ts b/src/sync-plugins/supabase.ts index 3ec516d4..e4ce94be 100644 --- a/src/sync-plugins/supabase.ts +++ b/src/sync-plugins/supabase.ts @@ -3,6 +3,7 @@ import { SyncTransform, SyncedOptions, SyncedOptionsGlobal, + SyncedSetParams, combineTransforms, removeNullUndefined, transformStringifyDates, @@ -11,15 +12,16 @@ import { } from '@legendapp/state/sync'; import { CrudAsOption, + CrudOnErrorFn, SyncedCrudPropsBase, SyncedCrudPropsMany, SyncedCrudReturnType, WaitForSetCrudFnParams, syncedCrud, } from '@legendapp/state/sync-plugins/crud'; +import type { FunctionsResponse } from '@supabase/functions-js'; import type { PostgrestFilterBuilder, PostgrestQueryBuilder } from '@supabase/postgrest-js'; import type { PostgrestSingleResponse, SupabaseClient } from '@supabase/supabase-js'; -import type { FunctionsResponse } from '@supabase/functions-js'; // Unused types but maybe useful in the future so keeping them for now type DatabaseOf = Client extends SupabaseClient ? TDB : never; @@ -127,10 +129,16 @@ export function configureSyncedSupabase(config: SyncedSupabaseConfiguration) { } function wrapSupabaseFn(fn: (...args: any) => PromiseLike) { - return async (...args: any) => { - const { data, error } = await fn(...args); + return async (params: SyncedGetParams, ...args: any) => { + const { onError } = params; + const { data, error } = await fn(params, ...args); if (error) { - throw new Error(error.message); + (onError as CrudOnErrorFn)(new Error(error.message), { + getParams: params, + source: 'list', + type: 'get', + retry: params, + }); } return data; }; @@ -194,7 +202,7 @@ export function syncedSupabase< ? listParam ? wrapSupabaseFn(listParam) : async (params: SyncedGetParams) => { - const { lastSync } = params; + const { lastSync, onError } = params; const clientSchema = schema ? client.schema(schema as string) : client; const from = clientSchema.from(collection); let select = selectFn ? selectFn(from) : from.select(); @@ -209,24 +217,38 @@ export function syncedSupabase< select = filter(select, params); } const { data, error } = await select; - if (error) { - throw new Error(error?.message); + if (data) { + return (data! || []) as SupabaseRowOf[]; + } else if (error) { + (onError as CrudOnErrorFn)(new Error(error.message), { + getParams: params, + source: 'list', + type: 'get', + retry: params, + }); } - return (data! || []) as SupabaseRowOf[]; + return null; } : undefined; const create = createParam ? wrapSupabaseFn(createParam) : !actions || actions.includes('create') - ? async (input: SupabaseRowOf) => { + ? async (input: SupabaseRowOf, params: SyncedSetParams) => { + const { onError } = params; const res = await client.from(collection).insert(input).select(); const { data, error } = res; if (data) { const created = data[0]; return created; - } else { - throw new Error(error?.message); + } else if (error) { + (onError as CrudOnErrorFn)(new Error(error.message), { + setParams: params, + source: 'create', + type: 'set', + retry: params, + input, + }); } } : undefined; @@ -235,14 +257,21 @@ export function syncedSupabase< !actions || actions.includes('update') ? updateParam ? wrapSupabaseFn(updateParam) - : async (input: SupabaseRowOf) => { + : async (input: SupabaseRowOf, params: SyncedSetParams) => { + const { onError } = params; const res = await client.from(collection).update(input).eq('id', input.id).select(); const { data, error } = res; if (data) { const created = data[0]; return created; - } else { - throw new Error(error?.message); + } else if (error) { + (onError as CrudOnErrorFn)(new Error(error.message), { + setParams: params, + source: 'update', + type: 'set', + retry: params, + input, + }); } } : undefined; @@ -251,15 +280,25 @@ export function syncedSupabase< !fieldDeleted && (!actions || actions.includes('delete')) ? deleteParam ? wrapSupabaseFn(deleteParam) - : async (input: { id: SupabaseRowOf['id'] }) => { + : async ( + input: { id: SupabaseRowOf['id'] }, + params: SyncedSetParams, + ) => { + const { onError } = params; const id = input.id; const res = await client.from(collection).delete().eq('id', id).select(); const { data, error } = res; if (data) { const created = data[0]; return created; - } else { - throw new Error(error?.message); + } else if (error) { + (onError as CrudOnErrorFn)(new Error(error.message), { + setParams: params, + source: 'delete', + type: 'set', + retry: params, + input, + }); } } : undefined; diff --git a/src/sync/syncObservable.ts b/src/sync/syncObservable.ts index 70ea10e8..11b2d5fe 100644 --- a/src/sync/syncObservable.ts +++ b/src/sync/syncObservable.ts @@ -633,18 +633,26 @@ async function doChangeRemote(changeInfo: PreppedChangeRemote | undefined) { } | undefined = undefined; - let errorHandled = false; - const onError = (error: Error, retryParams?: OnErrorRetryParams) => { - state$.error.set(error); - if (!errorHandled) { - syncOptions.onError?.(error, { - setParams: setParams as SyncedSetParams, - source: 'set', - value$: obs$, - retryParams, - }); + let lastErrorHandled: Error | undefined; + + const onSetError = (error: Error, params?: SyncedErrorParams, noThrow?: boolean) => { + if (lastErrorHandled !== error) { + if (!params) { + params = { + setParams: setParams as SyncedSetParams, + source: 'set', + type: 'set', + input: value, + retry: setParams, + }; + } + state$.error.set(error); + syncOptions.onError?.(error, params); + lastErrorHandled = error; + if (!noThrow) { + throw error; + } } - errorHandled = true; }; const setParams: SyncedSetParams = { @@ -652,7 +660,7 @@ async function doChangeRemote(changeInfo: PreppedChangeRemote | undefined) { value$: obs$, changes: changesRemote, value, - onError, + onError: onSetError, update: (params: UpdateFnParams) => { if (updateResult) { const { value, lastSync, mode } = params; @@ -676,7 +684,7 @@ async function doChangeRemote(changeInfo: PreppedChangeRemote | undefined) { async () => { return syncOptions!.set!(setParams); }, - onError, + (error) => onSetError(error, undefined, true), ); let didError = false; @@ -684,7 +692,7 @@ async function doChangeRemote(changeInfo: PreppedChangeRemote | undefined) { await savedPromise.catch((error) => { didError = true; if (!syncOptions.retry) { - onError(error); + onSetError(error, undefined, true); } }); } @@ -937,13 +945,23 @@ export function syncObservable( allSyncStates.set(syncState$, node); syncStateValue.getPendingChanges = () => localState.pendingChanges; - let errorHandled = false; - const onGetError = (error: Error, params: Omit) => { - syncState$.error.set(error); - if (!errorHandled) { - syncOptions.onError?.(error, { ...params, value$: obs$ }); + let lastErrorHandled: Error | undefined; + const onGetError = (error: Error, params: SyncedErrorParams, noThrow?: boolean) => { + if (lastErrorHandled !== error) { + if (!params) { + params = { + source: 'get', + type: 'get', + retry: params, + }; + } + syncState$.error.set(error); + syncOptions.onError?.(error, params); + lastErrorHandled = error; + if (!noThrow) { + throw error; + } } - errorHandled = true; }; loadLocal(obs$, syncOptions, syncState$, localState); @@ -1140,7 +1158,13 @@ export function syncObservable( ); }, refresh: () => when(syncState$.isLoaded, sync), - onError: (error: Error) => onGetError(error, { source: 'subscribe', subscribeParams }), + onError: (error: Error) => + onGetError(error, { + source: 'subscribe', + subscribeParams, + type: 'get', + retry: {} as OnErrorRetryParams, + }), }; unsubscribe = subscribe(subscribeParams); }; @@ -1154,7 +1178,9 @@ export function syncObservable( const existingValue = getNodeValue(node); if (get) { - const onError = (error: Error) => onGetError(error, { getParams, source: 'get' }); + const onError = (error: Error) => { + onGetError(error, { getParams, source: 'get', type: 'get', retry: getParams }); + }; const getParams: SyncedGetParams = { node, value$: obs$, @@ -1165,7 +1191,7 @@ export function syncObservable( options: syncOptions, lastSync, updateLastSync: (lastSync: number) => (getParams.lastSync = lastSync), - onError, + onError: onGetError, retryNum: 0, cancelRetry: false, }; @@ -1239,7 +1265,13 @@ export function syncObservable( }); }; if (isPromise(got)) { - got.then(handle).catch(onError); + got.then(handle).catch((error) => { + onGetError( + error, + { getParams, source: 'get', type: 'get', retry: getParams }, + true, + ); + }); } else { handle(got); } diff --git a/src/sync/syncTypes.ts b/src/sync/syncTypes.ts index 4236f2b5..2d87a1f0 100644 --- a/src/sync/syncTypes.ts +++ b/src/sync/syncTypes.ts @@ -51,13 +51,13 @@ export interface SyncedGetParams extends SyncedGetSetBaseParams { lastSync: number | undefined; updateLastSync: (lastSync: number) => void; mode: GetMode; - onError: (error: Error) => void; + onError: (error: Error, params: SyncedErrorParams) => void; options: SyncedOptions; } export interface SyncedSetParams extends Pick, 'changes' | 'value'>, SyncedGetSetBaseParams { update: UpdateFn; - onError: (error: Error, retryParams: OnErrorRetryParams) => void; + onError: (error: Error, params: SyncedErrorParams) => void; } export interface SyncedSubscribeParams extends SyncedGetSetSubscribeBaseParams { @@ -67,11 +67,12 @@ export interface SyncedSubscribeParams extends SyncedGetSetSubscribeBas } export interface SyncedErrorParams { + source: 'get' | 'set' | 'subscribe'; + type: 'get' | 'set'; + retry: OnErrorRetryParams; getParams?: SyncedGetParams; setParams?: SyncedSetParams; subscribeParams?: SyncedSubscribeParams; - source: 'get' | 'set' | 'subscribe'; - type: 'get' | 'set'; input?: any; } diff --git a/tests/crud.test.ts b/tests/crud.test.ts index 7a2ca6d2..13f3482c 100644 --- a/tests/crud.test.ts +++ b/tests/crud.test.ts @@ -396,6 +396,53 @@ describe('Crud object get', () => { as: 'value', })); }); +describe('Crud as value list', () => { + test('does not overwrite if returns []', async () => { + const persistName = getPersistName(); + localStorage.setItem(persistName, JSON.stringify({ id: 'id', test: 'hi', updatedAt: 1 })); + localStorage.setItem( + persistName + '__m', + JSON.stringify({ lastSync: 1000, pending: { test: { p: 'h', t: ['object'], v: 'hi' } } }), + ); + const obs$ = observable( + syncedCrud({ + list: () => promiseTimeout(0, []), + as: 'value', + persist: { + name: persistName, + plugin: ObservablePersistLocalStorage, + }, + }), + ); + + expect(obs$.get()).toEqual({ id: 'id', test: 'hi', updatedAt: 1 }); + + await promiseTimeout(1); + + expect(obs$.get()).toEqual({ id: 'id', test: 'hi', updatedAt: 1 }); + }); + test('sets if returns new value', async () => { + const persistName = getPersistName(); + localStorage.setItem(persistName, JSON.stringify({ id: 'id', test: 'hi', updatedAt: 1 })); + const obs$ = observable( + syncedCrud({ + list: () => promiseTimeout(0, [{ id: 'id2', test: 'hi2', updatedAt: 2 }]), + as: 'value', + persist: { + name: persistName, + plugin: ObservablePersistLocalStorage, + }, + }), + ); + + expect(obs$.get()).toEqual({ id: 'id', test: 'hi', updatedAt: 1 }); + + await promiseTimeout(1); + + expect(obs$.get()).toEqual({ id: 'id2', test: 'hi2', updatedAt: 2 }); + }); +}); + describe('Crud as Object list', () => { test('defaults to object', async () => { const obs = observable( @@ -2774,6 +2821,7 @@ describe('Error is set', () => { }); test('onError is called if create fails', async () => { let errorAtOnError: Error | undefined = undefined; + let numErrors = 0; const obs$ = observable( syncedCrud({ list: () => promiseTimeout(0, [ItemBasicValue()]), @@ -2782,6 +2830,7 @@ describe('Error is set', () => { throw new Error('test'); }, onError: (error) => { + numErrors++; errorAtOnError = error; }, }), @@ -2797,6 +2846,32 @@ describe('Error is set', () => { await promiseTimeout(1); expect(errorAtOnError).toEqual(new Error('test')); + expect(numErrors).toEqual(1); + }); + test('onError is called if list fails', async () => { + let errorAtOnError: Error | undefined = undefined; + let numErrors = 0; + const obs$ = observable( + syncedCrud({ + list: () => { + throw new Error('test'); + }, + as: 'object', + onError: (error) => { + numErrors++; + errorAtOnError = error; + }, + }), + ); + + expect(obs$.get()).toEqual(undefined); + + await promiseTimeout(1); + + expect(obs$.get()).toEqual(undefined); + + expect(errorAtOnError).toEqual(new Error('test')); + expect(numErrors).toEqual(1); }); }); describe('soft delete', () => { diff --git a/tests/keel.test.ts b/tests/keel.test.ts index a1698df4..2bb97046 100644 --- a/tests/keel.test.ts +++ b/tests/keel.test.ts @@ -1,7 +1,7 @@ import { observable } from '@legendapp/state'; import { syncedKeel as syncedKeelOrig } from '../src/sync-plugins/keel'; -import { promiseTimeout } from './testglobals'; import { configureSynced } from '../src/sync/configureSynced'; +import { promiseTimeout } from './testglobals'; type APIError = { type: string; message: string; requestId?: string }; @@ -86,6 +86,86 @@ describe('keel', () => { expect(obs.id1.get()).toEqual({ id: 'id1', test: 'hi', createdAt: 1, updatedAt: 1 }); expect(obs.id1.test.get()).toEqual('hi'); }); + test('list error retries', async () => { + let numLists = 0; + const obs = observable( + syncedKeel({ + list: () => { + numLists++; + return { error: { message: 'test' }, data: undefined } as any; + }, + retry: { + delay: 1, + times: 2, + }, + }), + ); + + expect(obs.get()).toEqual(undefined); + expect(numLists).toEqual(1); + + await promiseTimeout(10); + + expect(obs.get()).toEqual(undefined); + expect(numLists).toEqual(2); + }); + test('list error calls onError', async () => { + let numLists = 0; + let errorCalled: string | undefined; + const obs = observable( + syncedKeel({ + list: async () => { + numLists++; + return { error: { message: 'test' }, data: undefined } as any; + }, + onError(error) { + errorCalled = error.message; + }, + retry: { + delay: 1, + times: 2, + }, + }), + ); + + expect(obs.get()).toEqual(undefined); + expect(numLists).toEqual(1); + + await promiseTimeout(10); + + expect(errorCalled).toEqual('test'); + expect(obs.get()).toEqual(undefined); + expect(numLists).toEqual(2); + }); + test('list error calls onError and can cancel retry', async () => { + let numLists = 0; + let errorCalled: string | undefined; + const obs = observable( + syncedKeel({ + list: async () => { + numLists++; + return { error: { message: 'test' }, data: undefined } as any; + }, + onError(error, params) { + errorCalled = error.message; + params.retry.cancelRetry = true; + }, + retry: { + delay: 1, + times: 2, + }, + }), + ); + + expect(obs.get()).toEqual(undefined); + expect(numLists).toEqual(1); + + await promiseTimeout(10); + + expect(errorCalled).toEqual('test'); + expect(obs.get()).toEqual(undefined); + expect(numLists).toEqual(1); + }); test('get', async () => { const obs = observable( syncedKeel({ @@ -184,4 +264,71 @@ describe('keel', () => { other: null, }); }); + test('setting error retries', async () => { + let numUpdates = 0; + const obs = observable( + syncedKeel({ + get: () => fakeKeelGet({ ...ItemBasicValue(), other: 2, another: 3 }), + update: async () => { + numUpdates++; + return { error: { message: 'test' } } as any; + }, + retry: { + delay: 1, + times: 2, + }, + }), + ); + + obs.get(); + + await promiseTimeout(1); + + obs.other.set(4); + + await promiseTimeout(1); + expect(numUpdates).toEqual(1); + + await promiseTimeout(10); + + expect(numUpdates).toEqual(2); + }); + test('setting error retries', async () => { + let numUpdates = 0; + let numErrors = 0; + let errorMessage: string | undefined; + const obs = observable( + syncedKeel({ + get: () => fakeKeelGet({ ...ItemBasicValue(), other: 2, another: 3 }), + update: async () => { + numUpdates++; + return { error: { message: 'test' } } as any; + }, + retry: { + delay: 1, + times: 2, + }, + onError(error) { + numErrors++; + errorMessage = error.message; + }, + }), + ); + + obs.get(); + + await promiseTimeout(1); + + obs.other.set(4); + + await promiseTimeout(1); + expect(numUpdates).toEqual(1); + expect(numErrors).toEqual(1); + + await promiseTimeout(10); + + expect(numUpdates).toEqual(2); + expect(numErrors).toEqual(2); + expect(errorMessage).toEqual('test'); + }); });