Fix error handling in keyedCache and collectionEditView

We were previously not showing error message at all if the user didn't have permissions to edit a collection (or fetching a collection has failed for another reason).
galaxyproject · Sep 2, 2024 · 59607f1 · 59607f1
1 parent b10e8f8
commit 59607f1
Show file tree

Hide file tree

Showing 5 changed files with 62 additions and 11 deletions.
diff --git a/client/src/components/Collections/common/CollectionEditView.vue b/client/src/components/Collections/common/CollectionEditView.vue
@@ -55,9 +55,26 @@ const collectionChangeKey = ref(0);
 const attributesData = computed(() => {
     return collectionAttributesStore.getAttributes(props.collectionId);
 });
+const attributesLoadError = computed(() =>
+    errorMessageAsString(collectionAttributesStore.hasItemLoadError(props.collectionId))
+);
+
 const collection = computed(() => {
     return collectionStore.getCollectionById(props.collectionId);
 });
+const collectionLoadError = computed(() => {
+    if (collection.value) {
+        return errorMessageAsString(collectionStore.hasLoadingCollectionElementsError(collection.value));
+    }
+    return "";
+});
+watch([attributesLoadError, collectionLoadError], () => {
+    if (attributesLoadError.value) {
+        errorMessage.value = attributesLoadError.value;
+    } else if (collectionLoadError.value) {
+        errorMessage.value = collectionLoadError.value;
+    }
+});
 const databaseKeyFromElements = computed(() => {
     return attributesData.value?.dbkey;
 });
@@ -101,7 +118,7 @@ async function clickedSave(attribute: string, newValue: any) {
     try {
         await copyCollection(props.collectionId, dbKey);
     } catch (err) {
-        errorMessage.value = errorMessageAsString(err, "History import failed.");
+        errorMessage.value = errorMessageAsString(err, `Changing ${attribute} failed`);
     }
 }
 
@@ -119,7 +136,7 @@ async function clickedConvert(selectedConverter: any) {
         await axios.post(url, data).catch(handleError);
         successMessage.value = "Conversion started successfully.";
     } catch (err) {
-        errorMessage.value = errorMessageAsString(err, "History import failed.");
+        errorMessage.value = errorMessageAsString(err, "Conversion failed.");
     }
 }
 
