Skip to content

Commit

Permalink
fix(component-store): make synchronous updater errors catchable (#3490)
Browse files Browse the repository at this point in the history 
* fix(component-store): make synchronous updater errors catchable

* chore: CR suggestions
  • Loading branch information
@markostanimirovic
markostanimirovic committed Aug 15, 2022
1 parent 4da9a67 commit 1a906fd
Show file tree
Hide file tree
Showing 2 changed files with 147 additions and 86 deletions.
196 changes: 124 additions & 72 deletions modules/component-store/spec/component-store.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
scheduled,
queueScheduler,
asyncScheduler,
throwError,
} from 'rxjs';
import {
delayWhen,
Expand Down Expand Up @@ -79,20 +80,9 @@ describe('Component Store', () => {
it(
'throws an Error when setState with a function/callback is called' +
' before initialization',
marbles((m) => {
() => {
const componentStore = new ComponentStore();

m.expect(componentStore.state$).toBeObservable(
m.hot(
'#',
{},
new Error(
'ComponentStore has not been initialized yet. ' +
'Please make sure it is initialized before updating/getting.'
)
)
);

expect(() => {
componentStore.setState(() => ({ setState: 'new state' }));
}).toThrow(
Expand All @@ -101,7 +91,7 @@ describe('Component Store', () => {
'Please make sure it is initialized before updating/getting.'
)
);
})
}
);

it('throws an Error when patchState with an object is called before initialization', () => {
Expand Down Expand Up @@ -147,55 +137,30 @@ describe('Component Store', () => {
}
);

it(
'throws an Error when updater is called before initialization',
marbles((m) => {
const componentStore = new ComponentStore();

m.expect(componentStore.state$).toBeObservable(
m.hot(
'#',
{},
new Error(
'ComponentStore has not been initialized yet. ' +
'Please make sure it is initialized before updating/getting.'
)
)
);
it('throws an Error synchronously when updater is called before initialization', () => {
const componentStore = new ComponentStore();

expect(() => {
componentStore.updater((state, value: object) => value)({
updater: 'new state',
});
}).toThrow(
new Error(
'ComponentStore has not been initialized yet. ' +
'Please make sure it is initialized before updating/getting.'
)
);
})
);
expect(() => {
componentStore.updater((state, value: object) => value)({
updater: 'new state',
});
}).toThrow(
new Error(
'ComponentStore has not been initialized yet. ' +
'Please make sure it is initialized before updating/getting.'
)
);
});

it(
'throws an Error when updater is called with sync Observable' +
' before initialization',
marbles((m) => {
() => {
const componentStore = new ComponentStore();
const syncronousObservable$ = of({
updater: 'new state',
});

m.expect(componentStore.state$).toBeObservable(
m.hot(
'#',
{},
new Error(
'ComponentStore has not been initialized yet. ' +
'Please make sure it is initialized before updating/getting.'
)
)
);

expect(() => {
componentStore.updater<object>((state, value) => value)(
syncronousObservable$
Expand All @@ -206,39 +171,32 @@ describe('Component Store', () => {
'Please make sure it is initialized before updating/getting.'
)
);
})
}
);

it(
'does not throw an Error when updater is called with async Observable' +
' before initialization, however closes the subscription and does not' +
' update the state and sends error in state$',
'throws an Error asynchronously when updater is called with async' +
' Observable before initialization, however closes the subscription' +
' and does not update the state',
marbles((m) => {
const componentStore = new ComponentStore();
const asyncronousObservable$ = m.cold('-u', {
const asynchronousObservable$ = m.cold('-u', {
u: { updater: 'new state' },
});

let subscription: Subscription | undefined;

m.expect(componentStore.state$).toBeObservable(
m.hot(
'-#',
{},
new Error(
'ComponentStore has not been initialized yet. ' +
'Please make sure it is initialized before updating/getting.'
)
)
);

expect(() => {
subscription = componentStore.updater(
(state, value: object) => value
)(asyncronousObservable$);
}).not.toThrow();

m.flush();
)(asynchronousObservable$);
m.flush();
}).toThrow(
new Error(
'ComponentStore has not been initialized yet. ' +
'Please make sure it is initialized before updating/getting.'
)
);

expect(subscription!.closed).toBe(true);
})
Expand Down Expand Up @@ -638,6 +596,100 @@ describe('Component Store', () => {
);
});

describe('throws an error', () => {
it('synchronously when synchronous error is thrown within updater', () => {
const componentStore = new ComponentStore({});
const error = new Error('ERROR!');
const updater = componentStore.updater(() => {
throw error;
});

expect(() => updater()).toThrow(error);
});

it('synchronously when synchronous error is thrown within setState callback', () => {
const componentStore = new ComponentStore({});
const error = new Error('ERROR!');

expect(() => {
componentStore.setState(() => {
throw error;
});
}).toThrow(error);
});

it('synchronously when synchronous error is thrown within patchState callback', () => {
const componentStore = new ComponentStore({});
const error = new Error('ERROR!');

expect(() => {
componentStore.patchState(() => {
throw error;
});
}).toThrow(error);
});

it('synchronously when synchronous observable throws an error with updater', () => {
const componentStore = new ComponentStore({});
const error = new Error('ERROR!');
const updater = componentStore.updater<unknown>(() => ({}));

expect(() => {
updater(throwError(() => error));
}).toThrow(error);
});

it('synchronously when synchronous observable throws an error with patchState', () => {
const componentStore = new ComponentStore({});
const error = new Error('ERROR!');

expect(() => {
componentStore.patchState(throwError(() => error));
}).toThrow(error);
});

it(
'asynchronously when asynchronous observable throws an error with updater',
marbles((m) => {
const componentStore = new ComponentStore({});
const error = new Error('ERROR!');
const updater = componentStore.updater<unknown>(() => ({}));
const asyncObs$ = m.cold('-#', {}, error);

expect(() => {
try {
updater(asyncObs$);
} catch {
throw new Error('updater should not throw an error synchronously');
}

m.flush();
}).toThrow(error);
})
);

it(
'asynchronously when asynchronous observable throws an error with patchState',
marbles((m) => {
const componentStore = new ComponentStore({});
const error = new Error('ERROR!');
const asyncObs$ = m.cold('-#', {}, error);

expect(() => {
try {
componentStore.patchState(asyncObs$);
} catch {
throw new Error(
'patchState should not throw an error synchronously'
);
}

m.flush();
}).toThrow(error);
})
);
});

describe('selector', () => {
interface State {
value: string;
Expand Down
37 changes: 23 additions & 14 deletions modules/component-store/src/component-store.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
queueScheduler,
scheduled,
asyncScheduler,
EMPTY,
} from 'rxjs';
import {
concatMap,
Expand All @@ -19,6 +20,8 @@ import {
distinctUntilChanged,
shareReplay,
take,
tap,
catchError,
} from 'rxjs/operators';
import { debounceSync } from './debounce-sync';
import {
Expand Down Expand Up @@ -110,7 +113,10 @@ export class ComponentStore<T extends object> implements OnDestroy {
return ((
observableOrValue?: OriginType | Observable<OriginType>
): Subscription => {
let initializationError: Error | undefined;
// We need to explicitly throw an error if a synchronous error occurs.
// This is necessary to make synchronous errors catchable.
let isSyncUpdate = true;
let syncError: unknown;
// We can receive either the value or an observable. In case it's a
// simple value, we'll wrap it with `of` operator to turn it into
// Observable.
Expand All @@ -128,23 +134,26 @@ export class ComponentStore<T extends object> implements OnDestroy {
: // If state was not initialized, we'll throw an error.
throwError(() => new Error(this.notInitializedErrorMessage))
),
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
map(([value, currentState]) => updaterFn(currentState, value!)),
tap((newState) => this.stateSubject$.next(newState)),
catchError((error: unknown) => {
if (isSyncUpdate) {
syncError = error;
return EMPTY;
}

return throwError(() => error);
}),
takeUntil(this.destroy$)
)
.subscribe({
next: ([value, currentState]) => {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
this.stateSubject$.next(updaterFn(currentState, value!));
},
error: (error: Error) => {
initializationError = error;
this.stateSubject$.error(error);
},
});
.subscribe();

if (initializationError) {
// prettier-ignore
throw /** @type {!Error} */ (initializationError);
if (syncError) {
throw syncError;
}
isSyncUpdate = false;

return subscription;
}) as unknown as ReturnType;
}
Expand Down

0 comments on commit 1a906fd

Please sign in to comment.