Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle resolution errors in @wordpress/data #38669

Merged
merged 29 commits into from
Mar 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
ce74617
First stab at the FAIL_RESOLUTION meta action
adamziel Feb 9, 2022
f4dcd3a
Adjust hasStartedResolution
adamziel Feb 10, 2022
12d9c34
Remove try/catch to see if it helps with e2e failures
adamziel Feb 11, 2022
3637c5d
Adjust resolvers-cache-middleware to use the new state shape
adamziel Feb 11, 2022
6635a8c
Add unit tests
adamziel Feb 11, 2022
0fa1efe
Improve test coverage
adamziel Feb 11, 2022
22a569c
make resolveSelect reject on failure
adamziel Feb 11, 2022
ef8fd08
Fix unit tests
adamziel Feb 14, 2022
ae16800
Replace LastResolution with just Resolution
adamziel Feb 15, 2022
15b7cf6
Rename getResolutionFailure to getResolutionError
adamziel Feb 17, 2022
d35bab0
Use real timers in Jest tests
adamziel Feb 17, 2022
d992438
Simplify the reducer code
adamziel Feb 17, 2022
468a343
Add a unit test for the falsy error case
adamziel Feb 17, 2022
e8ffe33
Add the missing selectorArgsToStateKey call
adamziel Feb 21, 2022
5f6dc6a
Align doc strings
adamziel Feb 21, 2022
37c4d2f
Denote that error may be of unknown type in the docstring
adamziel Feb 21, 2022
4e32352
Explain that an error may be of unknown type
adamziel Feb 21, 2022
aa11187
Adjust typescript type hints
adamziel Feb 22, 2022
4a28600
Fix tests, use the actual store to test selectors
adamziel Feb 22, 2022
afd54d0
Remove dev artifact
adamziel Feb 22, 2022
fed1275
Refactor isResolving to enum-based status
adamziel Mar 1, 2022
07f4dbf
Rename e to error
adamziel Mar 2, 2022
61612c8
Use undefined for an idle status so that getResolutionState can alway…
adamziel Mar 2, 2022
f37c02c
Code style: Inline the awaited statement in resolve() in tests
adamziel Mar 2, 2022
9a6f520
Replace advanceTimersByTime with runAllTimers
adamziel Mar 2, 2022
10f6af0
Update the documentation and condition formulation in the resolve-cac…
adamziel Mar 2, 2022
1944699
Remove the undefined status type and only use the values actually ret…
adamziel Mar 4, 2022
cd78776
Simplify hasStartedResolution
adamziel Mar 4, 2022
7ace315
Add a changelog entry
adamziel Mar 7, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/data/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
### New Features

