Skip to content

Commit 8e267ed

Browse files
WintusWintus
committed
refactor(PageTitleInput): fix stuck ErrorBoundary with react-error-boundary
Replace custom ErrorBoundary class with react-error-boundary library and add QueryErrorResetBoundary to properly reset both React and TanStack Query error states. Key improvements: - Install react-error-boundary (v6.1.0) - Use QueryErrorResetBoundary + ErrorBoundary combination - Use resetKeys={[debouncedTitle]} instead of key prop (no unmount) - Distinguish 404 vs transient errors via error.cause.status - 404 errors show "Page not found." (no retry) - Transient errors show retry button with type="button" - Add comprehensive tests for all error scenarios Fixes the two-layer stuck state where neither React's error boundary nor TanStack Query's cached error would reset on retry attempts. Co-Authored-By: Claude
1 parent 75f072d commit 8e267ed

File tree

3 files changed

+240
-76
lines changed

3 files changed

+240
-76
lines changed

‎package-lock.json‎

Lines changed: 12 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@
2020
"@tanstack/react-query": "^5.90.20",
2121
"@tanstack/react-query-devtools": "^5.91.3",
2222
"react": "^19.2.4",
23-
"react-dom": "^19.2.4"
23+
"react-dom": "^19.2.4",
24+
"react-error-boundary": "^6.1.0"
2425
},
2526
"devDependencies": {
2627
"@eslint/js": "^9.39.2",

‎src/components/PageTitleInput.tsx‎

Lines changed: 226 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -1,42 +1,18 @@
1-
import { Component, type ReactNode, Suspense, useState } from 'react';
1+
import { Suspense, useState } from 'react';
22
import { useDebouncedValue } from '@tanstack/react-pacer';
3+
import { ErrorBoundary } from 'react-error-boundary';
4+
import { QueryErrorResetBoundary } from '@tanstack/react-query';
35
import { PageIdFetcher } from './PageIdFetcher';
46

57
interface PageTitleInputProps {
68
initialPageTitle: string;
79
wikiUrl: URL;
810
}
911

10-
interface ErrorBoundaryProps {
11-
children: ReactNode;
12-
fallback: ReactNode;
13-
}
14-
15-
interface ErrorBoundaryState {
16-
hasError: boolean;
17-
}
18-
19-
class ErrorBoundary extends Component<ErrorBoundaryProps, ErrorBoundaryState> {
20-
constructor(props: ErrorBoundaryProps) {
21-
super(props);
22-
this.state = { hasError: false };
23-
}
24-
25-
static getDerivedStateFromError(): ErrorBoundaryState {
26-
return { hasError: true };
27-
}
28-
29-
componentDidCatch(error: Error): void {
30-
console.error('Error fetching page ID:', error);
31-
}
32-
33-
render() {
34-
if (this.state.hasError) {
35-
return this.props.fallback;
36-
}
37-
return this.props.children;
38-
}
39-
}
12+
const isNotFoundError = (error: unknown): boolean =>
13+
error instanceof Error &&
14+
error.cause instanceof Response &&
15+
error.cause.status === 404;
4016

4117
export function PageTitleInput({
4218
initialPageTitle,
@@ -58,33 +34,48 @@ export function PageTitleInput({
5834
required
5935
/>
6036

61-
<ErrorBoundary
62-
fallback={
63-
<>
64-
<div className="status-message-container">
65-
<span className="error-message">
66-
Error fetching page ID. Please try again.
67-
</span>
68-
</div>
69-
<input
70-
type="hidden"
71-
name="pageId"
72-
value=""
73-
data-testid="pageId-input"
74-
/>
75-
</>
76-
}
77-
>
78-
<Suspense
79-
fallback={
80-
<div className="status-message-container">
81-
<span className="loading-indicator">Checking title...</span>
82-
</div>
83-
}
84-
>
85-
<PageIdFetcher wikiUrl={wikiUrl} pageTitle={debouncedTitle} />
86-
</Suspense>
87-
</ErrorBoundary>
37+
<QueryErrorResetBoundary>
38+
{({ reset }) => (
39+
<ErrorBoundary
40+
resetKeys={[debouncedTitle]}
41+
onReset={reset}
42+
fallbackRender={({ error, resetErrorBoundary }) => (
43+
<>
44+
<div className="status-message-container">
45+
<span className="error-message">
46+
{isNotFoundError(error) ? (
47+
'Page not found.'
48+
) : (
49+
<>
50+
Error fetching page ID.{' '}
51+
<button type="button" onClick={resetErrorBoundary}>
52+
Try again
53+
</button>
54+
</>
55+
)}
56+
</span>
57+
</div>
58+
<input
59+
type="hidden"
60+
name="pageId"
61+
value=""
62+
data-testid="pageId-input"
63+
/>
64+
</>
65+
)}
66+
>
67+
<Suspense
68+
fallback={
69+
<div className="status-message-container">
70+
<span className="loading-indicator">Checking title...</span>
71+
</div>
72+
}
73+
>
74+
<PageIdFetcher wikiUrl={wikiUrl} pageTitle={debouncedTitle} />
75+
</Suspense>
76+
</ErrorBoundary>
77+
)}
78+
</QueryErrorResetBoundary>
8879
</div>
8980
);
9081
}
@@ -259,8 +250,11 @@ if (import.meta.vitest) {
259250
expect(hiddenInput.value).toBe('123');
260251
});
261252

262-
it('displays error message and sets empty hidden input when fetch fails', async () => {
263-
mockFetchPageId.mockRejectedValue(new Error('API Error'));
253+
it('displays 404 error message without retry button', async () => {
254+
const mockResponse = new Response('Not Found', { status: 404 });
255+
mockFetchPageId.mockRejectedValue(
256+
new Error('Failed to fetch page ID', { cause: mockResponse })
257+
);
264258
const consoleErrorSpy = vi
265259
.spyOn(console, 'error')
266260
.mockImplementation(() => {});
@@ -273,26 +267,187 @@ if (import.meta.vitest) {
273267
/>
274268
)
275269
);
276-
// Wait for the API call
277-
expect(mockFetchPageId).toHaveBeenCalledTimes(1);
278-
expect(screen.getByTestId('pageId-input')).toBeInTheDocument();
270+
271+
const errorMessage = await screen.findByText('Page not found.');
272+
expect(errorMessage).toBeInTheDocument();
273+
274+
// Should NOT have a retry button for 404
275+
expect(
276+
screen.queryByRole('button', { name: /try again/i })
277+
).not.toBeInTheDocument();
279278

280279
const hiddenInput = screen.getByTestId(
281280
'pageId-input'
282281
) as HTMLInputElement;
283282
expect(hiddenInput.value).toBe('');
284-
expect(consoleErrorSpy).toHaveBeenCalledWith(
285-
'Error fetching page ID:',
286-
expect.any(Error)
283+
consoleErrorSpy.mockRestore();
284+
});
285+
286+
it('displays error message with retry button for network errors', async () => {
287+
mockFetchPageId.mockRejectedValue(new Error('Network error'));
288+
const consoleErrorSpy = vi
289+
.spyOn(console, 'error')
290+
.mockImplementation(() => {});
291+
292+
await act(async () =>
293+
renderWithQuery(
294+
<PageTitleInput
295+
initialPageTitle="Test Title"
296+
wikiUrl={stubWikiUrl}
297+
/>
298+
)
287299
);
300+
288301
const errorMessage = await screen.findByText(
289-
'Error fetching page ID. Please try again.',
290-
{},
291-
{
292-
timeout: 300,
293-
}
302+
/Error fetching page ID\./
303+
);
304+
expect(errorMessage).toBeInTheDocument();
305+
306+
// Should have a retry button for network errors
307+
const retryButton = screen.getByRole('button', { name: /try again/i });
308+
expect(retryButton).toBeInTheDocument();
309+
expect(retryButton).toHaveAttribute('type', 'button');
310+
311+
const hiddenInput = screen.getByTestId(
312+
'pageId-input'
313+
) as HTMLInputElement;
314+
expect(hiddenInput.value).toBe('');
315+
consoleErrorSpy.mockRestore();
316+
});
317+
318+
it('displays error message with retry button for 5XX server errors', async () => {
319+
const mockResponse = new Response('Server Error', { status: 500 });
320+
mockFetchPageId.mockRejectedValue(
321+
new Error('Failed to fetch page ID', { cause: mockResponse })
322+
);
323+
const consoleErrorSpy = vi
324+
.spyOn(console, 'error')
325+
.mockImplementation(() => {});
326+
327+
await act(async () =>
328+
renderWithQuery(
329+
<PageTitleInput
330+
initialPageTitle="Test Title"
331+
wikiUrl={stubWikiUrl}
332+
/>
333+
)
334+
);
335+
336+
const errorMessage = await screen.findByText(
337+
/Error fetching page ID\./
338+
);
339+
expect(errorMessage).toBeInTheDocument();
340+
341+
// Should have a retry button for 5XX errors
342+
const retryButton = screen.getByRole('button', { name: /try again/i });
343+
expect(retryButton).toBeInTheDocument();
344+
345+
const hiddenInput = screen.getByTestId(
346+
'pageId-input'
347+
) as HTMLInputElement;
348+
expect(hiddenInput.value).toBe('');
349+
consoleErrorSpy.mockRestore();
350+
});
351+
352+
it('clicking retry button re-fetches page ID successfully', async () => {
353+
const user = userEvent.setup();
354+
// First call fails, second succeeds
355+
mockFetchPageId
356+
.mockRejectedValueOnce(new Error('Network error'))
357+
.mockResolvedValueOnce(456);
358+
359+
const consoleErrorSpy = vi
360+
.spyOn(console, 'error')
361+
.mockImplementation(() => {});
362+
363+
await act(async () =>
364+
renderWithQuery(
365+
<PageTitleInput
366+
initialPageTitle="Test Title"
367+
wikiUrl={stubWikiUrl}
368+
/>
369+
)
370+
);
371+
372+
// Wait for error state
373+
const retryButton = await screen.findByRole('button', {
374+
name: /try again/i,
375+
});
376+
expect(retryButton).toBeInTheDocument();
377+
expect(mockFetchPageId).toHaveBeenCalledTimes(1);
378+
379+
// Click retry
380+
await user.click(retryButton);
381+
382+
// Should show loading state
383+
const loadingIndicator = await screen.findByText('Checking title...');
384+
expect(loadingIndicator).toBeInTheDocument();
385+
386+
// Wait for success state
387+
await waitForElementToBeRemoved(() =>
388+
screen.getByText('Checking title...')
389+
);
390+
391+
expect(mockFetchPageId).toHaveBeenCalledTimes(2);
392+
const hiddenInput = screen.getByTestId(
393+
'pageId-input'
394+
) as HTMLInputElement;
395+
expect(hiddenInput.value).toBe('456');
396+
consoleErrorSpy.mockRestore();
397+
});
398+
399+
it('recovers from error when typing a different title', async () => {
400+
const user = userEvent.setup();
401+
// First call fails for "Test Title", second succeeds for "New Title"
402+
mockFetchPageId
403+
.mockRejectedValueOnce(new Error('Network error'))
404+
.mockResolvedValueOnce(789);
405+
406+
const consoleErrorSpy = vi
407+
.spyOn(console, 'error')
408+
.mockImplementation(() => {});
409+
410+
await act(async () =>
411+
renderWithQuery(
412+
<PageTitleInput
413+
initialPageTitle="Test Title"
414+
wikiUrl={stubWikiUrl}
415+
/>
416+
)
417+
);
418+
419+
// Wait for error state
420+
const errorMessage = await screen.findByText(
421+
/Error fetching page ID\./
294422
);
295423
expect(errorMessage).toBeInTheDocument();
424+
expect(mockFetchPageId).toHaveBeenCalledTimes(1);
425+
426+
// Change the title
427+
const inputElement = screen.getByLabelText(/Wiki Article Title:/i);
428+
await user.clear(inputElement);
429+
await user.type(inputElement, 'New Title');
430+
431+
// Should show loading state
432+
const loadingIndicator = await screen.findByText(
433+
'Checking title...',
434+
{},
435+
{ timeout: 350 }
436+
);
437+
expect(loadingIndicator).toBeInTheDocument();
438+
439+
// Wait for success state
440+
await waitForElementToBeRemoved(
441+
() => screen.getByText('Checking title...'),
442+
{ timeout: 1000 }
443+
);
444+
445+
expect(mockFetchPageId).toHaveBeenCalledTimes(2);
446+
const hiddenInput = screen.getByTestId(
447+
'pageId-input'
448+
) as HTMLInputElement;
449+
expect(hiddenInput.value).toBe('789');
450+
consoleErrorSpy.mockRestore();
296451
});
297452
});
298453

0 commit comments

Comments
 (0)