From 501640facbaea35cecb1d17872c724ebed687d70 Mon Sep 17 00:00:00 2001 From: Dave Falke Date: Thu, 11 Jul 2024 10:57:36 -0400 Subject: [PATCH] Eda study data race condition (#1132) * Add options to usePromise hook * Use new options of usePromise hook, to prevent inconsistent data * Return the `value` property of `usePromise` result (cherry picked from commit f2a73f9ea289dadecb684b8a1e2041b1d54aca4a) --- .../components/EDAAnalysisListContainer.tsx | 2 +- .../core/components/EDAWorkspaceContainer.tsx | 5 +++-- .../libs/eda/src/lib/core/hooks/promise.ts | 18 +++++++++++++++--- packages/libs/eda/src/lib/core/hooks/study.ts | 10 ++++++---- .../js/client/routes/userDatasetRoutes.tsx | 2 +- 5 files changed, 26 insertions(+), 11 deletions(-) diff --git a/packages/libs/eda/src/lib/core/components/EDAAnalysisListContainer.tsx b/packages/libs/eda/src/lib/core/components/EDAAnalysisListContainer.tsx index b81d12f480..456796f841 100644 --- a/packages/libs/eda/src/lib/core/components/EDAAnalysisListContainer.tsx +++ b/packages/libs/eda/src/lib/core/components/EDAAnalysisListContainer.tsx @@ -40,7 +40,7 @@ export function EDAAnalysisListContainer(props: Props) { const studyMetadata = useStudyMetadata(studyId, subsettingClient); const contextValue = useDeepValue({ ...studyRecordState, - studyMetadata, + studyMetadata: studyMetadata.value, analysisClient, subsettingClient, dataClient, diff --git a/packages/libs/eda/src/lib/core/components/EDAWorkspaceContainer.tsx b/packages/libs/eda/src/lib/core/components/EDAWorkspaceContainer.tsx index 9fcb54da47..9b8d1c2431 100644 --- a/packages/libs/eda/src/lib/core/components/EDAWorkspaceContainer.tsx +++ b/packages/libs/eda/src/lib/core/components/EDAWorkspaceContainer.tsx @@ -44,12 +44,13 @@ export function EDAWorkspaceContainer(props: Props) { const wdkStudyRecordState = useWdkStudyRecord(studyId); const studyMetadata = useStudyMetadata(studyId, subsettingClient); - if (wdkStudyRecordState == null || studyMetadata == null) return ; + if (wdkStudyRecordState == null || studyMetadata.value == null) + return ; return ( ); } diff --git a/packages/libs/eda/src/lib/core/hooks/promise.ts b/packages/libs/eda/src/lib/core/hooks/promise.ts index a7ccef4692..b4ff11326d 100644 --- a/packages/libs/eda/src/lib/core/hooks/promise.ts +++ b/packages/libs/eda/src/lib/core/hooks/promise.ts @@ -6,6 +6,11 @@ export type PromiseHookState = { error?: unknown; }; +export type PromiseHookOptions = { + keepPreviousValue?: boolean; + throwError?: boolean; +}; + /** * Invokes `task` and returns an object representing its current state and resolved value. * The last resolved value will remain util a new promise is resolved. @@ -15,7 +20,11 @@ export type PromiseHookState = { * @param task A function that returns a promise * @returns PromiseHookState */ -export function usePromise(task: () => Promise): PromiseHookState { +export function usePromise( + task: () => Promise, + options: PromiseHookOptions = {} +): PromiseHookState { + const { keepPreviousValue = true, throwError = false } = options; const [state, setState] = useState>({ pending: true, }); @@ -23,7 +32,7 @@ export function usePromise(task: () => Promise): PromiseHookState { let ignoreResolve = false; setState((prev) => ({ pending: true, - value: prev.value, + value: keepPreviousValue ? prev.value : undefined, error: undefined, })); task().then( @@ -36,6 +45,9 @@ export function usePromise(task: () => Promise): PromiseHookState { }, (error) => { if (ignoreResolve) return; + if (throwError) { + throw error; + } setState({ error, pending: false, @@ -45,6 +57,6 @@ export function usePromise(task: () => Promise): PromiseHookState { return function cleanup() { ignoreResolve = true; }; - }, [task]); + }, [keepPreviousValue, task, throwError]); return state; } diff --git a/packages/libs/eda/src/lib/core/hooks/study.ts b/packages/libs/eda/src/lib/core/hooks/study.ts index 9beabef9d6..647763e61c 100644 --- a/packages/libs/eda/src/lib/core/hooks/study.ts +++ b/packages/libs/eda/src/lib/core/hooks/study.ts @@ -186,7 +186,7 @@ export function isStubEntity(entity: StudyEntity) { export function useStudyMetadata(datasetId: string, client: SubsettingClient) { const permissionsResponse = usePermissions(); - const { error, value } = usePromise( + return usePromise( useCallback(async () => { if (permissionsResponse.loading) return; const { permissions } = permissionsResponse; @@ -208,8 +208,10 @@ export function useStudyMetadata(datasetId: string, client: SubsettingClient) { } throw error; } - }, [client, datasetId, permissionsResponse]) + }, [client, datasetId, permissionsResponse]), + { + keepPreviousValue: false, + throwError: true, + } ); - if (error) throw error; - return value; } diff --git a/packages/sites/clinepi-site/webapp/js/client/routes/userDatasetRoutes.tsx b/packages/sites/clinepi-site/webapp/js/client/routes/userDatasetRoutes.tsx index a0fb273a04..5644928db4 100644 --- a/packages/sites/clinepi-site/webapp/js/client/routes/userDatasetRoutes.tsx +++ b/packages/sites/clinepi-site/webapp/js/client/routes/userDatasetRoutes.tsx @@ -102,7 +102,7 @@ export const userDatasetRoutes: RouteEntry[] = [ function useEdaStudyMetadata(wdkDatasetId: string) { try { const subsettingClient = useConfiguredSubsettingClient(edaServiceUrl); - return useStudyMetadata(wdkDatasetId, subsettingClient); + return useStudyMetadata(wdkDatasetId, subsettingClient).value; } catch (error) { console.error(error); return undefined;