From fa3929fd0252f98dfed92631f81b431fae89af38 Mon Sep 17 00:00:00 2001 From: "Per G. da Silva" Date: Fri, 17 Jul 2026 12:05:52 +0200 Subject: [PATCH 1/6] NO-ISSUE: Fix AbortError from cached promise surfacing as real error When useOperatorLifecycle re-runs (deps changed) while a fetch is in-flight, fetchLifecycleData returns the existing cached promise (which was created with the previous AbortController's signal). When the old controller is then aborted, that promise rejects with AbortError. The .catch handler only checked !controller.signal.aborted (the NEW controller, which is not aborted), so the AbortError was incorrectly surfaced as a real error, producing NS_BINDING_ERROR noise in the browser console. Fix: also skip setError when err.name === 'AbortError', while still calling setLoading(false) so the loading state settles correctly. Co-Authored-By: Claude Sonnet 4.6 (1M context) --- .../__tests__/useOperatorLifecycle.spec.ts | 90 +++++++++++++++++++ .../src/hooks/useOperatorLifecycle.ts | 6 +- 2 files changed, 94 insertions(+), 2 deletions(-) create mode 100644 frontend/packages/operator-lifecycle-manager/src/hooks/__tests__/useOperatorLifecycle.spec.ts diff --git a/frontend/packages/operator-lifecycle-manager/src/hooks/__tests__/useOperatorLifecycle.spec.ts b/frontend/packages/operator-lifecycle-manager/src/hooks/__tests__/useOperatorLifecycle.spec.ts new file mode 100644 index 00000000000..af2632187cb --- /dev/null +++ b/frontend/packages/operator-lifecycle-manager/src/hooks/__tests__/useOperatorLifecycle.spec.ts @@ -0,0 +1,90 @@ +import { act, renderHook } from '@testing-library/react'; +import { coFetchJSON } from '@console/shared/src/utils/console-fetch'; +import { useOperatorLifecycle } from '../useOperatorLifecycle'; + +jest.mock('@console/shared/src/utils/console-fetch', () => ({ + coFetchJSON: jest.fn(), +})); + +const mockCoFetchJSON = coFetchJSON as jest.Mock; + +// Each test uses a unique packageName to avoid sharing module-level cache entries. +const CATALOG = 'redhat-operators'; +const NS = 'openshift-marketplace'; + +describe('useOperatorLifecycle', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('returns data when fetch succeeds', async () => { + const mockData = { lifecycleSchema: 'test', properties: '{}' }; + mockCoFetchJSON.mockResolvedValueOnce(mockData); + + const { result } = renderHook(() => useOperatorLifecycle('pkg-success', CATALOG, NS)); + + expect(result.current[1]).toBe(true); // loading initially + + await act(async () => { + await Promise.resolve(); + }); + + const [data, loading, error] = result.current; + expect(data).toEqual(mockData); + expect(loading).toBe(false); + expect(error).toBeNull(); + }); + + it('does not surface AbortError as a real error', async () => { + // The bug: fetchLifecycleData re-throws AbortErrors after clearing the cache. + // The hook's .catch only checked !controller.signal.aborted — if the signal was + // never explicitly aborted (e.g., AbortError came from an old cached promise's + // signal), the error was incorrectly set in state. + // + // The fix: also guard on err.name !== 'AbortError' so abort-flavored errors + // are never surfaced regardless of which signal triggered them. + const abortError = new DOMException('The operation was aborted.', 'AbortError'); + mockCoFetchJSON.mockRejectedValueOnce(abortError); + + const { result } = renderHook(() => useOperatorLifecycle('pkg-abort', CATALOG, NS)); + + await act(async () => { + await Promise.resolve(); + await Promise.resolve(); // allow rejection microtask to settle + }); + + const [data, loading, error] = result.current; + expect(error).toBeNull(); // AbortError must not surface as a real error + expect(data).toBeNull(); + expect(loading).toBe(false); + }); + + it('surfaces real network errors as errors', async () => { + const networkError = new Error('Network failure'); + mockCoFetchJSON.mockRejectedValueOnce(networkError); + + const { result } = renderHook(() => useOperatorLifecycle('pkg-network-error', CATALOG, NS)); + + await act(async () => { + await Promise.resolve(); + await Promise.resolve(); + }); + + const [data, loading, error] = result.current; + expect(error).toBe(networkError); + expect(data).toBeNull(); + expect(loading).toBe(false); + }); + + it('skips fetch when packageName is missing', () => { + renderHook(() => useOperatorLifecycle(undefined, CATALOG, NS)); + + expect(mockCoFetchJSON).not.toHaveBeenCalled(); + const [data, loading, error] = renderHook(() => + useOperatorLifecycle(undefined, CATALOG, NS), + ).result.current; + expect(data).toBeNull(); + expect(loading).toBe(false); + expect(error).toBeNull(); + }); +}); diff --git a/frontend/packages/operator-lifecycle-manager/src/hooks/useOperatorLifecycle.ts b/frontend/packages/operator-lifecycle-manager/src/hooks/useOperatorLifecycle.ts index 8426da67ee5..30b8e3fbd9a 100644 --- a/frontend/packages/operator-lifecycle-manager/src/hooks/useOperatorLifecycle.ts +++ b/frontend/packages/operator-lifecycle-manager/src/hooks/useOperatorLifecycle.ts @@ -130,9 +130,11 @@ export const useOperatorLifecycle = ( }) .catch((err: Error) => { if (!controller.signal.aborted) { - setData(null); - setError(err); setLoading(false); + if (err.name !== 'AbortError') { + setData(null); + setError(err); + } } }); From 73fad79d9f273b8047648d3d2b83a577a11041a5 Mon Sep 17 00:00:00 2001 From: "Per G. da Silva" Date: Fri, 17 Jul 2026 13:27:37 +0200 Subject: [PATCH 2/6] NO-ISSUE: Suppress AbortError console.warn and fix cache signal coupling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix A — console-fetch.ts: Don't console.warn for AbortErrors. Aborted fetches are expected when AbortController.abort() is called (component unmount, dep change), not failures. The warn produced NS_BINDING_ABORTED noise in Firefox devtools. Fix B — useOperatorLifecycle.ts: When fetchLifecycleData returns a cached in-flight promise tied to a different caller's signal, chain a .catch() that detects if the original was aborted by someone else while our signal is still live and retries with a fresh request. Previously, the spurious AbortError would propagate to the hook's .catch and leave the row in an empty non-loading state. Co-Authored-By: Claude Sonnet 4.6 (1M context) --- .../src/utils/__tests__/console-fetch.spec.ts | 26 ++++++++++++ .../console-shared/src/utils/console-fetch.ts | 2 + .../__tests__/useOperatorLifecycle.spec.ts | 42 +++++++++++++++++++ .../src/hooks/useOperatorLifecycle.ts | 11 ++++- 4 files changed, 80 insertions(+), 1 deletion(-) diff --git a/frontend/packages/console-shared/src/utils/__tests__/console-fetch.spec.ts b/frontend/packages/console-shared/src/utils/__tests__/console-fetch.spec.ts index 527ee36306a..79a7a752714 100644 --- a/frontend/packages/console-shared/src/utils/__tests__/console-fetch.spec.ts +++ b/frontend/packages/console-shared/src/utils/__tests__/console-fetch.spec.ts @@ -103,6 +103,32 @@ describe('consoleFetch', () => { ).rejects.not.toBeInstanceOf(RetryError); }); + it('does not console.warn when fetch is aborted via AbortController', async () => { + const consoleSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + const controller = new AbortController(); + window.fetch = jest.fn(() => + Promise.reject(new DOMException('The operation was aborted.', 'AbortError')), + ); + + await expect( + coFetch('/api/lifecycle/test', { signal: controller.signal }), + ).rejects.toMatchObject({ name: 'AbortError' }); + expect(consoleSpy).not.toHaveBeenCalled(); + consoleSpy.mockRestore(); + }); + + it('does console.warn when fetch fails with a real network error', async () => { + const consoleSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + window.fetch = jest.fn(() => Promise.reject(new TypeError('Network failure'))); + + await expect(coFetch('/api/lifecycle/test')).rejects.toThrow('Network failure'); + expect(consoleSpy).toHaveBeenCalledWith( + expect.stringContaining('/api/lifecycle/test'), + expect.any(TypeError), + ); + consoleSpy.mockRestore(); + }); + it('should retry up to 3 times when RetryError is thrown', async () => { window.fetch = jest.fn(() => Promise.resolve({ status: 404, headers: emptyHeaders } as Response), diff --git a/frontend/packages/console-shared/src/utils/console-fetch.ts b/frontend/packages/console-shared/src/utils/console-fetch.ts index 7adf9a5cf55..d6fedcdb246 100644 --- a/frontend/packages/console-shared/src/utils/console-fetch.ts +++ b/frontend/packages/console-shared/src/utils/console-fetch.ts @@ -39,6 +39,8 @@ export const coFetch: ConsoleFetch = async (url, options = {}, timeout = 60000) } catch (e) { if (e instanceof RetryError) { retry = true; + } else if (e instanceof DOMException && e.name === 'AbortError') { + throw e; // expected cancellation — don't warn } else { // eslint-disable-next-line no-console console.warn(`consoleFetch failed for url ${url}`, e); diff --git a/frontend/packages/operator-lifecycle-manager/src/hooks/__tests__/useOperatorLifecycle.spec.ts b/frontend/packages/operator-lifecycle-manager/src/hooks/__tests__/useOperatorLifecycle.spec.ts index af2632187cb..093b70c2b7e 100644 --- a/frontend/packages/operator-lifecycle-manager/src/hooks/__tests__/useOperatorLifecycle.spec.ts +++ b/frontend/packages/operator-lifecycle-manager/src/hooks/__tests__/useOperatorLifecycle.spec.ts @@ -76,6 +76,48 @@ describe('useOperatorLifecycle', () => { expect(loading).toBe(false); }); + it('retries with a fresh request when the cached in-flight promise was aborted by another caller', async () => { + // Fix B scenario: two hook instances share the same cache key. + // Hook A starts the fetch (P1 enters cache). Hook B arrives and gets P1 from cache. + // Hook A is then aborted — P1 rejects with AbortError. + // Fix B detects that B's signal is still live and retries with B's signal. + const retryData = { lifecycleSchema: 'retry', properties: '{}' }; + let rejectP1: (err: Error) => void; + + // Hook A's fetch hangs until we manually reject it + mockCoFetchJSON.mockImplementationOnce( + () => + new Promise((_resolve, reject) => { + rejectP1 = reject; + }), + ); + // Hook B's retry fetch resolves immediately + mockCoFetchJSON.mockResolvedValueOnce(retryData); + + const PKG = 'pkg-cached-shared'; + + // Hook A: effect runs → P1 created and stored in cache + const { unmount: unmountA } = renderHook(() => useOperatorLifecycle(PKG, CATALOG, NS)); + + // Hook B: same key → effect finds P1 in cache → Fix B wraps P1 with B's signal + const { result: resultB } = renderHook(() => useOperatorLifecycle(PKG, CATALOG, NS)); + + await act(async () => { + unmountA(); // aborts A's controller (signal_A); our mock doesn't react to signals + // Manually reject P1 to simulate what signal_A abort does to the real fetch + rejectP1(new DOMException('The operation was aborted.', 'AbortError')); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); // allow retry to complete + }); + + const [data, loading, error] = resultB.current; + expect(error).toBeNull(); + expect(data).toEqual(retryData); // retry produced real data + expect(loading).toBe(false); + expect(mockCoFetchJSON).toHaveBeenCalledTimes(2); // P1 (aborted) + P2 (retry) + }); + it('skips fetch when packageName is missing', () => { renderHook(() => useOperatorLifecycle(undefined, CATALOG, NS)); diff --git a/frontend/packages/operator-lifecycle-manager/src/hooks/useOperatorLifecycle.ts b/frontend/packages/operator-lifecycle-manager/src/hooks/useOperatorLifecycle.ts index 30b8e3fbd9a..9b64ad28b0c 100644 --- a/frontend/packages/operator-lifecycle-manager/src/hooks/useOperatorLifecycle.ts +++ b/frontend/packages/operator-lifecycle-manager/src/hooks/useOperatorLifecycle.ts @@ -48,7 +48,16 @@ const fetchLifecycleData = ( ): Promise => { const existing = lifecycleCache.get(cacheKey); if (existing?.promise) { - return existing.promise; + // The cached promise is tied to the ORIGINAL caller's signal, not ours. + // If the original was aborted by someone else while our signal is still live, + // issue a fresh retry rather than surfacing a spurious AbortError. + return existing.promise.catch((err: Error) => { + if (err.name === 'AbortError' && !signal.aborted) { + lifecycleCache.delete(cacheKey); + return fetchLifecycleData(cacheKey, url, signal); + } + throw err; + }); } const promise = coFetchJSON(url, 'GET', { signal }).then( From 4068438f167e3bdf78603d55c979851986dc90a3 Mon Sep 17 00:00:00 2001 From: "Per G. da Silva" Date: Fri, 17 Jul 2026 13:32:45 +0200 Subject: [PATCH 3/6] NO-ISSUE: Fix jest.Mock cast for TypeScript strict overlap check Co-Authored-By: Claude Sonnet 4.6 (1M context) --- .../src/hooks/__tests__/useOperatorLifecycle.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/packages/operator-lifecycle-manager/src/hooks/__tests__/useOperatorLifecycle.spec.ts b/frontend/packages/operator-lifecycle-manager/src/hooks/__tests__/useOperatorLifecycle.spec.ts index 093b70c2b7e..aa6355942fd 100644 --- a/frontend/packages/operator-lifecycle-manager/src/hooks/__tests__/useOperatorLifecycle.spec.ts +++ b/frontend/packages/operator-lifecycle-manager/src/hooks/__tests__/useOperatorLifecycle.spec.ts @@ -6,7 +6,7 @@ jest.mock('@console/shared/src/utils/console-fetch', () => ({ coFetchJSON: jest.fn(), })); -const mockCoFetchJSON = coFetchJSON as jest.Mock; +const mockCoFetchJSON = (coFetchJSON as unknown) as jest.Mock; // Each test uses a unique packageName to avoid sharing module-level cache entries. const CATALOG = 'redhat-operators'; From de160a42c539de7e75c132a19e91d3027c24d205 Mon Sep 17 00:00:00 2001 From: "Per G. da Silva" Date: Fri, 17 Jul 2026 14:17:26 +0200 Subject: [PATCH 4/6] NO-ISSUE: Fix multi-caller cache stomp on retry When N callers share the same aborted cached promise, the previous fix unconditionally deleted the cache before retrying, causing each caller to start its own redundant fetch and overwrite the previous retry. Fix: capture the promise reference before .catch() and only delete when the cache still holds that exact promise. The first caller to run the handler starts the single retry; all later callers see a newer promise in the cache and reuse it via the recursive fetchLifecycleData call. Adds a three-caller regression test. Co-Authored-By: Claude Sonnet 4.6 (1M context) --- .../__tests__/useOperatorLifecycle.spec.ts | 43 +++++++++++++++++++ .../src/hooks/useOperatorLifecycle.ts | 11 ++++- 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/frontend/packages/operator-lifecycle-manager/src/hooks/__tests__/useOperatorLifecycle.spec.ts b/frontend/packages/operator-lifecycle-manager/src/hooks/__tests__/useOperatorLifecycle.spec.ts index aa6355942fd..e701660e235 100644 --- a/frontend/packages/operator-lifecycle-manager/src/hooks/__tests__/useOperatorLifecycle.spec.ts +++ b/frontend/packages/operator-lifecycle-manager/src/hooks/__tests__/useOperatorLifecycle.spec.ts @@ -118,6 +118,49 @@ describe('useOperatorLifecycle', () => { expect(mockCoFetchJSON).toHaveBeenCalledTimes(2); // P1 (aborted) + P2 (retry) }); + it('issues only one retry when three callers share the same aborted cached promise', async () => { + // Regression for: multiple callers each unconditionally deleting the cache and + // starting their own retry, causing N redundant fetches. + // Only the first caller to detect the abort should start a retry; the others + // should reuse the retry promise already stored by the first. + const retryData = { lifecycleSchema: 'three-caller', properties: '{}' }; + let rejectP1: (err: Error) => void; + + // P1: hangs until manually rejected + mockCoFetchJSON.mockImplementationOnce( + () => + new Promise((_resolve, reject) => { + rejectP1 = reject; + }), + ); + // P2: the single retry (only one should be started) + mockCoFetchJSON.mockResolvedValueOnce(retryData); + + const PKG = 'pkg-three-callers'; + + // Mount A, B, C — all share the same cache key; B and C get P1 from cache + const { unmount: unmountA } = renderHook(() => useOperatorLifecycle(PKG, CATALOG, NS)); + const { result: resultB } = renderHook(() => useOperatorLifecycle(PKG, CATALOG, NS)); + const { result: resultC } = renderHook(() => useOperatorLifecycle(PKG, CATALOG, NS)); + + await act(async () => { + unmountA(); + rejectP1(new DOMException('The operation was aborted.', 'AbortError')); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + }); + + // Both B and C should see the retry data — not an error, not null + expect(resultB.current[2]).toBeNull(); + expect(resultB.current[0]).toEqual(retryData); + expect(resultC.current[2]).toBeNull(); + expect(resultC.current[0]).toEqual(retryData); + + // Only two coFetchJSON calls total: P1 (aborted) + P2 (single retry) + expect(mockCoFetchJSON).toHaveBeenCalledTimes(2); + }); + it('skips fetch when packageName is missing', () => { renderHook(() => useOperatorLifecycle(undefined, CATALOG, NS)); diff --git a/frontend/packages/operator-lifecycle-manager/src/hooks/useOperatorLifecycle.ts b/frontend/packages/operator-lifecycle-manager/src/hooks/useOperatorLifecycle.ts index 9b64ad28b0c..f21e7478c39 100644 --- a/frontend/packages/operator-lifecycle-manager/src/hooks/useOperatorLifecycle.ts +++ b/frontend/packages/operator-lifecycle-manager/src/hooks/useOperatorLifecycle.ts @@ -51,9 +51,16 @@ const fetchLifecycleData = ( // The cached promise is tied to the ORIGINAL caller's signal, not ours. // If the original was aborted by someone else while our signal is still live, // issue a fresh retry rather than surfacing a spurious AbortError. - return existing.promise.catch((err: Error) => { + // Capture a reference so multiple concurrent callers agree on which promise + // they are watching: only the first one to run this .catch() will find the + // cache still pointing at existingPromise and start a single retry; all + // later callers will see a newer promise in the cache and reuse it. + const existingPromise = existing.promise; + return existingPromise.catch((err: Error) => { if (err.name === 'AbortError' && !signal.aborted) { - lifecycleCache.delete(cacheKey); + if (lifecycleCache.get(cacheKey)?.promise === existingPromise) { + lifecycleCache.delete(cacheKey); + } return fetchLifecycleData(cacheKey, url, signal); } throw err; From 24cc23c449193d692026cf28dcdad7d5c5428e87 Mon Sep 17 00:00:00 2001 From: "Per G. da Silva" Date: Fri, 17 Jul 2026 21:38:54 +0200 Subject: [PATCH 5/6] NO-ISSUE: Remove AbortController from useOperatorLifecycle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The NS_BINDING_ABORTED errors are inherent to the abort-on-unmount pattern: every row unmount (sort, filter, navigation) cancels its in-flight lifecycle request, Firefox logs the cancellation, and the cache-sharing fixes we added are complex responses to a self-inflicted problem. Root fix: let requests complete naturally. - Remove AbortController, abortRef, cleanup abort, and all signal-passing code from useOperatorLifecycle. - Simplify fetchLifecycleData: no more AbortError special-casing, Fix-B retry logic, or multi-caller stomp guard — just deduplicate concurrent requests via the module-level cache and let every request reach the backend exactly once. - When a component unmounts mid-flight the request finishes, populates the cache, and React silently discards the resulting state update. The next render of the same operator (after sort/filter/remount) gets an instant cache hit. - Drop the abort-specific test cases; replace with deduplication and cache-hit tests that describe the new invariants. Co-Authored-By: Claude Sonnet 4.6 (1M context) --- .../__tests__/useOperatorLifecycle.spec.ts | 149 ++++++------------ .../src/hooks/useOperatorLifecycle.ts | 85 +++------- 2 files changed, 63 insertions(+), 171 deletions(-) diff --git a/frontend/packages/operator-lifecycle-manager/src/hooks/__tests__/useOperatorLifecycle.spec.ts b/frontend/packages/operator-lifecycle-manager/src/hooks/__tests__/useOperatorLifecycle.spec.ts index e701660e235..0ecad10ca07 100644 --- a/frontend/packages/operator-lifecycle-manager/src/hooks/__tests__/useOperatorLifecycle.spec.ts +++ b/frontend/packages/operator-lifecycle-manager/src/hooks/__tests__/useOperatorLifecycle.spec.ts @@ -5,7 +5,6 @@ import { useOperatorLifecycle } from '../useOperatorLifecycle'; jest.mock('@console/shared/src/utils/console-fetch', () => ({ coFetchJSON: jest.fn(), })); - const mockCoFetchJSON = (coFetchJSON as unknown) as jest.Mock; // Each test uses a unique packageName to avoid sharing module-level cache entries. @@ -35,30 +34,6 @@ describe('useOperatorLifecycle', () => { expect(error).toBeNull(); }); - it('does not surface AbortError as a real error', async () => { - // The bug: fetchLifecycleData re-throws AbortErrors after clearing the cache. - // The hook's .catch only checked !controller.signal.aborted — if the signal was - // never explicitly aborted (e.g., AbortError came from an old cached promise's - // signal), the error was incorrectly set in state. - // - // The fix: also guard on err.name !== 'AbortError' so abort-flavored errors - // are never surfaced regardless of which signal triggered them. - const abortError = new DOMException('The operation was aborted.', 'AbortError'); - mockCoFetchJSON.mockRejectedValueOnce(abortError); - - const { result } = renderHook(() => useOperatorLifecycle('pkg-abort', CATALOG, NS)); - - await act(async () => { - await Promise.resolve(); - await Promise.resolve(); // allow rejection microtask to settle - }); - - const [data, loading, error] = result.current; - expect(error).toBeNull(); // AbortError must not surface as a real error - expect(data).toBeNull(); - expect(loading).toBe(false); - }); - it('surfaces real network errors as errors', async () => { const networkError = new Error('Network failure'); mockCoFetchJSON.mockRejectedValueOnce(networkError); @@ -76,100 +51,64 @@ describe('useOperatorLifecycle', () => { expect(loading).toBe(false); }); - it('retries with a fresh request when the cached in-flight promise was aborted by another caller', async () => { - // Fix B scenario: two hook instances share the same cache key. - // Hook A starts the fetch (P1 enters cache). Hook B arrives and gets P1 from cache. - // Hook A is then aborted — P1 rejects with AbortError. - // Fix B detects that B's signal is still live and retries with B's signal. - const retryData = { lifecycleSchema: 'retry', properties: '{}' }; - let rejectP1: (err: Error) => void; - - // Hook A's fetch hangs until we manually reject it - mockCoFetchJSON.mockImplementationOnce( - () => - new Promise((_resolve, reject) => { - rejectP1 = reject; - }), - ); - // Hook B's retry fetch resolves immediately - mockCoFetchJSON.mockResolvedValueOnce(retryData); - - const PKG = 'pkg-cached-shared'; - - // Hook A: effect runs → P1 created and stored in cache - const { unmount: unmountA } = renderHook(() => useOperatorLifecycle(PKG, CATALOG, NS)); - - // Hook B: same key → effect finds P1 in cache → Fix B wraps P1 with B's signal - const { result: resultB } = renderHook(() => useOperatorLifecycle(PKG, CATALOG, NS)); - - await act(async () => { - unmountA(); // aborts A's controller (signal_A); our mock doesn't react to signals - // Manually reject P1 to simulate what signal_A abort does to the real fetch - rejectP1(new DOMException('The operation was aborted.', 'AbortError')); - await Promise.resolve(); - await Promise.resolve(); - await Promise.resolve(); // allow retry to complete - }); + it('skips fetch when packageName is missing', () => { + renderHook(() => useOperatorLifecycle(undefined, CATALOG, NS)); - const [data, loading, error] = resultB.current; - expect(error).toBeNull(); - expect(data).toEqual(retryData); // retry produced real data + expect(mockCoFetchJSON).not.toHaveBeenCalled(); + const [data, loading, error] = renderHook(() => + useOperatorLifecycle(undefined, CATALOG, NS), + ).result.current; + expect(data).toBeNull(); expect(loading).toBe(false); - expect(mockCoFetchJSON).toHaveBeenCalledTimes(2); // P1 (aborted) + P2 (retry) + expect(error).toBeNull(); }); - it('issues only one retry when three callers share the same aborted cached promise', async () => { - // Regression for: multiple callers each unconditionally deleting the cache and - // starting their own retry, causing N redundant fetches. - // Only the first caller to detect the abort should start a retry; the others - // should reuse the retry promise already stored by the first. - const retryData = { lifecycleSchema: 'three-caller', properties: '{}' }; - let rejectP1: (err: Error) => void; - - // P1: hangs until manually rejected - mockCoFetchJSON.mockImplementationOnce( - () => - new Promise((_resolve, reject) => { - rejectP1 = reject; - }), - ); - // P2: the single retry (only one should be started) - mockCoFetchJSON.mockResolvedValueOnce(retryData); - - const PKG = 'pkg-three-callers'; - - // Mount A, B, C — all share the same cache key; B and C get P1 from cache - const { unmount: unmountA } = renderHook(() => useOperatorLifecycle(PKG, CATALOG, NS)); + it('multiple callers sharing the same cache key all receive data from a single fetch', async () => { + // Core deduplication invariant: N components for the same operator should never + // issue more than one in-flight request. + const sharedData = { lifecycleSchema: 'shared', properties: '{}' }; + mockCoFetchJSON.mockResolvedValueOnce(sharedData); + + const PKG = 'pkg-shared-dedup'; + + // All three mount and subscribe to the same in-flight promise + const { result: resultA } = renderHook(() => useOperatorLifecycle(PKG, CATALOG, NS)); const { result: resultB } = renderHook(() => useOperatorLifecycle(PKG, CATALOG, NS)); const { result: resultC } = renderHook(() => useOperatorLifecycle(PKG, CATALOG, NS)); await act(async () => { - unmountA(); - rejectP1(new DOMException('The operation was aborted.', 'AbortError')); - await Promise.resolve(); await Promise.resolve(); await Promise.resolve(); }); - // Both B and C should see the retry data — not an error, not null - expect(resultB.current[2]).toBeNull(); - expect(resultB.current[0]).toEqual(retryData); - expect(resultC.current[2]).toBeNull(); - expect(resultC.current[0]).toEqual(retryData); - - // Only two coFetchJSON calls total: P1 (aborted) + P2 (single retry) - expect(mockCoFetchJSON).toHaveBeenCalledTimes(2); + expect(resultA.current[0]).toEqual(sharedData); + expect(resultB.current[0]).toEqual(sharedData); + expect(resultC.current[0]).toEqual(sharedData); + // Only one fetch regardless of how many components share the key + expect(mockCoFetchJSON).toHaveBeenCalledTimes(1); }); - it('skips fetch when packageName is missing', () => { - renderHook(() => useOperatorLifecycle(undefined, CATALOG, NS)); + it('a component mounting after the cache is populated serves data immediately without fetching', async () => { + const cachedData = { lifecycleSchema: 'cached', properties: '{}' }; + mockCoFetchJSON.mockResolvedValueOnce(cachedData); - expect(mockCoFetchJSON).not.toHaveBeenCalled(); - const [data, loading, error] = renderHook(() => - useOperatorLifecycle(undefined, CATALOG, NS), - ).result.current; - expect(data).toBeNull(); - expect(loading).toBe(false); - expect(error).toBeNull(); + const PKG = 'pkg-cache-hit'; + + // First component populates the cache + const { result: result1 } = renderHook(() => useOperatorLifecycle(PKG, CATALOG, NS)); + + await act(async () => { + await Promise.resolve(); + }); + + expect(result1.current[0]).toEqual(cachedData); + expect(mockCoFetchJSON).toHaveBeenCalledTimes(1); + + // Second component — cache hit, no new fetch + const { result: result2 } = renderHook(() => useOperatorLifecycle(PKG, CATALOG, NS)); + expect(result2.current[0]).toEqual(cachedData); + expect(result2.current[1]).toBe(false); // not loading + expect(result2.current[2]).toBeNull(); + expect(mockCoFetchJSON).toHaveBeenCalledTimes(1); // still just one fetch }); }); diff --git a/frontend/packages/operator-lifecycle-manager/src/hooks/useOperatorLifecycle.ts b/frontend/packages/operator-lifecycle-manager/src/hooks/useOperatorLifecycle.ts index f21e7478c39..2a7a2cf51d4 100644 --- a/frontend/packages/operator-lifecycle-manager/src/hooks/useOperatorLifecycle.ts +++ b/frontend/packages/operator-lifecycle-manager/src/hooks/useOperatorLifecycle.ts @@ -1,4 +1,4 @@ -import { useEffect, useRef, useState } from 'react'; +import { useEffect, useState } from 'react'; import { coFetchJSON } from '@console/shared/src/utils/console-fetch'; import type { LifecycleData } from '../components/operator-lifecycle-status'; import { DEFAULT_SOURCE_NAMESPACE } from '../const'; @@ -41,62 +41,28 @@ const extractPackageName = (olmProperties: string): string | undefined => { } }; -const fetchLifecycleData = ( - cacheKey: string, - url: string, - signal: AbortSignal, -): Promise => { +// Fetch lifecycle data and deduplicate concurrent requests via a module-level cache. +// Requests are intentionally not cancelled on component unmount: the response +// populates the cache so any later render of the same operator avoids a re-fetch, +// and React silently ignores state updates after unmount. +const fetchLifecycleData = (cacheKey: string, url: string): Promise => { const existing = lifecycleCache.get(cacheKey); if (existing?.promise) { - // The cached promise is tied to the ORIGINAL caller's signal, not ours. - // If the original was aborted by someone else while our signal is still live, - // issue a fresh retry rather than surfacing a spurious AbortError. - // Capture a reference so multiple concurrent callers agree on which promise - // they are watching: only the first one to run this .catch() will find the - // cache still pointing at existingPromise and start a single retry; all - // later callers will see a newer promise in the cache and reuse it. - const existingPromise = existing.promise; - return existingPromise.catch((err: Error) => { - if (err.name === 'AbortError' && !signal.aborted) { - if (lifecycleCache.get(cacheKey)?.promise === existingPromise) { - lifecycleCache.delete(cacheKey); - } - return fetchLifecycleData(cacheKey, url, signal); - } - throw err; - }); + return existing.promise; // deduplicate: all callers share the one in-flight request } - const promise = coFetchJSON(url, 'GET', { signal }).then( + const promise = coFetchJSON(url, 'GET', {}).then( (result: LifecycleData) => { - lifecycleCache.set(cacheKey, { - data: result, - error: null, - timestamp: Date.now(), - }); + lifecycleCache.set(cacheKey, { data: result, error: null, timestamp: Date.now() }); return result; }, (err: Error) => { - if (err.name === 'AbortError') { - lifecycleCache.delete(cacheKey); - } else { - lifecycleCache.set(cacheKey, { - data: null, - error: err, - timestamp: Date.now(), - }); - } + lifecycleCache.set(cacheKey, { data: null, error: err, timestamp: Date.now() }); throw err; }, ); - lifecycleCache.set(cacheKey, { - data: null, - error: null, - timestamp: Date.now(), - promise, - }); - + lifecycleCache.set(cacheKey, { data: null, error: null, timestamp: Date.now(), promise }); return promise; }; @@ -108,7 +74,6 @@ export const useOperatorLifecycle = ( const [data, setData] = useState(null); const [loading, setLoading] = useState(false); const [error, setError] = useState(null); - const abortRef = useRef(); useEffect(() => { if (!packageName || !catalogName || !catalogNamespace) { @@ -126,37 +91,25 @@ export const useOperatorLifecycle = ( return undefined; } - abortRef.current?.abort(); - const controller = new AbortController(); - abortRef.current = controller; - setLoading(true); const url = `/api/olm/lifecycle/${encodeURIComponent(catalogNamespace)}/${encodeURIComponent( catalogName, )}/${encodeURIComponent(packageName)}`; - fetchLifecycleData(cacheKey, url, controller.signal) + fetchLifecycleData(cacheKey, url) .then((result) => { - if (!controller.signal.aborted) { - setData(result); - setError(null); - setLoading(false); - } + setData(result); + setError(null); + setLoading(false); }) .catch((err: Error) => { - if (!controller.signal.aborted) { - setLoading(false); - if (err.name !== 'AbortError') { - setData(null); - setError(err); - } - } + setData(null); + setError(err); + setLoading(false); }); - return () => { - controller.abort(); - }; + return undefined; // no cleanup: requests complete naturally }, [packageName, catalogName, catalogNamespace]); return [data, loading, error]; From 4d62a7835ea9e3e09fb000ba133234e17e4a5290 Mon Sep 17 00:00:00 2001 From: "Per G. da Silva" Date: Sat, 18 Jul 2026 09:27:56 +0200 Subject: [PATCH 6/6] NO-ISSUE: Add width constraints to lifecycle table columns The Cluster compatibility and Support phase columns shared the same responsive class as Provided APIs (pf-m-hidden pf-m-visible-on-xl) with no explicit width. When lifecycle data loads successfully and badge content renders (e.g. "Incompatible", "General availability") instead of the "-" fallback, the unconstrained columns compete with Provided APIs for space, causing content to overflow and overlap. Fix: add pf-m-width-15 to both lifecycle columns so they each claim a fixed 15% of table width, keeping them bounded and leaving Provided APIs its natural allocation. Co-Authored-By: Claude Sonnet 4.6 (1M context) --- .../src/components/clusterserviceversion.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/packages/operator-lifecycle-manager/src/components/clusterserviceversion.tsx b/frontend/packages/operator-lifecycle-manager/src/components/clusterserviceversion.tsx index d7a5de3124e..b97656eb2fe 100644 --- a/frontend/packages/operator-lifecycle-manager/src/components/clusterserviceversion.tsx +++ b/frontend/packages/operator-lifecycle-manager/src/components/clusterserviceversion.tsx @@ -166,8 +166,8 @@ const managedNamespacesColumnClass = css('pf-m-hidden', 'pf-m-visible-on-sm'); const statusColumnClass = css('pf-m-hidden', 'pf-m-visible-on-lg'); const lastUpdatedColumnClass = css('pf-m-hidden', 'pf-m-visible-on-2xl'); const providedAPIsColumnClass = css('pf-m-hidden', 'pf-m-visible-on-xl'); -const clusterCompatibilityColumnClass = css('pf-m-hidden', 'pf-m-visible-on-xl'); -const supportPhaseColumnClass = css('pf-m-hidden', 'pf-m-visible-on-xl'); +const clusterCompatibilityColumnClass = css('pf-m-hidden', 'pf-m-visible-on-xl', 'pf-m-width-15'); +const supportPhaseColumnClass = css('pf-m-hidden', 'pf-m-visible-on-xl', 'pf-m-width-15'); const SubscriptionStatus: FC<{ muted?: boolean; subscription: SubscriptionKind }> = ({ muted = false,