From 7f8dcd75bb0cab40f27d0a8856e0436639d7dd22 Mon Sep 17 00:00:00 2001 From: Dannon Baker Date: Tue, 24 Feb 2026 12:53:01 -0500 Subject: [PATCH 1/7] Add ApiError and retry-aware error handling to keyedCache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a useKeyedCache fetch handler fails, getItemById previously saw undefined in storedItems and re-triggered the fetch in an infinite loop. Now the cache checks loadingErrors before initiating a new fetch — non- retryable errors (plain Error or 4xx ApiError) permanently block further attempts, while retryable server errors (429, 5xx) allow up to 3 retries before giving up. Adds ApiError class and rethrowSimpleWithStatus helper to simple-error.ts so fetch handlers can preserve HTTP status codes from API responses. --- client/src/composables/keyedCache.test.ts | 65 +++++++++++++++++++++++ client/src/composables/keyedCache.ts | 26 ++++----- 2 files changed, 79 insertions(+), 12 deletions(-) diff --git a/client/src/composables/keyedCache.test.ts b/client/src/composables/keyedCache.test.ts index d4523217f407..6a048c8bf26f 100644 --- a/client/src/composables/keyedCache.test.ts +++ b/client/src/composables/keyedCache.test.ts @@ -292,4 +292,69 @@ describe("useKeyedCache", () => { await flushPromises(); expect(true).toBe(true); }); + + it("should not retry after a plain Error", async () => { + const id = "1"; + fetchItem.mockRejectedValue(new Error("something broke")); + + const { getItemById } = useKeyedCache(fetchItem); + + getItemById.value(id); + await flushPromises(); + expect(fetchItem).toHaveBeenCalledTimes(1); + + // Subsequent calls should not re-fetch + getItemById.value(id); + await flushPromises(); + expect(fetchItem).toHaveBeenCalledTimes(1); + }); + + it("should retry up to 3 times for a retryable ApiError then stop", async () => { + const id = "1"; + fetchItem.mockRejectedValue(new ApiError("rate limited", 429)); + + const { getItemById } = useKeyedCache(fetchItem); + + // Initial fetch + 3 retries = 4 total calls + for (let i = 0; i < 5; i++) { + getItemById.value(id); + await flushPromises(); + } + expect(fetchItem).toHaveBeenCalledTimes(4); + }); + + it("should not retry for a non-retryable ApiError", async () => { + const id = "1"; + fetchItem.mockRejectedValue(new ApiError("forbidden", 403)); + + const { getItemById } = useKeyedCache(fetchItem); + + getItemById.value(id); + await flushPromises(); + expect(fetchItem).toHaveBeenCalledTimes(1); + + getItemById.value(id); + await flushPromises(); + expect(fetchItem).toHaveBeenCalledTimes(1); + }); + + it("should recover after a retryable error followed by success", async () => { + const id = "1"; + const item = { id, name: "Item 1" }; + fetchItem.mockRejectedValueOnce(new ApiError("service unavailable", 503)); + fetchItem.mockResolvedValue(item); + + const { getItemById, storedItems, getItemLoadError } = useKeyedCache(fetchItem); + + getItemById.value(id); + await flushPromises(); + expect(fetchItem).toHaveBeenCalledTimes(1); + expect(getItemLoadError.value(id)).toBeTruthy(); + + // Retry should succeed + getItemById.value(id); + await flushPromises(); + expect(fetchItem).toHaveBeenCalledTimes(2); + expect(storedItems.value[id]).toEqual(item); + }); }); diff --git a/client/src/composables/keyedCache.ts b/client/src/composables/keyedCache.ts index 51cd7625f675..708c43430269 100644 --- a/client/src/composables/keyedCache.ts +++ b/client/src/composables/keyedCache.ts @@ -4,6 +4,16 @@ import { computed, del, type Ref, ref, set, unref } from "vue"; import { LastQueue } from "@/utils/lastQueue"; import { ApiError } from "@/utils/simple-error"; +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; +} + /** * Parameters for fetching an item from the server. * @@ -30,16 +40,6 @@ type ShouldFetchHandler = (item?: T) => boolean; */ const fetchIfAbsent = (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. * @@ -56,17 +56,18 @@ export function useKeyedCache( ) { const storedItems = ref<{ [key: string]: T }>({}); const loadingErrors = ref<{ [key: string]: Error }>({}); - const retryCounts: { [key: string]: number } = {}; const loadingRequests = ref<{ [key: string]: Promise }>({}); + const retryCounts: { [key: string]: number } = {}; + const fetchQueue = new LastQueue>(); const getItemById = computed(() => { return (id: string) => { const item = storedItems.value[id]; const existingError = loadingErrors.value[id]; - const canRetry = existingError && isRetryableError(existingError) && (retryCounts[id] ?? 0) < MAX_RETRIES; + const canRetry = existingError && isRetryableError(existingError) && (retryCounts[id] ?? 0) <= MAX_RETRIES; if (shouldFetch(item) && (!existingError || canRetry)) { fetchItemById({ id: id }); } @@ -105,6 +106,7 @@ export function useKeyedCache( const fetchItem = unref(fetchItemHandler); const item = await fetchQueue.enqueue(fetchItem, { id: itemId }, itemId); set(storedItems.value, itemId, item); + del(loadingErrors.value, itemId); delete retryCounts[itemId]; return item; } catch (error) { From f7e404b4e953da1832711f772f22ecfa2736d97f Mon Sep 17 00:00:00 2001 From: Dannon Baker Date: Tue, 24 Feb 2026 13:02:30 -0500 Subject: [PATCH 2/7] Update all useKeyedCache fetch handlers to preserve HTTP status codes Switches the 12 cache-backed fetch handlers across 6 store/api files from rethrowSimple to rethrowSimpleWithStatus so that the keyedCache retry logic can distinguish retryable server errors from permanent client errors. The cancelWorkflowScheduling handler (a DELETE, not a cache handler) keeps using rethrowSimple. --- client/src/api/datasets.ts | 4 +-- .../src/stores/collectionAttributesStore.ts | 6 ++--- client/src/stores/datasetExtraFilesStore.ts | 6 ++--- client/src/stores/invocationStore.ts | 26 +++++++++---------- .../stores/jobDestinationParametersStore.ts | 6 ++--- client/src/stores/jobStore.ts | 6 ++--- 6 files changed, 27 insertions(+), 27 deletions(-) diff --git a/client/src/api/datasets.ts b/client/src/api/datasets.ts index 6640d136854d..8219498194bb 100644 --- a/client/src/api/datasets.ts +++ b/client/src/api/datasets.ts @@ -55,7 +55,7 @@ export async function loadDatasets(options: LoadDatasetsOptions): Promise { - const { data, error } = await GalaxyApi().GET("/api/datasets/{dataset_id}/get_content_as_text", { + const { data, error, response } = await GalaxyApi().GET("/api/datasets/{dataset_id}/get_content_as_text", { params: { path: { dataset_id: params.id, @@ -64,7 +64,7 @@ export async function fetchDatasetTextContentDetails(params: { id: string }): Pr }); if (error) { - rethrowSimple(error); + rethrowSimpleWithStatus(error, response); } return data; } diff --git a/client/src/stores/collectionAttributesStore.ts b/client/src/stores/collectionAttributesStore.ts index e3ba4631bfbb..360468de084e 100644 --- a/client/src/stores/collectionAttributesStore.ts +++ b/client/src/stores/collectionAttributesStore.ts @@ -2,15 +2,15 @@ import { defineStore } from "pinia"; import { type DatasetCollectionAttributes, GalaxyApi } from "@/api"; import { type FetchParams, useKeyedCache } from "@/composables/keyedCache"; -import { rethrowSimple } from "@/utils/simple-error"; +import { rethrowSimpleWithStatus } from "@/utils/simple-error"; export const useCollectionAttributesStore = defineStore("collectionAttributesStore", () => { async function fetchAttributes(params: FetchParams): Promise { - const { data, error } = await GalaxyApi().GET("/api/dataset_collections/{hdca_id}/attributes", { + const { data, error, response } = await GalaxyApi().GET("/api/dataset_collections/{hdca_id}/attributes", { params: { path: { hdca_id: params.id } }, }); if (error) { - rethrowSimple(error); + rethrowSimpleWithStatus(error, response); } return data; } diff --git a/client/src/stores/datasetExtraFilesStore.ts b/client/src/stores/datasetExtraFilesStore.ts index 4e3c796784cd..157831369045 100644 --- a/client/src/stores/datasetExtraFilesStore.ts +++ b/client/src/stores/datasetExtraFilesStore.ts @@ -3,15 +3,15 @@ import { defineStore } from "pinia"; import { GalaxyApi } from "@/api"; import type { DatasetExtraFiles } from "@/api/datasets"; import { type FetchParams, useKeyedCache } from "@/composables/keyedCache"; -import { rethrowSimple } from "@/utils/simple-error"; +import { rethrowSimpleWithStatus } from "@/utils/simple-error"; export const useDatasetExtraFilesStore = defineStore("datasetExtraFilesStore", () => { async function fetchDatasetExtraFiles(params: FetchParams): Promise { - const { data, error } = await GalaxyApi().GET("/api/datasets/{dataset_id}/extra_files", { + const { data, error, response } = await GalaxyApi().GET("/api/datasets/{dataset_id}/extra_files", { params: { path: { dataset_id: params.id } }, }); if (error) { - rethrowSimple(error); + rethrowSimpleWithStatus(error, response); } return data; } diff --git a/client/src/stores/invocationStore.ts b/client/src/stores/invocationStore.ts index 644570ecfc26..a96773b6f7d7 100644 --- a/client/src/stores/invocationStore.ts +++ b/client/src/stores/invocationStore.ts @@ -10,53 +10,53 @@ import type { WorkflowInvocationRequest, } from "@/api/invocations"; import { type FetchParams, useKeyedCache } from "@/composables/keyedCache"; -import { rethrowSimple } from "@/utils/simple-error"; +import { rethrowSimple, rethrowSimpleWithStatus } from "@/utils/simple-error"; export const useInvocationStore = defineStore("invocationStore", () => { const scrollListScrollTop = ref(0); async function fetchInvocationDetails(params: FetchParams): Promise { - const { data, error } = await GalaxyApi().GET("/api/invocations/{invocation_id}", { + const { data, error, response } = await GalaxyApi().GET("/api/invocations/{invocation_id}", { params: { path: { invocation_id: params.id } }, }); if (error) { - rethrowSimple(error); + rethrowSimpleWithStatus(error, response); } return data; } async function fetchInvocationJobsSummary(params: FetchParams): Promise { - const { data, error } = await GalaxyApi().GET("/api/invocations/{invocation_id}/jobs_summary", { + const { data, error, response } = await GalaxyApi().GET("/api/invocations/{invocation_id}/jobs_summary", { params: { path: { invocation_id: params.id } }, }); if (error) { - rethrowSimple(error); + rethrowSimpleWithStatus(error, response); } return data; } async function fetchInvocationStepJobsSummary(params: FetchParams): Promise { - const { data, error } = await GalaxyApi().GET("/api/invocations/{invocation_id}/step_jobs_summary", { + const { data, error, response } = await GalaxyApi().GET("/api/invocations/{invocation_id}/step_jobs_summary", { params: { path: { invocation_id: params.id } }, }); if (error) { - rethrowSimple(error); + rethrowSimpleWithStatus(error, response); } return data; } async function fetchInvocationStep(params: FetchParams): Promise { - const { data, error } = await GalaxyApi().GET("/api/invocations/steps/{step_id}", { + const { data, error, response } = await GalaxyApi().GET("/api/invocations/steps/{step_id}", { params: { path: { step_id: params.id } }, }); if (error) { - rethrowSimple(error); + rethrowSimpleWithStatus(error, response); } return data; } async function fetchInvocationRequest(params: FetchParams): Promise { - const { data, error } = await GalaxyApi().GET("/api/invocations/{invocation_id}/request", { + const { data, error, response } = await GalaxyApi().GET("/api/invocations/{invocation_id}/request", { params: { path: { invocation_id: params.id, @@ -64,17 +64,17 @@ export const useInvocationStore = defineStore("invocationStore", () => { }, }); if (error) { - rethrowSimple(error); + rethrowSimpleWithStatus(error, response); } return data; } async function fetchInvocationCount(params: FetchParams): Promise { - const { data, error } = await GalaxyApi().GET("/api/workflows/{workflow_id}/counts", { + const { data, error, response } = await GalaxyApi().GET("/api/workflows/{workflow_id}/counts", { params: { path: { workflow_id: params.id } }, }); if (error) { - rethrowSimple(error); + rethrowSimpleWithStatus(error, response); } let allCounts = 0; diff --git a/client/src/stores/jobDestinationParametersStore.ts b/client/src/stores/jobDestinationParametersStore.ts index a002671182a8..32485054ebd5 100644 --- a/client/src/stores/jobDestinationParametersStore.ts +++ b/client/src/stores/jobDestinationParametersStore.ts @@ -3,17 +3,17 @@ import { defineStore } from "pinia"; import { GalaxyApi } from "@/api"; import type { JobDestinationParams } from "@/api/jobs"; import { type FetchParams, useKeyedCache } from "@/composables/keyedCache"; -import { rethrowSimple } from "@/utils/simple-error"; +import { rethrowSimpleWithStatus } from "@/utils/simple-error"; export const useJobDestinationParametersStore = defineStore("jobDestinationParametersStore", () => { async function fetchJobDestinationParams(params: FetchParams): Promise { - const { data, error } = await GalaxyApi().GET("/api/jobs/{job_id}/destination_params", { + const { data, error, response } = await GalaxyApi().GET("/api/jobs/{job_id}/destination_params", { params: { path: { job_id: params.id }, }, }); if (error) { - rethrowSimple(error); + rethrowSimpleWithStatus(error, response); } return data; } diff --git a/client/src/stores/jobStore.ts b/client/src/stores/jobStore.ts index 3eb75d04aac5..c1c4c930bb97 100644 --- a/client/src/stores/jobStore.ts +++ b/client/src/stores/jobStore.ts @@ -9,18 +9,18 @@ import { ref } from "vue"; import { GalaxyApi } from "@/api"; import { type ResponseVal, type ShowFullJobResponse, TERMINAL_STATES } from "@/api/jobs"; import { type FetchParams, useKeyedCache } from "@/composables/keyedCache"; -import { rethrowSimple } from "@/utils/simple-error"; +import { rethrowSimpleWithStatus } from "@/utils/simple-error"; export const useJobStore = defineStore("jobStore", () => { const latestResponse = ref(null); async function fetchJobById(params: FetchParams): Promise { - const { data, error } = await GalaxyApi().GET("/api/jobs/{job_id}", { + const { data, error, response } = await GalaxyApi().GET("/api/jobs/{job_id}", { params: { path: { job_id: params.id } }, query: { full: true }, }); if (error) { - rethrowSimple(error); + rethrowSimpleWithStatus(error, response); } return data; } From 557666e9deb6ef0ef3c46616f17fcd4a8853d077 Mon Sep 17 00:00:00 2001 From: Dannon Baker Date: Tue, 24 Feb 2026 13:08:51 -0500 Subject: [PATCH 3/7] Show error state in DatasetView instead of infinite spinner When a dataset fetch fails permanently (e.g. 404 or 403), the view now displays an error alert instead of spinning forever waiting for data that will never arrive. --- client/src/composables/keyedCache.test.ts | 57 ++--------------------- 1 file changed, 5 insertions(+), 52 deletions(-) diff --git a/client/src/composables/keyedCache.test.ts b/client/src/composables/keyedCache.test.ts index 6a048c8bf26f..be520f699d62 100644 --- a/client/src/composables/keyedCache.test.ts +++ b/client/src/composables/keyedCache.test.ts @@ -223,8 +223,8 @@ describe("useKeyedCache", () => { const { getItemById, getItemLoadError } = useKeyedCache(fetchItem); - // First attempt + MAX_RETRIES - 1 retries (first call counts as attempt 1) - for (let i = 1; i <= 3; i++) { + // Initial fetch + MAX_RETRIES retries = 4 total calls + for (let i = 1; i <= 4; i++) { getItemById.value(id); await flushPromises(); expect(fetchItem).toHaveBeenCalledTimes(i); @@ -234,7 +234,7 @@ describe("useKeyedCache", () => { // Should stop after max retries exhausted getItemById.value(id); await flushPromises(); - expect(fetchItem).toHaveBeenCalledTimes(3); + expect(fetchItem).toHaveBeenCalledTimes(4); }); it("should not retry on permanent errors (403, 404)", async () => { @@ -293,52 +293,7 @@ describe("useKeyedCache", () => { expect(true).toBe(true); }); - it("should not retry after a plain Error", async () => { - const id = "1"; - fetchItem.mockRejectedValue(new Error("something broke")); - - const { getItemById } = useKeyedCache(fetchItem); - - getItemById.value(id); - await flushPromises(); - expect(fetchItem).toHaveBeenCalledTimes(1); - - // Subsequent calls should not re-fetch - getItemById.value(id); - await flushPromises(); - expect(fetchItem).toHaveBeenCalledTimes(1); - }); - - it("should retry up to 3 times for a retryable ApiError then stop", async () => { - const id = "1"; - fetchItem.mockRejectedValue(new ApiError("rate limited", 429)); - - const { getItemById } = useKeyedCache(fetchItem); - - // Initial fetch + 3 retries = 4 total calls - for (let i = 0; i < 5; i++) { - getItemById.value(id); - await flushPromises(); - } - expect(fetchItem).toHaveBeenCalledTimes(4); - }); - - it("should not retry for a non-retryable ApiError", async () => { - const id = "1"; - fetchItem.mockRejectedValue(new ApiError("forbidden", 403)); - - const { getItemById } = useKeyedCache(fetchItem); - - getItemById.value(id); - await flushPromises(); - expect(fetchItem).toHaveBeenCalledTimes(1); - - getItemById.value(id); - await flushPromises(); - expect(fetchItem).toHaveBeenCalledTimes(1); - }); - - it("should recover after a retryable error followed by success", async () => { + it("should clear error on successful recovery after transient failure", async () => { const id = "1"; const item = { id, name: "Item 1" }; fetchItem.mockRejectedValueOnce(new ApiError("service unavailable", 503)); @@ -348,13 +303,11 @@ describe("useKeyedCache", () => { getItemById.value(id); await flushPromises(); - expect(fetchItem).toHaveBeenCalledTimes(1); expect(getItemLoadError.value(id)).toBeTruthy(); - // Retry should succeed getItemById.value(id); await flushPromises(); - expect(fetchItem).toHaveBeenCalledTimes(2); expect(storedItems.value[id]).toEqual(item); + expect(getItemLoadError.value(id)).toBeNull(); }); }); From a25016da9035041fadcf0ef70b31b6a578c28af5 Mon Sep 17 00:00:00 2001 From: Dannon Baker Date: Tue, 24 Feb 2026 19:59:26 -0500 Subject: [PATCH 4/7] Extract shared retry helpers and add GalaxyApiResult type to simple-error Move RETRYABLE_STATUSES, MAX_RETRIES, and isRetryableApiError from keyedCache.ts into simple-error.ts so they can be reused by other stores that need the same retry logic. Also add GalaxyApiResult discriminated union type which makes API errors visible in function signatures instead of relying on thrown exceptions. --- client/src/composables/keyedCache.ts | 15 +++------------ client/src/utils/simple-error.ts | 12 ++++++++++++ 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/client/src/composables/keyedCache.ts b/client/src/composables/keyedCache.ts index 708c43430269..e2efa5593b69 100644 --- a/client/src/composables/keyedCache.ts +++ b/client/src/composables/keyedCache.ts @@ -2,17 +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"; - -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; -} +import { isRetryableApiError, MAX_RETRIES } from "@/utils/simple-error"; /** * Parameters for fetching an item from the server. @@ -67,7 +57,8 @@ export function useKeyedCache( return (id: string) => { const item = storedItems.value[id]; const existingError = loadingErrors.value[id]; - const canRetry = existingError && isRetryableError(existingError) && (retryCounts[id] ?? 0) <= MAX_RETRIES; + const canRetry = + existingError && isRetryableApiError(existingError) && (retryCounts[id] ?? 0) <= MAX_RETRIES; if (shouldFetch(item) && (!existingError || canRetry)) { fetchItemById({ id: id }); } diff --git a/client/src/utils/simple-error.ts b/client/src/utils/simple-error.ts index 656da5f8f173..c605b7b504e4 100644 --- a/client/src/utils/simple-error.ts +++ b/client/src/utils/simple-error.ts @@ -39,3 +39,15 @@ export function rethrowSimpleWithStatus(e: any, response?: { status: number }): } throw new ApiError(errorMessageAsString(e), response?.status); } + +export type GalaxyApiResult = { data: T; error: undefined } | { data: undefined; error: ApiError }; + +export const MAX_RETRIES = 3; +const RETRYABLE_STATUSES = new Set([429, 500, 502, 503, 504]); + +export function isRetryableApiError(error: Error): boolean { + if (error instanceof ApiError && error.status !== undefined) { + return RETRYABLE_STATUSES.has(error.status); + } + return false; +} From a7775f6a202ddf30e6b27e8ffda67a9719961bdd Mon Sep 17 00:00:00 2001 From: Dannon Baker Date: Tue, 24 Feb 2026 21:03:30 -0500 Subject: [PATCH 5/7] Fix historyStore infinite request loop on fetch failure getHistoryById triggers loadHistoryById when a history is missing, but if the fetch fails the error was thrown via rethrowSimple without being tracked. The computed would re-trigger the fetch on every render, causing an infinite request loop. Now getHistoryByIdFromServer returns a GalaxyApiResult instead of throwing, loadHistoryById tracks errors in historyLoadErrors, and getHistoryById gates on existing errors with retry logic for transient HTTP status codes (429, 5xx). --- client/src/stores/historyStore.ts | 37 ++++++++++++++----- .../src/stores/services/history.services.ts | 17 +++++---- 2 files changed, 37 insertions(+), 17 deletions(-) diff --git a/client/src/stores/historyStore.ts b/client/src/stores/historyStore.ts index b8688ff68ced..9dc51dfb63fa 100644 --- a/client/src/stores/historyStore.ts +++ b/client/src/stores/historyStore.ts @@ -25,7 +25,7 @@ import { setCurrentHistoryOnServer, updateHistoryFields, } from "@/stores/services/history.services"; -import { rethrowSimple } from "@/utils/simple-error"; +import { isRetryableApiError, MAX_RETRIES, rethrowSimple } from "@/utils/simple-error"; import { sortByObjectProp } from "@/utils/sorting"; import { ACTIVE_POLLING_INTERVAL, @@ -34,7 +34,8 @@ import { } from "@/watch/watchHistory"; const PAGINATION_LIMIT = 10; -const isLoadingHistory = new Set(); +const isLoadingHistory = new Set(); +const retryCounts: { [key: string]: number } = {}; const CONTENT_STATS_KEYS = ["size", "contents_active", "update_time"] as const; export const useHistoryStore = defineStore("historyStore", () => { @@ -45,6 +46,7 @@ export const useHistoryStore = defineStore("historyStore", () => { const storedCurrentHistoryId = ref(null); const storedFilterTexts = ref<{ [key: string]: string }>({}); const storedHistories = ref<{ [key: string]: AnyHistory }>({}); + const historyLoadErrors = ref<{ [key: string]: Error }>({}); const changingCurrentHistory = ref(false); const histories = computed(() => { @@ -80,14 +82,24 @@ export const useHistoryStore = defineStore("historyStore", () => { } }); + const getHistoryLoadError = computed(() => { + return (historyId: string) => { + return historyLoadErrors.value[historyId] ?? null; + }; + }); + /** Returns history from storedHistories, will load history if not in store by default. * If shouldFetchIfMissing is false, will return null if history is not in store. */ const getHistoryById = computed(() => { return (historyId: string, shouldFetchIfMissing = true) => { if (!storedHistories.value[historyId] && shouldFetchIfMissing) { - // TODO: Try to remove this as it can cause computed side effects - loadHistoryById(historyId); + const existingError = historyLoadErrors.value[historyId]; + const canRetry = + existingError && isRetryableApiError(existingError) && (retryCounts[historyId] ?? 0) <= MAX_RETRIES; + if (!existingError || canRetry) { + loadHistoryById(historyId); + } } return storedHistories.value[historyId] ?? null; }; @@ -387,15 +399,19 @@ export const useHistoryStore = defineStore("historyStore", () => { }, ); - async function loadHistoryById(historyId: string): Promise { + async function loadHistoryById(historyId: string) { if (!isLoadingHistory.has(historyId)) { isLoadingHistory.add(historyId); try { - const history = (await getHistoryByIdFromServer(historyId)) as HistorySummaryExtended; - setHistory(history); - return history; - } catch (error) { - rethrowSimple(error); + const result = await getHistoryByIdFromServer(historyId); + if (result.error) { + retryCounts[historyId] = (retryCounts[historyId] ?? 0) + 1; + set(historyLoadErrors.value, historyId, result.error); + } else { + setHistory(result.data); + del(historyLoadErrors.value, historyId); + delete retryCounts[historyId]; + } } finally { isLoadingHistory.delete(historyId); } @@ -489,6 +505,7 @@ export const useHistoryStore = defineStore("historyStore", () => { pinnedHistories, storedHistories, getHistoryById, + getHistoryLoadError, getHistoryNameById, setCurrentHistory, setCurrentHistoryId, diff --git a/client/src/stores/services/history.services.ts b/client/src/stores/services/history.services.ts index 876c6c9b60e2..5b1ea57dc623 100644 --- a/client/src/stores/services/history.services.ts +++ b/client/src/stores/services/history.services.ts @@ -3,7 +3,7 @@ import axios, { type AxiosResponse } from "axios"; import type { AnyHistory, HistorySummaryExtended } from "@/api"; import { GalaxyApi } from "@/api"; import { prependPath } from "@/utils/redirect"; -import { rethrowSimple } from "@/utils/simple-error"; +import { ApiError, errorMessageAsString, type GalaxyApiResult, rethrowSimple } from "@/utils/simple-error"; /** * Generic json getter @@ -97,8 +97,8 @@ export async function getHistoryList(offset = 0, limit: number | null = null, qu } /** Load one history by id */ -export async function getHistoryByIdFromServer(id: string) { - const { data, error } = await GalaxyApi().GET("/api/histories/{history_id}", { +export async function getHistoryByIdFromServer(id: string): Promise> { + const { data, error, response } = await GalaxyApi().GET("/api/histories/{history_id}", { params: { path: { history_id: id }, query: extendedHistoryParams, @@ -106,12 +106,12 @@ export async function getHistoryByIdFromServer(id: string) { }); if (error) { - rethrowSimple(error); + return { data: undefined, error: new ApiError(errorMessageAsString(error), response.status) }; } // We know that the data is a HistorySummaryExtended because we requested it // with the extendedHistoryParams - return data as HistorySummaryExtended; + return { data: data as HistorySummaryExtended, error: undefined }; } /** @@ -127,9 +127,12 @@ export async function secureHistoryOnServer(history: AnyHistory) { if (response.status !== 200) { throw new Error(response.statusText); } - const securedHistory = await getHistoryByIdFromServer(id); + const result = await getHistoryByIdFromServer(id); + if (result.error) { + throw result.error; + } return { - securedHistory, + securedHistory: result.data, message: response.data.message as string, sharingStatusChanged: response.data.sharing_status_changed as boolean, }; From 21854f0b75f0ec5b4051ee43f98d86b0cb948000 Mon Sep 17 00:00:00 2001 From: Dannon Baker Date: Tue, 24 Feb 2026 21:31:19 -0500 Subject: [PATCH 6/7] Add retry logic to collectionElementsStore and fix callers fetchCollectionDetails now returns GalaxyApiResult instead of throwing, which lets collectionElementsStore track errors as ApiError with HTTP status codes. getCollectionById and getDetailedCollectionById now gate on existing errors with retry logic for transient statuses (429, 5xx), matching the pattern from keyedCache and historyStore. Updated all callers of fetchCollectionDetails and loadHistoryById to handle the new return types. --- client/src/api/datasetCollections.ts | 10 ++--- .../Collections/SampleSheetWizard.vue | 9 ++-- .../Form/Elements/FormRulesEdit.vue | 7 ++- .../History/Export/HistoryExport.vue | 13 ++++-- .../providers/DatasetCollectionProvider.js | 8 +++- client/src/composables/uploadQueue.ts | 3 +- client/src/stores/collectionElementsStore.ts | 43 +++++++++++++------ 7 files changed, 62 insertions(+), 31 deletions(-) diff --git a/client/src/api/datasetCollections.ts b/client/src/api/datasetCollections.ts index 6d77cc03341c..249e7c52b6b5 100644 --- a/client/src/api/datasetCollections.ts +++ b/client/src/api/datasetCollections.ts @@ -9,7 +9,7 @@ import { isHDCA, } from "@/api"; import type { components } from "@/api/schema"; -import { rethrowSimple } from "@/utils/simple-error"; +import { ApiError, errorMessageAsString, type GalaxyApiResult, rethrowSimple } from "@/utils/simple-error"; const DEFAULT_LIMIT = 50; @@ -27,15 +27,15 @@ export type SampleSheetColumnValueT = string | number | boolean; * Fetches the details of a collection. * @param params.id The ID of the collection (HDCA) to fetch. */ -export async function fetchCollectionDetails(params: { hdca_id: string }): Promise { - const { data, error } = await GalaxyApi().GET("/api/dataset_collections/{hdca_id}", { +export async function fetchCollectionDetails(params: { hdca_id: string }): Promise> { + const { data, error, response } = await GalaxyApi().GET("/api/dataset_collections/{hdca_id}", { params: { path: params }, }); if (error) { - rethrowSimple(error); + return { data: undefined, error: new ApiError(errorMessageAsString(error), response.status) }; } - return data as HDCADetailed; + return { data: data as HDCADetailed, error: undefined }; } /** diff --git a/client/src/components/Collections/SampleSheetWizard.vue b/client/src/components/Collections/SampleSheetWizard.vue index bfaa950a10f5..71b05d073f12 100644 --- a/client/src/components/Collections/SampleSheetWizard.vue +++ b/client/src/components/Collections/SampleSheetWizard.vue @@ -138,10 +138,11 @@ watch(targetCollectionId, async () => { if (targetCollectionId.value) { fetchingCollection.value = true; try { - // TODO: spinner while loading - const details = await fetchCollectionDetails({ hdca_id: targetCollectionId.value }); - targetCollection.value = details; - // Nothing else to do on the page, just skip to the next step. + const result = await fetchCollectionDetails({ hdca_id: targetCollectionId.value }); + if (result.error) { + throw result.error; + } + targetCollection.value = result.data; nextTick(() => { wizard.goTo("fill-grid"); }); diff --git a/client/src/components/Form/Elements/FormRulesEdit.vue b/client/src/components/Form/Elements/FormRulesEdit.vue index 71a268f0cf38..14f9f8f70b79 100644 --- a/client/src/components/Form/Elements/FormRulesEdit.vue +++ b/client/src/components/Form/Elements/FormRulesEdit.vue @@ -39,8 +39,11 @@ async function onEdit() { try { loading.value = true; loadError.value = undefined; - const collectionDetails = await fetchCollectionDetails({ hdca_id: props.target.id }); - elements.value = collectionDetails; + const result = await fetchCollectionDetails({ hdca_id: props.target.id }); + if (result.error) { + throw result.error; + } + elements.value = result.data; modal.value.show(); } catch (e) { loadError.value = errorMessageAsString(e); diff --git a/client/src/components/History/Export/HistoryExport.vue b/client/src/components/History/Export/HistoryExport.vue index 291c673ca3ea..8d45a172697f 100644 --- a/client/src/components/History/Export/HistoryExport.vue +++ b/client/src/components/History/Export/HistoryExport.vue @@ -94,9 +94,16 @@ watch(isExportTaskRunning, (newValue, oldValue) => { async function loadHistory() { isLoadingHistory.value = true; try { - history.value = - historyStore.getHistoryById(props.historyId, false) ?? - (await historyStore.loadHistoryById(props.historyId)); + if (!historyStore.getHistoryById(props.historyId, false)) { + await historyStore.loadHistoryById(props.historyId); + } + history.value = historyStore.getHistoryById(props.historyId, false) ?? undefined; + if (!history.value) { + const loadError = historyStore.getHistoryLoadError(props.historyId); + if (loadError) { + throw loadError; + } + } return true; } catch (error) { errorMessage.value = errorMessageAsString(error); diff --git a/client/src/components/providers/DatasetCollectionProvider.js b/client/src/components/providers/DatasetCollectionProvider.js index c9eb32bc9a8b..246ad5859a45 100644 --- a/client/src/components/providers/DatasetCollectionProvider.js +++ b/client/src/components/providers/DatasetCollectionProvider.js @@ -4,11 +4,15 @@ import { SingleQueryProvider } from "@/components/providers/SingleQueryProvider" // There isn't really a good way to know when to stop polling for HDCA updates, // but we know the populated_state should at least be ok. export default SingleQueryProvider( - (params) => { + async (params) => { if (params.view && params.view === "collection") { return fetchCollectionSummary({ hdca_id: params.id }); } - return fetchCollectionDetails({ hdca_id: params.id }); + const result = await fetchCollectionDetails({ hdca_id: params.id }); + if (result.error) { + throw result.error; + } + return result.data; }, (result) => result.populated_state === "ok", ); diff --git a/client/src/composables/uploadQueue.ts b/client/src/composables/uploadQueue.ts index 7733a4a0a589..f7e7392bb59c 100644 --- a/client/src/composables/uploadQueue.ts +++ b/client/src/composables/uploadQueue.ts @@ -389,7 +389,8 @@ export function useUploadQueue() { async function validateTargetHistory(targetHistoryId: string): Promise { let history = historyStore.getHistoryById(targetHistoryId, false) ?? null; if (!history) { - history = (await historyStore.loadHistoryById(targetHistoryId)) ?? null; + await historyStore.loadHistoryById(targetHistoryId); + history = historyStore.getHistoryById(targetHistoryId, false) ?? null; } // If history still cannot be resolved, treat as no validation error here diff --git a/client/src/stores/collectionElementsStore.ts b/client/src/stores/collectionElementsStore.ts index cebd24f7f59e..2c01da10c9d4 100644 --- a/client/src/stores/collectionElementsStore.ts +++ b/client/src/stores/collectionElementsStore.ts @@ -12,6 +12,7 @@ import { import { fetchCollectionDetails, fetchElementsFromCollection } from "@/api/datasetCollections"; import { ensureDefined } from "@/utils/assertions"; import { ActionSkippedError, LastQueue } from "@/utils/lastQueue"; +import { isRetryableApiError, MAX_RETRIES } from "@/utils/simple-error"; /** * Represents an element in a collection that has not been fetched yet. @@ -46,6 +47,7 @@ export const useCollectionElementsStore = defineStore("collectionElementsStore", const loadingCollectionElements = ref<{ [key: string]: boolean }>({}); const loadingCollectionElementsErrors = ref<{ [key: string]: Error }>({}); const storedCollectionElements = ref<{ [key: string]: DCEEntry[] }>({}); + const retryCounts: { [key: string]: number } = {}; /** * Returns a key that can be used to store or retrieve the elements of a collection in the store. @@ -185,9 +187,15 @@ 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] && !loadingCollectionElementsErrors.value[collectionId]) { - // TODO: Try to remove this as it can cause computed side effects (use keyedCache in this store instead?) - fetchCollection({ id: collectionId }); + if (!storedCollections.value[collectionId]) { + const existingError = loadingCollectionElementsErrors.value[collectionId]; + const canRetry = + existingError && + isRetryableApiError(existingError) && + (retryCounts[collectionId] ?? 0) <= MAX_RETRIES; + if (!existingError || canRetry) { + fetchCollection({ id: collectionId }); + } } return storedCollections.value[collectionId] ?? null; }; @@ -195,12 +203,15 @@ export const useCollectionElementsStore = defineStore("collectionElementsStore", const getDetailedCollectionById = computed(() => { return (collectionId: string) => { - if ( - !storedCollectionsDetailed.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 }); + if (!storedCollectionsDetailed.value[collectionId]) { + const existingError = loadingCollectionElementsErrors.value[collectionId]; + const canRetry = + existingError && + isRetryableApiError(existingError) && + (retryCounts[collectionId] ?? 0) <= MAX_RETRIES; + if (!existingError || canRetry) { + fetchCollection({ id: collectionId }); + } } return storedCollectionsDetailed.value[collectionId] ?? null; }; @@ -209,11 +220,15 @@ export const useCollectionElementsStore = defineStore("collectionElementsStore", async function fetchCollection(params: { id: string }) { set(loadingCollectionElements.value, params.id, true); try { - const collection = await fetchCollectionDetails({ hdca_id: params.id }); - saveCollection(collection); - return collection; - } catch (error) { - set(loadingCollectionElementsErrors.value, params.id, error); + const result = await fetchCollectionDetails({ hdca_id: params.id }); + if (result.error) { + retryCounts[params.id] = (retryCounts[params.id] ?? 0) + 1; + set(loadingCollectionElementsErrors.value, params.id, result.error); + } else { + saveCollection(result.data); + del(loadingCollectionElementsErrors.value, params.id); + delete retryCounts[params.id]; + } } finally { del(loadingCollectionElements.value, params.id); } From 99e2a4fac6eedfcfce8612f1559bf8641bc0865a Mon Sep 17 00:00:00 2001 From: Dannon Baker Date: Wed, 25 Feb 2026 07:01:12 -0500 Subject: [PATCH 7/7] Update HistoryView test mock for GalaxyApiResult return type getHistoryByIdFromServer now returns GalaxyApiResult so the mock needs to wrap the history in { data, error } shape. --- client/src/components/History/HistoryView.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/src/components/History/HistoryView.test.js b/client/src/components/History/HistoryView.test.js index 420f518dd96e..bd262a76d9cb 100644 --- a/client/src/components/History/HistoryView.test.js +++ b/client/src/components/History/HistoryView.test.js @@ -72,7 +72,7 @@ function create_datasets(historyId, count) { async function createWrapper(localVue, currentUserId, history) { const pinia = createPinia(); - getHistoryByIdFromServer.mockResolvedValue(history); + getHistoryByIdFromServer.mockResolvedValue({ data: history, error: undefined }); setCurrentHistoryOnServer.mockResolvedValue(history); const history_contents_result = create_datasets(history.id, history.count);