From 0850b6a7fe040360540b6b7cdf29b1c9705f469b Mon Sep 17 00:00:00 2001 From: Roland Date: Thu, 16 Jul 2026 19:40:28 -0600 Subject: [PATCH] app: handle form fetch failures without unhandled rejection Catch useForm fetcher errors and surface them so DomainEditPage can show a not-found message instead of hanging on the loader (and so the webpack overlay no longer flakes Cypress). --- app/src/components/Archive.js | 9 +++++ app/src/components/DomainEditPage.test.js | 33 ++++++++++++---- app/src/hooks/useForm.js | 24 ++++++++++-- app/src/hooks/useForm.test.js | 47 +++++++++++++++++++++++ app/src/test-utils.js | 1 + 5 files changed, 104 insertions(+), 10 deletions(-) diff --git a/app/src/components/Archive.js b/app/src/components/Archive.js index 9416c1b4..6824fff4 100644 --- a/app/src/components/Archive.js +++ b/app/src/components/Archive.js @@ -630,6 +630,15 @@ export function DomainEditPage() { }; if (!form.ready) { + if (form.error) { + return <> + + + Domain not found +

{form.error}

+ + ; + } return Loading domain...; } diff --git a/app/src/components/DomainEditPage.test.js b/app/src/components/DomainEditPage.test.js index c767d78b..0a8ac9ae 100644 --- a/app/src/components/DomainEditPage.test.js +++ b/app/src/components/DomainEditPage.test.js @@ -154,10 +154,13 @@ describe('DomainEditPage', () => { }); describe('Error States', () => { - it('shows loader when form is not ready (fetch fails)', () => { - // When form.ready is false (e.g., fetch failed), show loader + it('shows error message when domain fetch fails', () => { const form = createTestForm({}, { - overrides: {ready: false, loading: false, error: new Error('Domain not found')} + overrides: { + ready: false, + loading: false, + error: 'Domain collection with ID 999 not found', + } }); mockUseDomain.mockReturnValue({ @@ -168,10 +171,26 @@ describe('DomainEditPage', () => { render(); - // Should show loading screen when not ready - expect(screen.getByText(/loading domain/i)).toBeInTheDocument(); + expect(screen.getByText(/domain not found/i)).toBeInTheDocument(); + expect(screen.getByText(/Domain collection with ID 999 not found/i)).toBeInTheDocument(); + expect(screen.queryByText(/loading domain/i)).not.toBeInTheDocument(); + expect(screen.queryByTestId('collection-edit-form')).not.toBeInTheDocument(); + }); - // Form should not be rendered + it('shows loader when form is not ready and there is no error', () => { + const form = createTestForm({}, { + overrides: {ready: false, loading: false, error: null} + }); + + mockUseDomain.mockReturnValue({ + domain: null, + form, + fetchDomain: jest.fn(), + }); + + render(); + + expect(screen.getByText(/loading domain/i)).toBeInTheDocument(); expect(screen.queryByTestId('collection-edit-form')).not.toBeInTheDocument(); }); @@ -182,7 +201,7 @@ describe('DomainEditPage', () => { }); const form = createTestForm(mockDomain, { - overrides: {ready: true, loading: false, error: new Error('Update failed')} + overrides: {ready: true, loading: false, error: 'Update failed'} }); mockUseDomain.mockReturnValue({ diff --git a/app/src/hooks/useForm.js b/app/src/hooks/useForm.js index ff76fa7b..0d205a84 100644 --- a/app/src/hooks/useForm.js +++ b/app/src/hooks/useForm.js @@ -24,6 +24,8 @@ export function useForm({ const [loading, setLoading] = React.useState(false); const [ready, setReady] = React.useState(false); const [dirty, setDirty] = React.useState(false); + // Form-level error (e.g. initial fetch failed). Distinct from field-level `errors`. + const [error, setError] = React.useState(null); const [errors, setErrors] = React.useState({}); const [validators, setValidators] = React.useState({}); @@ -47,6 +49,12 @@ export function useForm({ // Form is dirty if it has deviated from the default. setDirty(!_.isEqual(defaultFormData, memoizedFormData)); + // Fetch/load failures keep the form not ready so pages can show error UI. + if (error) { + console.debug('Form invalid because of form-level error'); + setReady(false); + return + } const errorValues = Object.values(errors).filter(i => !!i); if (errorValues.length > 0) { console.debug('Form invalid because it has errors'); @@ -66,12 +74,21 @@ export function useForm({ return } setReady(true); - }, [loading, memoizedFormData, memoizedErrors, memoizedValidValues, memoizedRequires]); + }, [loading, memoizedFormData, memoizedErrors, memoizedValidValues, memoizedRequires, error]); const localFetch = async () => { if (fetcher) { - const result = await fetcher(); - setFormData(result); + try { + const result = await fetcher(); + setFormData(result); + setError(null); + return result; + } catch (e) { + // Catch so callers (useEffect, page refresh) do not get an unhandled rejection + // (which would show the webpack-dev-server error overlay and break Cypress). + console.error('Failed to fetch form data:', e); + setError(e?.message || String(e) || 'Failed to load'); + } } } @@ -278,6 +295,7 @@ export function useForm({ return { dirty, disabled, + error, fetcher: localFetch, formData, getCustomProps, diff --git a/app/src/hooks/useForm.test.js b/app/src/hooks/useForm.test.js index 5ea0e167..8433ed1a 100644 --- a/app/src/hooks/useForm.test.js +++ b/app/src/hooks/useForm.test.js @@ -60,6 +60,7 @@ describe('useForm', () => { expect(result.current.dirty).toBe(false); expect(result.current.loading).toBe(false); expect(result.current.disabled).toBe(false); + expect(result.current.error).toBe(null); // Ready starts true when there are no validators or required fields expect(result.current.ready).toBe(true); }); @@ -91,6 +92,52 @@ describe('useForm', () => { await waitFor(() => { expect(result.current.formData).toEqual(fetchedData); }); + expect(result.current.error).toBe(null); + }); + + it('sets form error and stays not ready when fetcher rejects', async () => { + const fetcher = jest.fn().mockRejectedValue(new Error('Domain not found')); + + const {result} = renderHook(() => useForm({ + fetcher, + defaultFormData: {}, + submitter: jest.fn(), + })); + + await waitFor(() => { + expect(result.current.error).toBe('Domain not found'); + }); + expect(result.current.ready).toBe(false); + expect(consoleErrorSpy).toHaveBeenCalledWith( + 'Failed to fetch form data:', + expect.any(Error), + ); + }); + + it('clears form error when a subsequent fetch succeeds', async () => { + const fetcher = jest.fn() + .mockRejectedValueOnce(new Error('Domain not found')) + .mockResolvedValueOnce({name: 'recovered'}); + + const {result} = renderHook(() => useForm({ + fetcher, + defaultFormData: {}, + submitter: jest.fn(), + })); + + await waitFor(() => { + expect(result.current.error).toBe('Domain not found'); + }); + + await act(async () => { + await result.current.fetcher(); + }); + + await waitFor(() => { + expect(result.current.error).toBe(null); + expect(result.current.formData).toEqual({name: 'recovered'}); + }); + expect(result.current.ready).toBe(true); }); it('initializes with empty object when no defaultFormData provided', () => { diff --git a/app/src/test-utils.js b/app/src/test-utils.js index 9966ad41..151f96eb 100644 --- a/app/src/test-utils.js +++ b/app/src/test-utils.js @@ -245,6 +245,7 @@ export function createTestForm(initialData = {}, config = {}) { loading: false, disabled: false, dirty: false, + error: null, errors: {}, // Methods that update formData directly patchFormData: jest.fn((updates) => {