[26.0] Fix infinite request loop on dataset preview fetch failure

client/src/api/datasets.ts

@@ -8,7 +8,7 @@ import {
     type HDADetailed,
 } from "@/api";
 import { withPrefix } from "@/utils/redirect";
-import { rethrowSimple } from "@/utils/simple-error";
+import { rethrowSimple, rethrowSimpleWithStatus } from "@/utils/simple-error";
 
 export async function fetchDatasetTextContentDetails(params: { id: string }): Promise<DatasetTextContentDetails> {
     const { data, error } = await GalaxyApi().GET("/api/datasets/{dataset_id}/get_content_as_text", {
@@ -26,7 +26,7 @@ export async function fetchDatasetTextContentDetails(params: { id: string }): Pr
 }
 
 export async function fetchDatasetDetails(params: { id: string }, view: string = "detailed"): Promise<HDADetailed> {
-    const { data, error } = await GalaxyApi().GET("/api/datasets/{dataset_id}", {
+    const { data, error, response } = await GalaxyApi().GET("/api/datasets/{dataset_id}", {
         params: {
             path: {
                 dataset_id: params.id,
@@ -36,7 +36,7 @@ export async function fetchDatasetDetails(params: { id: string }, view: string =
     });
 
     if (error) {
-        rethrowSimple(error);
+        rethrowSimpleWithStatus(error, response);
     }
     return data as HDADetailed;
 }

client/src/components/Dataset/DatasetView.vue

@@ -42,6 +42,7 @@ const props = withDefaults(defineProps<Props>(), {
 const iframeLoading = ref(true);
 
 const dataset = computed(() => datasetStore.getDataset(props.datasetId));
+const loadError = computed(() => datasetStore.getDatasetError(props.datasetId));
 const downloadUrl = computed(() => withPrefix(`/datasets/${props.datasetId}/display`));
 const headerState = computed(() => (headerCollapsed.value ? "closed" : "open"));
 
@@ -102,7 +103,16 @@ watch(
 </script>
 
 <template>
-    <LoadingSpan v-if="isLoading || !dataset" message="Loading dataset details" />
+    <div v-if="loadError" class="alert alert-danger m-4">
+        <h4 class="alert-heading">Dataset Not Available</h4>
+        <p>
+            {{
+                loadError.message ||
+                "This dataset could not be loaded. It may not exist or you may not have permission to access it."
+            }}
+        </p>
+    </div>
+    <LoadingSpan v-else-if="isLoading || !dataset" message="Loading dataset details" />
     <div v-else class="dataset-view d-flex flex-column">
         <header v-if="!displayOnly" :key="`dataset-header-${dataset.id}`" class="dataset-header flex-shrink-0">
             <div class="d-flex">

client/src/composables/keyedCache.test.ts

@@ -2,6 +2,8 @@ import flushPromises from "flush-promises";
 import { beforeEach, describe, expect, it, vi } from "vitest";
 import { computed, ref } from "vue";
 
+import { ApiError } from "@/utils/simple-error";
+
 import { useKeyedCache } from "./keyedCache";
 
 interface ItemData {
@@ -193,6 +195,85 @@ describe("useKeyedCache", () => {
         expect(shouldFetch).toHaveBeenCalled();
     });
 
+    it("should not re-fetch after a failed request", async () => {
+        const id = "1";
+
+        fetchItem.mockRejectedValue(new Error("Request failed"));
+
+        const { getItemById, getItemLoadError, isLoadingItem } = useKeyedCache<ItemData>(fetchItem);
+
+        getItemById.value(id);
+        await flushPromises();
+
+        expect(isLoadingItem.value(id)).toBeFalsy();
+        expect(getItemLoadError.value(id)).toBeInstanceOf(Error);
+        expect(fetchItem).toHaveBeenCalledTimes(1);
+
+        // Calling getItemById again should not trigger another fetch
+        getItemById.value(id);
+        await flushPromises();
+
+        expect(fetchItem).toHaveBeenCalledTimes(1);
+    });
+
+    it("should retry on transient errors (429, 5xx) up to max retries", async () => {
+        const id = "1";
+
+        fetchItem.mockRejectedValue(new ApiError("Too Many Requests", 429));
+
+        const { getItemById, getItemLoadError } = useKeyedCache<ItemData>(fetchItem);
+
+        // First attempt + MAX_RETRIES - 1 retries (first call counts as attempt 1)
+        for (let i = 1; i <= 3; i++) {
+            getItemById.value(id);
+            await flushPromises();
+            expect(fetchItem).toHaveBeenCalledTimes(i);
+            expect(getItemLoadError.value(id)).toBeInstanceOf(ApiError);
+        }
+
+        // Should stop after max retries exhausted
+        getItemById.value(id);
+        await flushPromises();
+        expect(fetchItem).toHaveBeenCalledTimes(3);
+    });
+
+    it("should not retry on permanent errors (403, 404)", async () => {
+        const id = "1";
+
+        fetchItem.mockRejectedValue(new ApiError("Forbidden", 403));
+
+        const { getItemById, getItemLoadError } = useKeyedCache<ItemData>(fetchItem);
+
+        getItemById.value(id);
+        await flushPromises();
+        expect(fetchItem).toHaveBeenCalledTimes(1);
+        expect(getItemLoadError.value(id)).toBeInstanceOf(ApiError);
+
+        getItemById.value(id);
+        await flushPromises();
+        expect(fetchItem).toHaveBeenCalledTimes(1);
+    });
+
+    it("should recover on retry if transient error resolves", async () => {
+        const id = "1";
+        const item = { id, name: "Item 1" };
+
+        fetchItem.mockRejectedValueOnce(new ApiError("Service Unavailable", 503)).mockResolvedValueOnce(item);
+
+        const { getItemById, storedItems, getItemLoadError } = useKeyedCache<ItemData>(fetchItem);
+
+        getItemById.value(id);
+        await flushPromises();
+        expect(fetchItem).toHaveBeenCalledTimes(1);
+        expect(getItemLoadError.value(id)).toBeInstanceOf(ApiError);
+
+        // Retry should succeed
+        getItemById.value(id);
+        await flushPromises();
+        expect(fetchItem).toHaveBeenCalledTimes(2);
+        expect(storedItems.value[id]).toEqual(item);
+    });
+
     it("should handle fake timers without hanging when advanced manually", async () => {
         vi.useFakeTimers();
         const id = "1";

client/src/composables/keyedCache.ts

@@ -2,6 +2,7 @@ import { type MaybeRefOrGetter, toValue } from "@vueuse/core";
 import { computed, del, type Ref, ref, set, unref } from "vue";
 
 import { LastQueue } from "@/utils/lastQueue";
+import { ApiError } from "@/utils/simple-error";
 
 /**
  * Parameters for fetching an item from the server.
@@ -29,6 +30,16 @@ type ShouldFetchHandler<T> = (item?: T) => boolean;
  */
 const fetchIfAbsent = <T>(item?: T) => item === undefined;
 
+const RETRYABLE_STATUSES = new Set([429, 500, 502, 503, 504]);
+const MAX_RETRIES = 3;
+
+function isRetryableError(error: Error): boolean {
+    if (error instanceof ApiError && error.status !== undefined) {
+        return RETRYABLE_STATUSES.has(error.status);
+    }
+    return false;
+}
+
 /**
  * A composable that provides a simple key-value cache for items fetched from the server.
  *
@@ -45,6 +56,7 @@ export function useKeyedCache<T>(
 ) {
     const storedItems = ref<{ [key: string]: T }>({});
     const loadingErrors = ref<{ [key: string]: Error }>({});
+    const retryCounts: { [key: string]: number } = {};
 
     const loadingRequests = ref<{ [key: string]: Promise<T | undefined> }>({});
 
@@ -53,7 +65,9 @@ export function useKeyedCache<T>(
     const getItemById = computed(() => {
         return (id: string) => {
             const item = storedItems.value[id];
-            if (shouldFetch(item)) {
+            const existingError = loadingErrors.value[id];
+            const canRetry = existingError && isRetryableError(existingError) && (retryCounts[id] ?? 0) < MAX_RETRIES;
+            if (shouldFetch(item) && (!existingError || canRetry)) {
                 fetchItemById({ id: id });
             }
             return item ?? null;
@@ -91,8 +105,10 @@ export function useKeyedCache<T>(
                 const fetchItem = unref(fetchItemHandler);
                 const item = await fetchQueue.enqueue(fetchItem, { id: itemId }, itemId);
                 set(storedItems.value, itemId, item);
+                delete retryCounts[itemId];
                 return item;
             } catch (error) {
+                retryCounts[itemId] = (retryCounts[itemId] ?? 0) + 1;
                 set(loadingErrors.value, itemId, error as Error);
             } finally {
                 del(loadingRequests.value, itemId);

client/src/utils/simple-error.ts

@@ -24,3 +24,18 @@ export function rethrowSimple(e: any): never {
     }
     throw Error(errorMessageAsString(e));
 }
+
+export class ApiError extends Error {
+    status?: number;
+    constructor(message: string, status?: number) {
+        super(message);
+        this.status = status;
+    }
+}
+
+export function rethrowSimpleWithStatus(e: any, response?: { status: number }): never {
+    if (process.env.NODE_ENV != "test") {
+        console.debug(e);
+    }
+    throw new ApiError(errorMessageAsString(e), response?.status);
+}