Skip to content

Commit 08de5ae

Browse files
committed
frontend: apiDiscovery: Tighten fallback logging and spy cleanup
Bound the aggregated-discovery fallback log to a type plus the top-level key list (capped at 10 entries) instead of dumping the whole payload. A real discovery response can carry thousands of entries and burying the devtools obscures the actual signal we wanted to surface in #4840. Errors on the rejected branch still pass through raw so stacks survive. Reword the legacy .catch log to say "fetch or parse" since the same handler also catches res.json() rejections, not only transport errors. Hoist the console.debug spy to a beforeEach/afterEach pair scoped to the new describe block so the cleanup runs even when an assertion throws, and restore only that single spy rather than vi.restoreAllMocks which would clobber the file-level vi.mock for ./fetch. Signed-off-by: Rudraksha Singh Sengar <rudraksharss@gmail.com>
1 parent 2e5a5b0 commit 08de5ae

2 files changed

Lines changed: 69 additions & 14 deletions

File tree

frontend/src/lib/k8s/api/v2/apiDiscovery.test.ts

Lines changed: 39 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,16 @@
1414
* limitations under the License.
1515
*/
1616

17-
import { afterEach, beforeEach, describe, expect, it, MockedFunction, vi } from 'vitest';
17+
import {
18+
afterEach,
19+
beforeEach,
20+
describe,
21+
expect,
22+
it,
23+
MockedFunction,
24+
MockInstance,
25+
vi,
26+
} from 'vitest';
1827
import { apiDiscovery } from './apiDiscovery'; // Adjust path as needed
1928
// Adjust path as needed
2029
import { clusterFetch } from './fetch'; // Adjust path as needed
@@ -582,14 +591,21 @@ describe('apiDiscovery', () => {
582591
// logging, making "missing resources in the UI" extremely hard to debug.
583592
// These tests pin the discovery path to its debug logging contract.
584593
describe('logs the cause of every fallback or skipped fetch (#4840)', () => {
585-
// Restore console.debug even if an assertion throws, so the spy never
586-
// leaks into later tests (the Vitest config has restoreMocks disabled).
594+
// One shared spy created in beforeEach and torn down in afterEach so the
595+
// cleanup runs even when an assertion throws. Restoring the single spy
596+
// (rather than `vi.restoreAllMocks()`) leaves the file-level `vi.mock`
597+
// for `./fetch` intact and avoids cross-test interference.
598+
let debugSpy: MockInstance;
599+
600+
beforeEach(() => {
601+
debugSpy = vi.spyOn(console, 'debug').mockImplementation(() => {});
602+
});
603+
587604
afterEach(() => {
588-
vi.restoreAllMocks();
605+
debugSpy.mockRestore();
589606
});
590607

591608
it('logs the rejection reason when aggregated /api rejects', async () => {
592-
const debugSpy = vi.spyOn(console, 'debug').mockImplementation(() => {});
593609
const networkError = new Error('aggregated /api network down');
594610

595611
mockClusterFetch
@@ -608,7 +624,6 @@ describe('apiDiscovery', () => {
608624
});
609625

610626
it('logs the unusable payload when aggregated returns no items array', async () => {
611-
const debugSpy = vi.spyOn(console, 'debug').mockImplementation(() => {});
612627
const malformed = { unexpected: 'shape' };
613628

614629
mockClusterFetch
@@ -623,10 +638,21 @@ describe('apiDiscovery', () => {
623638
expect(
624639
messages.some(m => m.includes('Aggregated /api discovery unusable for cluster cluster1'))
625640
).toBe(true);
641+
// The unusable payload itself is logged as a bounded summary (type +
642+
// top-level keys), not the raw body. Verifying the keys list keeps the
643+
// contract honest without coupling the test to summary internals.
644+
const summariesWithUnexpectedKey = debugSpy.mock.calls
645+
.flat()
646+
.filter((arg): arg is { keys: string[] } => {
647+
if (!arg || typeof arg !== 'object') return false;
648+
const keys = (arg as { keys?: unknown }).keys;
649+
return Array.isArray(keys) && keys.every(k => typeof k === 'string');
650+
})
651+
.filter(summary => summary.keys.includes('unexpected'));
652+
expect(summariesWithUnexpectedKey.length).toBeGreaterThan(0);
626653
});
627654

628655
it('logs the rejection reason when the legacy /api fetch rejects', async () => {
629-
const debugSpy = vi.spyOn(console, 'debug').mockImplementation(() => {});
630656
const legacyApiError = new Error('legacy /api dropped');
631657

632658
mockClusterFetch
@@ -639,13 +665,14 @@ describe('apiDiscovery', () => {
639665

640666
const messages = debugSpy.mock.calls.map(args => String(args[0]));
641667
expect(
642-
messages.some(m => m.includes('Legacy /api discovery fetch failed for cluster cluster1'))
668+
messages.some(m =>
669+
m.includes('Legacy /api discovery fetch or parse failed for cluster cluster1')
670+
)
643671
).toBe(true);
644672
expect(debugSpy.mock.calls.some(args => args.includes(legacyApiError))).toBe(true);
645673
});
646674

647675
it('logs the rejection reason when the legacy /apis fetch rejects', async () => {
648-
const debugSpy = vi.spyOn(console, 'debug').mockImplementation(() => {});
649676
const legacyApisError = new Error('legacy /apis dropped');
650677

651678
mockClusterFetch
@@ -658,7 +685,9 @@ describe('apiDiscovery', () => {
658685

659686
const messages = debugSpy.mock.calls.map(args => String(args[0]));
660687
expect(
661-
messages.some(m => m.includes('Legacy /apis discovery fetch failed for cluster cluster1'))
688+
messages.some(m =>
689+
m.includes('Legacy /apis discovery fetch or parse failed for cluster cluster1')
690+
)
662691
).toBe(true);
663692
expect(debugSpy.mock.calls.some(args => args.includes(legacyApisError))).toBe(true);
664693
});

frontend/src/lib/k8s/api/v2/apiDiscovery.tsx

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,22 @@
1717
import type { ApiResource } from './ApiResource';
1818
import { clusterFetch } from './fetch';
1919

20+
/**
21+
* Produces a bounded representation of an aggregated-discovery payload for
22+
* fallback debug logs. A real discovery response can carry thousands of
23+
* entries; dumping it to the console slows devtools and buries the actual
24+
* "why was this unusable" signal. We log the value's type plus the first few
25+
* top-level keys, which is enough to tell a missing `items` array apart from
26+
* a wholly unexpected shape without flooding the log.
27+
*/
28+
function summariseAggregatedPayload(value: unknown): unknown {
29+
if (value && typeof value === 'object') {
30+
const keys = Object.keys(value as object);
31+
return { type: 'object', keys: keys.slice(0, 10), keyCount: keys.length };
32+
}
33+
return { type: typeof value, value };
34+
}
35+
2036
/**
2137
* Processes the items from an aggregated API discovery response.
2238
* @param items - The array of discovery items from the Kubernetes API discovery endpoint
@@ -183,15 +199,15 @@ export async function apiDiscovery(clusters: string[]): Promise<ApiResource[]> {
183199
`Aggregated /api discovery unusable for cluster ${cluster}; falling back to legacy:`,
184200
apiAggregatedResult.status === 'rejected'
185201
? apiAggregatedResult.reason
186-
: apiAggregatedResult.value
202+
: summariseAggregatedPayload(apiAggregatedResult.value)
187203
);
188204
}
189205
if (!apisAggregatedOk) {
190206
console.debug(
191207
`Aggregated /apis discovery unusable for cluster ${cluster}; falling back to legacy:`,
192208
apisAggregatedResult.status === 'rejected'
193209
? apisAggregatedResult.reason
194-
: apisAggregatedResult.value
210+
: summariseAggregatedPayload(apisAggregatedResult.value)
195211
);
196212
}
197213
useFallback = true;
@@ -206,16 +222,26 @@ export async function apiDiscovery(clusters: string[]): Promise<ApiResource[]> {
206222

207223
if (useFallback) {
208224
try {
225+
// The `.catch` here covers both the transport (`clusterFetch` rejects
226+
// on a network/HTTP failure) and the JSON parse step (`res.json()`
227+
// rejects on a malformed body), so we say "fetch or parse" rather
228+
// than just "fetch" to keep the log honest about which step blew up.
209229
const coreApiVersionsPromise = clusterFetch('/api', { cluster })
210230
.then(res => res.json())
211231
.catch(err => {
212-
console.debug(`Legacy /api discovery fetch failed for cluster ${cluster}:`, err);
232+
console.debug(
233+
`Legacy /api discovery fetch or parse failed for cluster ${cluster}:`,
234+
err
235+
);
213236
return null;
214237
});
215238
const apiGroupsPromise = clusterFetch('/apis', { cluster })
216239
.then(res => res.json())
217240
.catch(err => {
218-
console.debug(`Legacy /apis discovery fetch failed for cluster ${cluster}:`, err);
241+
console.debug(
242+
`Legacy /apis discovery fetch or parse failed for cluster ${cluster}:`,
243+
err
244+
);
219245
return null;
220246
});
221247

0 commit comments

Comments
 (0)