@@ -143,12 +160,12 @@ async function clickedDatatypeChange(selectedDatatype: any) {
         await updateHistoryItemsBulk(currentHistoryId.value ?? "", data);
         successMessage.value = "Datatype changed successfully.";
     } catch (err) {
-        errorMessage.value = errorMessageAsString(err, "History import failed.");
+        errorMessage.value = errorMessageAsString(err, "Datatype change failed.");
     }
 }
 
 function handleError(err: any) {
-    errorMessage.value = errorMessageAsString(err, "History import failed.");
+    errorMessage.value = errorMessageAsString(err, "Datatype conversion failed.");
 
     if (err?.data?.stderr) {
         jobError.value = err.data;
@@ -191,14 +208,14 @@ async function saveAttrs() {
             {{ localize(infoMessage) }}
         </BAlert>
 
-        <BAlert v-if="jobError" show variant="danger" dismissible>
+        <BAlert v-if="errorMessage" show variant="danger">
             {{ localize(errorMessage) }}
         </BAlert>
 
         <BAlert v-if="successMessage" show variant="success" dismissible>
             {{ localize(successMessage) }}
         </BAlert>
-        <BTabs class="mt-3">
+        <BTabs v-if="!errorMessage" class="mt-3">
             <BTab title-link-class="collection-edit-attributes-nav" @click="updateInfoMessage('')">
                 <template v-slot:title>
                     <FontAwesomeIcon :icon="faBars" class="mr-1" />

diff --git a/client/src/components/History/CurrentCollection/CollectionPanel.vue b/client/src/components/History/CurrentCollection/CollectionPanel.vue
@@ -8,10 +8,12 @@ import { canMutateHistory, isCollectionElement, isHDCA } from "@/api";
 import ExpandedItems from "@/components/History/Content/ExpandedItems";
 import { updateContentFields } from "@/components/History/model/queries";
 import { useCollectionElementsStore } from "@/stores/collectionElementsStore";
+import { errorMessageAsString } from "@/utils/simple-error";
 
 import CollectionDetails from "./CollectionDetails.vue";
 import CollectionNavigation from "./CollectionNavigation.vue";
 import CollectionOperations from "./CollectionOperations.vue";
+import Alert from "@/components/Alert.vue";
 import ContentItem from "@/components/History/Content/ContentItem.vue";
 import ListingLayout from "@/components/History/Layout/ListingLayout.vue";
 
@@ -54,6 +56,7 @@ watch(
 
 const collectionElements = computed(() => collectionElementsStore.getCollectionElements(dsc.value) ?? []);
 const loading = computed(() => collectionElementsStore.isLoadingCollectionElements(dsc.value));
+const error = computed(() => collectionElementsStore.hasLoadingCollectionElementsError(dsc.value));
 const jobState = computed(() => ("job_state_summary" in dsc.value ? dsc.value.job_state_summary : undefined));
 const populatedStateMsg = computed(() =>
     "populated_state_message" in dsc.value ? dsc.value.populated_state_message : undefined
@@ -118,7 +121,10 @@ watch(
 </script>
 
 <template>
-    <ExpandedItems v-slot="{ isExpanded, setExpanded }" :scope-key="dsc.id" :get-item-key="getItemKey">
+    <Alert v-if="error" variant="error">
+        {{ errorMessageAsString(error) }}
+    </Alert>
+    <ExpandedItems v-else v-slot="{ isExpanded, setExpanded }" :scope-key="dsc.id" :get-item-key="getItemKey">
         <section class="dataset-collection-panel w-100 d-flex flex-column">
             <section>
                 <CollectionNavigation

diff --git a/client/src/composables/keyedCache.ts b/client/src/composables/keyedCache.ts
@@ -45,6 +45,7 @@ export function useKeyedCache<T>(
 ) {
     const storedItems = ref<{ [key: string]: T }>({});
     const loadingItem = ref<{ [key: string]: boolean }>({});
+    const loadingErrors = ref<{ [key: string]: Error }>({});
 
     const getItemById = computed(() => {
         return (id: string) => {
@@ -69,10 +70,17 @@ export function useKeyedCache<T>(
         };
     });
 
+    const hasItemLoadError = computed(() => {
+        return (id: string) => {
+            return loadingErrors.value[id] ?? null;
+        };
+    });
+
     async function fetchItemById(params: FetchParams) {
         const itemId = params.id;
         const isAlreadyLoading = loadingItem.value[itemId] ?? false;
-        if (isAlreadyLoading) {
+        const failedLoading = loadingErrors.value[itemId];
+        if (isAlreadyLoading || failedLoading) {
             return;
         }
         set(loadingItem.value, itemId, true);
@@ -81,6 +89,8 @@ export function useKeyedCache<T>(
             const { data } = await fetchItem({ id: itemId });
             set(storedItems.value, itemId, data);
             return data;
+        } catch (error) {
+            set(loadingErrors.value, itemId, error);
         } finally {
             del(loadingItem.value, itemId);
         }
@@ -100,6 +110,10 @@ export function useKeyedCache<T>(
         /**
          * A computed function that returns true if the item with the given id is currently being fetched.
          */
+        hasItemLoadError,
+        /**
+         * A computed function holding errors
+         */
         isLoadingItem,
         /**
          * Fetches the item with the given id from the server.

diff --git a/client/src/stores/collectionAttributesStore.ts b/client/src/stores/collectionAttributesStore.ts
@@ -5,13 +5,14 @@ import { fetchCollectionAttributes } from "@/api/datasetCollections";
 import { useKeyedCache } from "@/composables/keyedCache";
 
 export const useCollectionAttributesStore = defineStore("collectionAttributesStore", () => {
-    const { storedItems, getItemById, isLoadingItem } = useKeyedCache<DatasetCollectionAttributes>((params) =>
-        fetchCollectionAttributes({ id: params.id, instance_type: "history" })
+    const { storedItems, getItemById, isLoadingItem, hasItemLoadError } = useKeyedCache<DatasetCollectionAttributes>(
+        (params) => fetchCollectionAttributes({ id: params.id, instance_type: "history" })
     );
 
     return {
         storedAttributes: storedItems,
         getAttributes: getItemById,
         isLoadingAttributes: isLoadingItem,
+        hasItemLoadError: hasItemLoadError,
     };
 });
diff --git a/client/src/stores/collectionElementsStore.ts b/client/src/stores/collectionElementsStore.ts
@@ -37,6 +37,7 @@ const FETCH_LIMIT = 50;
 export const useCollectionElementsStore = defineStore("collectionElementsStore", () => {
     const storedCollections = ref<{ [key: string]: HDCASummary }>({});
     const loadingCollectionElements = ref<{ [key: string]: boolean }>({});
+    const loadingCollectionElementsErrors = ref<{ [key: string]: Error }>({});
     const storedCollectionElements = ref<{ [key: string]: DCEEntry[] }>({});
 
     /**
@@ -63,6 +64,12 @@ export const useCollectionElementsStore = defineStore("collectionElementsStore",
         };
     });
 
+    const hasLoadingCollectionElementsError = computed(() => {
+        return (collection: CollectionEntry) => {
+            return loadingCollectionElementsErrors.value[getCollectionKey(collection) ?? false];
+        };
+    });
+
     type FetchParams = {
         storedElements: DCEEntry[];
         collection: CollectionEntry;
@@ -106,6 +113,8 @@ export const useCollectionElementsStore = defineStore("collectionElementsStore",
             });
 
             return { fetchedElements, elementOffset: offset };
+        } catch (error) {
+            set(loadingCollectionElementsErrors.value, collectionKey, error);
         } finally {
             del(loadingCollectionElements.value, collectionKey);
         }
@@ -162,7 +171,7 @@ export const useCollectionElementsStore = defineStore("collectionElementsStore",
     /** Returns collection from storedCollections, will load collection if not in store */
     const getCollectionById = computed(() => {
         return (collectionId: string) => {
-            if (!storedCollections.value[collectionId]) {
+            if (!storedCollections.value[collectionId] && !loadingCollectionElementsErrors.value[collectionId]) {
                 // TODO: Try to remove this as it can cause computed side effects (use keyedCache in this store instead?)
                 fetchCollection({ id: collectionId });
             }
@@ -176,6 +185,8 @@ export const useCollectionElementsStore = defineStore("collectionElementsStore",
             const collection = await fetchCollectionDetails({ id: params.id });
             set(storedCollections.value, collection.id, collection);
             return collection;
+        } catch (error) {
+            set(loadingCollectionElementsErrors.value, params.id, error);
         } finally {
             del(loadingCollectionElements.value, params.id);
         }
@@ -203,6 +214,8 @@ export const useCollectionElementsStore = defineStore("collectionElementsStore",
         storedCollectionElements,
         getCollectionElements,
         isLoadingCollectionElements,
+        hasLoadingCollectionElementsError,
+        loadingCollectionElementsErrors,
         getCollectionById,
         fetchCollection,
         invalidateCollectionElements,