- Enabled thunks by default for all stores and removed the `__experimentalUseThunks` flag.
- Store the resolution errors in store metadata and expose them using `hasResolutionFailed` the `getResolutionError` meta-selectors ([#38669](https://github.com/WordPress/gutenberg/pull/38669)).
- Expose the resolution status (undefined, resolving, finished, error) via the `getResolutionState` meta-selector ([#38669](https://github.com/WordPress/gutenberg/pull/38669)).

## 6.2.1 (2022-02-10)

Expand Down
52 changes: 40 additions & 12 deletions packages/data/src/redux-store/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -330,20 +330,35 @@ function mapResolveSelectors( selectors, store ) {
'getCachedResolvers',
] ),
( selector, selectorName ) => ( ...args ) =>
new Promise( ( resolve ) => {
new Promise( ( resolve, reject ) => {
const hasFinished = () =>
selectors.hasFinishedResolution( selectorName, args );
const finalize = ( result ) => {
const hasFailed = selectors.hasResolutionFailed(
selectorName,
args
);
if ( hasFailed ) {
const error = selectors.getResolutionError(
selectorName,
args
);
reject( error );
} else {
resolve( result );
}
};
const getResult = () => selector.apply( null, args );
// trigger the selector (to trigger the resolver)
const result = getResult();
if ( hasFinished() ) {
return resolve( result );
return finalize( result );
}

const unsubscribe = store.subscribe( () => {
if ( hasFinished() ) {
unsubscribe();
resolve( getResult() );
finalize( getResult() );
}
} );
} )
Expand Down Expand Up @@ -413,15 +428,28 @@ function mapResolvers( resolvers, selectors, store, resolversCache ) {
store.dispatch(
metadataActions.startResolution( selectorName, args )
);
await fulfillResolver(
store,
mappedResolvers,
selectorName,
...args
);
store.dispatch(
metadataActions.finishResolution( selectorName, args )
);
try {
await fulfillResolver(
store,
mappedResolvers,
selectorName,
...args
);
store.dispatch(
metadataActions.finishResolution(
selectorName,
args
)
);
} catch ( error ) {
store.dispatch(
metadataActions.failResolution(
selectorName,
args,
error
)
);
}
} );
}

Expand Down
47 changes: 43 additions & 4 deletions packages/data/src/redux-store/metadata/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,32 @@ export function finishResolution( selectorName, args ) {
};
}

/**
* Returns an action object used in signalling that selector resolution has
* failed.
*
* @param {string} selectorName Name of selector for which resolver triggered.
* @param {unknown[]} args Arguments to associate for uniqueness.
* @param {Error|unknown} error The error that caused the failure.
*
* @return {{ type: 'FAIL_RESOLUTION', selectorName: string, args: unknown[], error: Error|unknown }} Action object.
*/
export function failResolution( selectorName, args, error ) {
return {
type: 'FAIL_RESOLUTION',
selectorName,
args,
error,
};
}

/**
* Returns an action object used in signalling that a batch of selector resolutions has
* started.
*
* @param {string} selectorName Name of selector for which resolver triggered.
* @param {string} selectorName Name of selector for which resolver triggered.
* @param {unknown[][]} args Array of arguments to associate for uniqueness, each item
* is associated to a resolution.
* is associated to a resolution.
*
* @return {{ type: 'START_RESOLUTIONS', selectorName: string, args: unknown[][] }} Action object.
*/
Expand All @@ -54,9 +73,9 @@ export function startResolutions( selectorName, args ) {
* Returns an action object used in signalling that a batch of selector resolutions has
* completed.
*
* @param {string} selectorName Name of selector for which resolver triggered.
* @param {string} selectorName Name of selector for which resolver triggered.
* @param {unknown[][]} args Array of arguments to associate for uniqueness, each item
* is associated to a resolution.
* is associated to a resolution.
*
* @return {{ type: 'FINISH_RESOLUTIONS', selectorName: string, args: unknown[][] }} Action object.
*/
Expand All @@ -68,6 +87,26 @@ export function finishResolutions( selectorName, args ) {
};
}

/**
* Returns an action object used in signalling that a batch of selector resolutions has
* completed and at least one of them has failed.
*
* @param {string} selectorName Name of selector for which resolver triggered.
* @param {unknown[]} args Array of arguments to associate for uniqueness, each item
* is associated to a resolution.
* @param {(Error|unknown)[]} errors Array of errors to associate for uniqueness, each item
* is associated to a resolution.
* @return {{ type: 'FAIL_RESOLUTIONS', selectorName: string, args: unknown[], errors: Array<Error|unknown> }} Action object.
*/
export function failResolutions( selectorName, args, errors ) {
return {
type: 'FAIL_RESOLUTIONS',
selectorName,
args,
errors,
};
}

/**
* Returns an action object used in signalling that we should invalidate the resolution cache.
*
Expand Down
69 changes: 60 additions & 9 deletions packages/data/src/redux-store/metadata/reducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,23 @@ import { selectorArgsToStateKey, onSubKey } from './utils';
type Action =
| ReturnType< typeof import('./actions').startResolution >
| ReturnType< typeof import('./actions').finishResolution >
| ReturnType< typeof import('./actions').failResolution >
| ReturnType< typeof import('./actions').startResolutions >
| ReturnType< typeof import('./actions').finishResolutions >
| ReturnType< typeof import('./actions').failResolutions >
| ReturnType< typeof import('./actions').invalidateResolution >
| ReturnType< typeof import('./actions').invalidateResolutionForStore >
| ReturnType<
typeof import('./actions').invalidateResolutionForStoreSelector
>;

export type State = EquivalentKeyMap< unknown[] | unknown, boolean >;
type StateKey = unknown[] | unknown;
export type StateValue =
| { status: 'resolving' | 'finished' }
| { status: 'error'; error: Error | unknown };

export type Status = StateValue[ 'status' ];
export type State = EquivalentKeyMap< StateKey, StateValue >;

/**
* Reducer function returning next state for selector resolution of
Expand All @@ -34,23 +42,64 @@ const subKeysIsResolved: Reducer< Record< string, State >, Action > = onSubKey<
Action
>( 'selectorName' )( ( state = new EquivalentKeyMap(), action: Action ) => {
switch ( action.type ) {
case 'START_RESOLUTION':
case 'START_RESOLUTION': {
const nextState = new EquivalentKeyMap( state );
nextState.set( selectorArgsToStateKey( action.args ), {
status: 'resolving',
} );
return nextState;
}
case 'FINISH_RESOLUTION': {
const isStarting = action.type === 'START_RESOLUTION';
const nextState = new EquivalentKeyMap( state );
nextState.set( selectorArgsToStateKey( action.args ), isStarting );
nextState.set( selectorArgsToStateKey( action.args ), {
status: 'finished',
} );
return nextState;
}
case 'FAIL_RESOLUTION': {
const nextState = new EquivalentKeyMap( state );
nextState.set( selectorArgsToStateKey( action.args ), {
status: 'error',
error: action.error,
} );
return nextState;
}
case 'START_RESOLUTIONS': {
const nextState = new EquivalentKeyMap( state );
for ( const resolutionArgs of action.args ) {
nextState.set( selectorArgsToStateKey( resolutionArgs ), {
status: 'resolving',
} );
}
return nextState;
}
case 'START_RESOLUTIONS':
case 'FINISH_RESOLUTIONS': {
const isStarting = action.type === 'START_RESOLUTIONS';
const nextState = new EquivalentKeyMap( state );
for ( const resolutionArgs of action.args ) {
nextState.set( selectorArgsToStateKey( resolutionArgs ), {
status: 'finished',
} );
}
return nextState;
}
case 'FAIL_RESOLUTIONS': {
const nextState = new EquivalentKeyMap( state );
action.args.forEach( ( resolutionArgs, idx ) => {
const resolutionState: StateValue = {
status: 'error',
error: undefined,
};

const error = action.errors[ idx ];
if ( error ) {
resolutionState.error = error;
}

nextState.set(
selectorArgsToStateKey( resolutionArgs ),
isStarting
selectorArgsToStateKey( resolutionArgs as unknown[] ),
resolutionState
);
}
} );
return nextState;
}
case 'INVALIDATE_RESOLUTION': {
Expand Down Expand Up @@ -82,8 +131,10 @@ const isResolved = ( state: Record< string, State > = {}, action: Action ) => {
: state;
case 'START_RESOLUTION':
case 'FINISH_RESOLUTION':
case 'FAIL_RESOLUTION':
case 'START_RESOLUTIONS':
case 'FINISH_RESOLUTIONS':
case 'FAIL_RESOLUTIONS':
case 'INVALIDATE_RESOLUTION':
return subKeysIsResolved( state, action );
}
Expand Down
67 changes: 60 additions & 7 deletions packages/data/src/redux-store/metadata/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@ import { get } from 'lodash';
import { selectorArgsToStateKey } from './utils';

/** @typedef {Record<string, import('./reducer').State>} State */
/** @typedef {import('./reducer').StateValue} StateValue */
/** @typedef {import('./reducer').Status} Status */

/**
* Returns the raw `isResolving` value for a given selector name,
* Returns the raw resolution state value for a given selector name,
* and arguments set. May be undefined if the selector has never been resolved
* or not resolved for the given set of arguments, otherwise true or false for
* resolution started and completed respectively.
Expand All @@ -20,17 +22,35 @@ import { selectorArgsToStateKey } from './utils';
* @param {string} selectorName Selector name.
* @param {unknown[]?} args Arguments passed to selector.
*
* @return {boolean | undefined} isResolving value.
* @return {StateValue|undefined} isResolving value.
*/
export function getIsResolving( state, selectorName, args ) {
export function getResolutionState( state, selectorName, args ) {
adamziel marked this conversation as resolved.
Show resolved Hide resolved
const map = get( state, [ selectorName ] );
if ( ! map ) {
return undefined;
return;
}

return map.get( selectorArgsToStateKey( args ) );
}

/**
* Returns the raw `isResolving` value for a given selector name,
* and arguments set. May be undefined if the selector has never been resolved
* or not resolved for the given set of arguments, otherwise true or false for
* resolution started and completed respectively.
*
* @param {State} state Data state.
* @param {string} selectorName Selector name.
* @param {unknown[]?} args Arguments passed to selector.
*
* @return {boolean | undefined} isResolving value.
*/
export function getIsResolving( state, selectorName, args ) {
const resolutionState = getResolutionState( state, selectorName, args );

return resolutionState && resolutionState.status === 'resolving';
}

/**
* Returns true if resolution has already been triggered for a given
* selector name, and arguments set.
Expand All @@ -42,7 +62,7 @@ export function getIsResolving( state, selectorName, args ) {
* @return {boolean} Whether resolution has been triggered.
*/
export function hasStartedResolution( state, selectorName, args ) {
return getIsResolving( state, selectorName, args ) !== undefined;
return getResolutionState( state, selectorName, args ) !== undefined;
}

/**
Expand All @@ -56,7 +76,38 @@ export function hasStartedResolution( state, selectorName, args ) {
* @return {boolean} Whether resolution has completed.
*/
export function hasFinishedResolution( state, selectorName, args ) {
return getIsResolving( state, selectorName, args ) === false;
const status = getResolutionState( state, selectorName, args )?.status;
return status === 'finished' || status === 'error';
}

/**
* Returns true if resolution has failed for a given selector
* name, and arguments set.
*
* @param {State} state Data state.
* @param {string} selectorName Selector name.
* @param {unknown[]?} args Arguments passed to selector.
*
* @return {boolean} Has resolution failed
*/
export function hasResolutionFailed( state, selectorName, args ) {
return getResolutionState( state, selectorName, args )?.status === 'error';
}

/**
* Returns the resolution error for a given selector name, and arguments set.
* Note it may be of an Error type, but may also be null, undefined, or anything else
* that can be `throw`-n.
*
* @param {State} state Data state.
* @param {string} selectorName Selector name.
* @param {unknown[]?} args Arguments passed to selector.
*
* @return {Error|unknown} Last resolution error
*/
export function getResolutionError( state, selectorName, args ) {
const resolutionState = getResolutionState( state, selectorName, args );
return resolutionState?.status === 'error' ? resolutionState.error : null;
}

/**
Expand All @@ -70,7 +121,9 @@ export function hasFinishedResolution( state, selectorName, args ) {
* @return {boolean} Whether resolution is in progress.
*/
export function isResolving( state, selectorName, args ) {
return getIsResolving( state, selectorName, args ) === true;
return (
getResolutionState( state, selectorName, args )?.status === 'resolving'
);
}

/**
Expand Down
Loading