diff --git a/frontend/src/__tests__/riexchange.test.ts b/frontend/src/__tests__/riexchange.test.ts index e9f42d41c..4167aaff0 100644 --- a/frontend/src/__tests__/riexchange.test.ts +++ b/frontend/src/__tests__/riexchange.test.ts @@ -820,6 +820,48 @@ describe('reshape recommendations empty state', () => { expect(recsEl.textContent).toContain('1 convertible RI '); expect(recsEl.textContent).not.toContain('none are registered'); }); + + // Issue #1135: the three loads race. An empty reshape response that lands + // before the RI list must not keep the previous scope's count. + it('re-renders the empty-state count when recommendations resolve before the RI list', async () => { + (api.listConvertibleRIs as jest.Mock).mockResolvedValueOnce([sampleRI]); + await loadRIExchange(); + expect(recsEl.textContent).toContain('All 1 convertible RI '); + + let releaseRIs: (v: unknown) => void = () => undefined; + (api.listConvertibleRIs as jest.Mock).mockReturnValueOnce(new Promise(r => { releaseRIs = r; })); + const reload = loadRIExchange(); + await new Promise(r => setTimeout(r, 0)); + // Recommendations are in, RIs are not: no stale "All 1" claim. + expect(api.getReshapeRecommendations).toHaveBeenCalledTimes(2); + expect(recsEl.textContent).not.toContain('All 1 convertible RI'); + expect(recsEl.textContent).toContain('No reshape recommendations right now.'); + + releaseRIs([sampleRI, { ...sampleRI, reserved_instance_id: 'ri-2' }, { ...sampleRI, reserved_instance_id: 'ri-3' }]); + await reload; + expect(recsEl.textContent).toContain('All 3 convertible RIs meet your utilization threshold'); + }); + + it('does not quote a count from a failed RI load', async () => { + (api.listConvertibleRIs as jest.Mock).mockResolvedValueOnce([sampleRI]); + await loadRIExchange(); + (api.listConvertibleRIs as jest.Mock).mockRejectedValueOnce(new Error('boom')); + await loadRIExchange(); + expect(recsEl.textContent).toContain('No reshape recommendations right now.'); + expect(recsEl.textContent).not.toContain('convertible RI'); + }); + + it('keeps the error from a failed reshape load after the RI list arrives', async () => { + let releaseRIs: (v: unknown) => void = () => undefined; + (api.listConvertibleRIs as jest.Mock).mockReturnValueOnce(new Promise(r => { releaseRIs = r; })); + (api.getReshapeRecommendations as jest.Mock).mockRejectedValueOnce(new Error('ce down')); + const reload = loadRIExchange(); + await new Promise(r => setTimeout(r, 0)); + releaseRIs([sampleRI]); + await reload; + expect(recsEl.querySelector('.error')?.textContent).toContain('Failed to load recommendations: ce down'); + expect(recsEl.textContent).not.toContain('utilization threshold'); + }); }); // Staleness banner tests for issue #150. diff --git a/frontend/src/riexchange.ts b/frontend/src/riexchange.ts index 4b0855f7b..f334d7639 100644 --- a/frontend/src/riexchange.ts +++ b/frontend/src/riexchange.ts @@ -35,6 +35,11 @@ import { getCurrentUser } from './state'; // Module state let currentRIs: ConvertibleRI[] = []; +// True once currentRIs holds the convertible-RI list of the scope being +// shown. The reshape empty state only quotes currentRIs.length while this is +// set, so it never prints the previous scope's count, or a count from a list +// that failed to load (issue #1135). +let currentRIsLoaded = false; let currentUtilization: Map = new Map(); let currentRecommendations: ReshapeRecommendation[] = []; const exchangePaymentPattern = /^\d+(?:\.\d+)?$/; @@ -107,6 +112,7 @@ export async function loadRIExchange(): Promise { if (generation !== scopeGeneration) return; utilizationGeneration++; currentRIs = []; + currentRIsLoaded = false; currentUtilization = new Map(); currentRecommendations = []; currentRIAccountID = undefined; @@ -141,11 +147,17 @@ export async function loadRIExchange(): Promise { } // AWS (default / empty provider): full convertible-RI flow. - await Promise.all([ + const [, recsLoaded] = await Promise.all([ loadConvertibleRIs(accountID, generation), loadReshapeRecommendations(generation), loadExchangeHistory(generation), ]); + // The three loads race. When the reshape list came back empty before the + // RI list, its empty state was written without this scope's RI count, so + // render it again now that both are in. A failed reshape load keeps its + // error message (issue #1135). + if (generation !== scopeGeneration) return; + if (recsLoaded && currentRecommendations.length === 0) rerenderReshape(); } /** @@ -155,6 +167,7 @@ export async function loadRIExchange(): Promise { */ function renderGCPEmptyStates(): void { currentRIs = []; + currentRIsLoaded = false; currentUtilization = new Map(); currentRecommendations = []; currentRIAccountID = undefined; @@ -301,6 +314,7 @@ async function loadConvertibleRIs(accountID?: string, generation: number = scope showSkeletonRows(container, 3, 8); currentRIAccountID = accountID; + currentRIsLoaded = false; try { // issue #871: scope to the selected account so the page honours the @@ -308,6 +322,7 @@ async function loadConvertibleRIs(accountID?: string, generation: number = scope const ris = await api.listConvertibleRIs(accountID); if (generation !== scopeGeneration) return; currentRIs = ris; + currentRIsLoaded = true; renderRIsTable(container, accountID); // Load utilization asynchronously (Cost Explorer is slow) utilizationGeneration++; @@ -339,6 +354,7 @@ async function loadExchangeableAzureRIs(accountID: string | undefined, generatio // later provider switch back to AWS starts clean and reshape copy that // reads currentRIs.length isn't skewed by stale AWS rows. currentRIs = []; + currentRIsLoaded = false; currentUtilization = new Map(); currentRIAccountID = undefined; @@ -521,9 +537,11 @@ function appendAzureCell(tr: HTMLTableRowElement, text: string): void { // Reshape recommendations // ────────────────────────────────────────────── -export async function loadReshapeRecommendations(generation: number = scopeGeneration): Promise { +// Resolves true when this call rendered a fresh reshape list (empty or not), +// false when it was superseded, failed, or had nowhere to render. +export async function loadReshapeRecommendations(generation: number = scopeGeneration): Promise { const container = document.getElementById('ri-exchange-recommendations-list'); - if (!container) return; + if (!container) return false; // Issue #344 T3: skeleton rows for the reshape-recommendations table. // 3 rows × 8 cols matches the rendered table shape — see @@ -533,17 +551,19 @@ export async function loadReshapeRecommendations(generation: number = scopeGener try { const resp: ReshapeRecommendationsResponse = await api.getReshapeRecommendations(); - if (generation !== scopeGeneration) return; + if (generation !== scopeGeneration) return false; currentRecommendations = resp.recommendations ?? []; renderRecommendations(container); renderReshapeStalenessBanner(resp.recs_staleness, resp.recs_collected_at); renderExcludedRIsNotice(resp.excluded_ris); + return true; } catch (error) { - if (generation !== scopeGeneration) return; + if (generation !== scopeGeneration) return false; teardownSkeleton(container); renderExcludedRIsNotice(undefined); const err = error as Error; container.innerHTML = `

Failed to load recommendations: ${escapeHtml(err.message)}

`; + return false; } } @@ -1359,9 +1379,13 @@ function renderRecommendations(container: HTMLElement): void { // contains convertibles to be utilized. Prior to this commit the same // message fired regardless of fleet presence, so a brand-new tenant saw // a claim about utilization of a non-existent fleet. - const copy = currentRIs.length === 0 - ? 'Reshape recommendations appear once your accounts have active convertible Reserved Instances — none are registered yet.' - : `All ${currentRIs.length} convertible RI${currentRIs.length === 1 ? '' : 's'} meet your utilization threshold. No reshape needed.`; + // Until this scope's RI list is in (or when it failed to load), say + // nothing about the fleet rather than quote a stale count (#1135). + const copy = !currentRIsLoaded + ? 'No reshape recommendations right now.' + : currentRIs.length === 0 + ? 'Reshape recommendations appear once your accounts have active convertible Reserved Instances — none are registered yet.' + : `All ${currentRIs.length} convertible RI${currentRIs.length === 1 ? '' : 's'} meet your utilization threshold. No reshape needed.`; container.innerHTML = `

${escapeHtml(copy)}

`; return; }