Skip to content

Commit 612e39f

Browse files
authored
fix: fix apiFlags being undefined resulting in a crash (#617)
* fix: fix apiFlags being undefined resulting in a crash * refactor: undo unnecessary ts null safety check * test: remove unnecessary tests
1 parent 58c3f04 commit 612e39f

3 files changed

Lines changed: 123 additions & 4 deletions

File tree

packages/toolbar/src/core/tests/FlagsProvider.test.tsx

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,126 @@ describe('FlagsProvider', () => {
415415
});
416416
});
417417

418+
describe('Malformed API Response Resilience', () => {
419+
test('handles API response missing items property', async () => {
420+
// GIVEN: API returns a response without an items property (e.g. unexpected iframe response)
421+
mockGetFlags.mockResolvedValue({});
422+
423+
// WHEN: Flags are loaded
424+
render(
425+
<TestWrapper>
426+
<FlagsProvider>
427+
<TestConsumer />
428+
</FlagsProvider>
429+
</TestWrapper>,
430+
);
431+
432+
// THEN: Falls back to empty array instead of crashing
433+
await waitFor(() => {
434+
expect(screen.getByTestId('loading')).toHaveTextContent('false');
435+
});
436+
expect(screen.getByTestId('flags-count')).toHaveTextContent('0');
437+
});
438+
439+
test('handles API response with undefined items', async () => {
440+
// GIVEN: API returns { items: undefined }
441+
mockGetFlags.mockResolvedValue({ items: undefined });
442+
443+
// WHEN: Flags are loaded
444+
render(
445+
<TestWrapper>
446+
<FlagsProvider>
447+
<TestConsumer />
448+
</FlagsProvider>
449+
</TestWrapper>,
450+
);
451+
452+
// THEN: Falls back to empty array
453+
await waitFor(() => {
454+
expect(screen.getByTestId('loading')).toHaveTextContent('false');
455+
});
456+
expect(screen.getByTestId('flags-count')).toHaveTextContent('0');
457+
});
458+
459+
test('handles null API response', async () => {
460+
// GIVEN: API returns null (e.g. network issue or iframe communication failure)
461+
mockGetFlags.mockResolvedValue(null);
462+
463+
// WHEN: Flags are loaded
464+
render(
465+
<TestWrapper>
466+
<FlagsProvider>
467+
<TestConsumer />
468+
</FlagsProvider>
469+
</TestWrapper>,
470+
);
471+
472+
// THEN: Falls back to empty array
473+
await waitFor(() => {
474+
expect(screen.getByTestId('loading')).toHaveTextContent('false');
475+
});
476+
expect(screen.getByTestId('flags-count')).toHaveTextContent('0');
477+
});
478+
479+
test('handles undefined API response', async () => {
480+
// GIVEN: API returns undefined
481+
mockGetFlags.mockResolvedValue(undefined);
482+
483+
// WHEN: Flags are loaded
484+
render(
485+
<TestWrapper>
486+
<FlagsProvider>
487+
<TestConsumer />
488+
</FlagsProvider>
489+
</TestWrapper>,
490+
);
491+
492+
// THEN: Falls back to empty array
493+
await waitFor(() => {
494+
expect(screen.getByTestId('loading')).toHaveTextContent('false');
495+
});
496+
expect(screen.getByTestId('flags-count')).toHaveTextContent('0');
497+
});
498+
499+
test('recovers from malformed response when valid response follows', async () => {
500+
// GIVEN: First API call returns malformed response
501+
mockGetFlags.mockResolvedValue({});
502+
503+
const { rerender } = render(
504+
<TestWrapper>
505+
<FlagsProvider>
506+
<TestConsumer />
507+
</FlagsProvider>
508+
</TestWrapper>,
509+
);
510+
511+
await waitFor(() => {
512+
expect(screen.getByTestId('loading')).toHaveTextContent('false');
513+
});
514+
expect(screen.getByTestId('flags-count')).toHaveTextContent('0');
515+
516+
// WHEN: Project changes and the next API call returns valid data
517+
(useProjectContext as any).mockReturnValue({ projectKey: 'new-project' });
518+
mockGetFlags.mockResolvedValue(
519+
createFlagsResponse([{ key: 'flag-1', name: 'Flag 1', kind: 'boolean' } as ApiFlag]),
520+
);
521+
522+
rerender(
523+
<TestWrapper>
524+
<FlagsProvider>
525+
<TestConsumer />
526+
</FlagsProvider>
527+
</TestWrapper>,
528+
);
529+
530+
// THEN: Flags load correctly after recovery
531+
await waitFor(() => {
532+
expect(screen.getByTestId('flags-count')).toHaveTextContent('1');
533+
});
534+
expect(screen.getByTestId('flag-flag-1')).toHaveTextContent('Flag 1');
535+
});
536+
});
537+
418538
describe('Context Hook - useFlagsContext', () => {
419539
test('throws error when used without FlagsProvider', () => {
420540
// GIVEN: Component uses context without provider

packages/toolbar/src/core/ui/Toolbar/context/FlagSdkOverrideProvider.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,6 @@ export function FlagSdkOverrideProvider({ children, flagOverridePlugin }: FlagSd
6363
(allFlags: Record<string, any>, apiFlags: ApiFlag[]): Record<string, LocalFlag> => {
6464
const overrides = flagOverridePlugin.getAllOverrides();
6565

66-
// Create a map of API flags for quick lookup
6766
const apiFlagsMap = new Map<string, ApiFlag>();
6867
apiFlags.forEach((apiFlag) => {
6968
apiFlagsMap.set(apiFlag.key, apiFlag);

packages/toolbar/src/core/ui/Toolbar/context/api/FlagsProvider.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ export const FlagsProvider = ({ children }: { children: React.ReactNode }) => {
4848
setLoading(true);
4949
try {
5050
const flagsResponse = await getProjectFlags(projectKey);
51-
setFlags(flagsResponse.items);
51+
setFlags(flagsResponse?.items ?? []);
5252
setLoadedProjectKey(projectKey);
5353
} catch (error) {
5454
console.error('Error refreshing flags:', error);
@@ -82,9 +82,9 @@ export const FlagsProvider = ({ children }: { children: React.ReactNode }) => {
8282
let isMounted = true;
8383
setLoading(true);
8484
setFlags([]);
85-
getProjectFlags(projectKey).then((flags) => {
85+
getProjectFlags(projectKey).then((response) => {
8686
if (isMounted) {
87-
setFlags(flags.items);
87+
setFlags(response?.items ?? []);
8888
setLoadedProjectKey(projectKey);
8989
setHasInitialLoad(true);
9090
setLoading(false);

0 commit comments

Comments
 (0)