diff --git a/craft/src/hooks/useWhpApi.timeout.test.tsx b/craft/src/hooks/useWhpApi.timeout.test.tsx new file mode 100644 index 0000000..563640b --- /dev/null +++ b/craft/src/hooks/useWhpApi.timeout.test.tsx @@ -0,0 +1,167 @@ +import { describe, test, expect, vi, beforeEach, afterEach } from 'vitest'; +import React from 'react'; +import { createRoot, Root } from 'react-dom/client'; +import { act } from 'react-dom/test-utils'; + +/** + * Integration follow-ups #4/#47 (items 11 + 12): + * - a save / publish / load request that never completes is abandoned after + * API_TIMEOUT_MS with a readable error, so it cannot block the save queue; + * - a failed response whose JSON only has `error` (PHP's readable text) + * shows that text instead of "server returned HTTP 503". + */ +vi.mock('@craftjs/core', () => ({ + useEditor: (collector?: (state: unknown, query: unknown) => Record) => { + const query = { serialize: () => '{}', history: { canUndo: () => false, canRedo: () => false } }; + const actions = { history: { undo: vi.fn(), redo: vi.fn(), clear: vi.fn() }, deserialize: vi.fn() }; + return { actions, query, ...(collector ? collector({}, query) : {}) }; + }, +})); + +import { LoadGate } from '../editor/LoadGate'; +import { EditorConfigProvider } from '../state/EditorConfigContext'; +import { SaveGuardProvider, useSaveGuard } from '../state/SaveGuardContext'; +import { PageProvider } from '../state/PageContext'; +import { SiteDesignProvider } from '../state/SiteDesignContext'; +import { useWhpApi, API_TIMEOUT_MS } from './useWhpApi'; +import { WhpConfig } from '../types'; + +const whpConfig: WhpConfig = { + user: 'u', apiUrl: '/api', csrfToken: 't', siteId: 1, + siteDomain: 'example.com', siteName: 'Example', backUrl: '/', isRoot: false, +}; +const jsonResp = (body: unknown, status = 200) => ({ ok: status >= 200 && status < 300, status, json: async () => body }); +const never = () => new Promise(() => { /* the server never answers */ }); + +let container: HTMLDivElement; +let root: Root; +let fetchMock: ReturnType; +let api: ReturnType; +let guard: ReturnType; +const Probe: React.FC = () => { api = useWhpApi(); guard = useSaveGuard(); return null; }; +const flush = () => act(async () => { for (let i = 0; i < 10; i++) await Promise.resolve(); }); +const calls = (action: string) => fetchMock.mock.calls.filter(([u]) => String(u).includes(`action=${action}`)); + +async function mount(withLoadGate = true) { + container = document.createElement('div'); + document.body.appendChild(container); + act(() => { + root = createRoot(container); + root.render( + + + {withLoadGate && } + + + , + ); + }); + await flush(); +} + +beforeEach(() => { + vi.useFakeTimers(); + fetchMock = vi.fn(); + vi.stubGlobal('fetch', fetchMock); + vi.spyOn(console, 'error').mockImplementation(() => {}); +}); +afterEach(() => { + act(() => { root.unmount(); }); + container.remove(); + vi.useRealTimers(); + vi.unstubAllGlobals(); + vi.restoreAllMocks(); +}); + +describe('request timeout (item 11)', () => { + test('a hung save resolves with a readable timeout error and releases the save queue', async () => { + let saveN = 0; + fetchMock.mockImplementation((url: string) => { + if (String(url).includes('action=load')) return Promise.resolve(jsonResp({ success: true, project: null, rev: 'r1' })); + saveN += 1; + return saveN === 1 ? never() : Promise.resolve(jsonResp({ success: true, rev: 'r2' })); + }); + await mount(); + act(() => { guard.markDirty(); }); + + let first: any = 'pending'; + let second: any = 'pending'; + await act(async () => { void api.save().then((r) => { first = r; }); }); + await act(async () => { void api.save().then((r) => { second = r; }); }); + await flush(); + expect(calls('save')).toHaveLength(1); // the second waits behind the hung one + const signal: AbortSignal = calls('save')[0][1].signal; + expect(signal.aborted).toBe(false); + + await act(async () => { vi.advanceTimersByTime(API_TIMEOUT_MS - 1); }); + await flush(); + expect(first).toBe('pending'); + + await act(async () => { vi.advanceTimersByTime(1); }); + await flush(); + expect(signal.aborted).toBe(true); + expect(first).toMatchObject({ success: false, error: 'timeout' }); + expect(first.message).toMatch(/Save failed: the server did not respond within 30 seconds/); + // Queue released: the waiting save went out and succeeded. + expect(calls('save')).toHaveLength(2); + expect(second).toMatchObject({ success: true, rev: 'r2' }); + }); + + test('a hung publish times out with a readable error', async () => { + fetchMock.mockImplementation((url: string) => { + const u = String(url); + if (u.includes('action=load')) return Promise.resolve(jsonResp({ success: true, project: null, rev: 'r1' })); + if (u.includes('action=save')) return Promise.resolve(jsonResp({ success: true, rev: 'r2' })); + return never(); + }); + await mount(); + let res: any = 'pending'; + await act(async () => { void api.publish().then((r) => { res = r; }); }); + await flush(); + expect(calls('publish')).toHaveLength(1); + await act(async () => { vi.advanceTimersByTime(API_TIMEOUT_MS); }); + await flush(); + expect(res).toMatchObject({ success: false, error: 'timeout' }); + expect(res.message).toMatch(/^Publish failed: the server did not respond/); + }); + + test('a hung load rejects with a readable error', async () => { + fetchMock.mockImplementation(() => never()); + await mount(false); + let err: any = null; + await act(async () => { void api.load().catch((e) => { err = e; }); }); + await act(async () => { vi.advanceTimersByTime(API_TIMEOUT_MS); }); + await flush(); + expect(err).toBeInstanceOf(Error); + expect(err.message).toMatch(/^Load failed: the server did not respond within 30 seconds/); + }); +}); + +describe('readApiResult error text (item 12)', () => { + test('a 503 whose JSON has only `error` shows that text', async () => { + const text = 'Another save of this site is still in progress. Please try again in a moment.'; + fetchMock.mockImplementation((url: string) => { + if (String(url).includes('action=load')) return Promise.resolve(jsonResp({ success: true, project: null, rev: 'r1' })); + return Promise.resolve(jsonResp({ success: false, error: text }, 503)); + }); + await mount(); + let res: any = null; + await act(async () => { res = await api.save(); }); + expect(res).toMatchObject({ success: false, message: text }); + }); + + test('`message` still wins when present; neither -> the HTTP status text', async () => { + let body: any = { success: false, error: 'code', message: 'Readable' }; + fetchMock.mockImplementation((url: string) => { + if (String(url).includes('action=load')) return Promise.resolve(jsonResp({ success: true, project: null, rev: 'r1' })); + return Promise.resolve(jsonResp(body, 500)); + }); + await mount(); + let res: any = null; + await act(async () => { res = await api.save(); }); + expect(res.message).toBe('Readable'); + body = { success: false }; + await act(async () => { res = await api.save(); }); + expect(res.message).toBe('Save failed: server returned HTTP 500'); + }); +}); diff --git a/craft/src/hooks/useWhpApi.ts b/craft/src/hooks/useWhpApi.ts index 3994032..75071e3 100644 --- a/craft/src/hooks/useWhpApi.ts +++ b/craft/src/hooks/useWhpApi.ts @@ -263,6 +263,38 @@ export interface ApiResult { [key: string]: unknown; } +/** How long save / publish / load may take before the request is abandoned. */ +export const API_TIMEOUT_MS = 30_000; + +/** A request that got no complete answer within API_TIMEOUT_MS. */ +export class ApiTimeoutError extends Error {} + +/** + * Runs `fn` (a fetch plus reading its body) with an AbortSignal that fires + * after `ms`. Racing the timeout as well means a stalled body read can't hang + * the caller either: without this a request that never completes holds the + * save queue forever. + */ +async function withTimeout(action: string, fn: (signal: AbortSignal) => Promise, ms = API_TIMEOUT_MS): Promise { + const ctrl = new AbortController(); + let timer: ReturnType | undefined; + const timeout = new Promise((_, reject) => { + timer = setTimeout(() => { + ctrl.abort(); + reject(new ApiTimeoutError( + `${action} failed: the server did not respond within ${Math.round(ms / 1000)} seconds. Check your connection and try again.`, + )); + }, ms); + }); + try { + return await Promise.race([fn(ctrl.signal), timeout]); + } finally { + clearTimeout(timer); + } +} + +const timeoutResult = (e: ApiTimeoutError): ApiResult => ({ success: false, error: 'timeout', message: e.message }); + /** * Reads a save/publish response without ever throwing on the body: a non-2xx * status or a non-JSON body (a 502 HTML page, a PHP fatal) becomes a @@ -278,7 +310,12 @@ async function readApiResult(resp: Response, action: 'Save' | 'Publish'): Promis const obj = body && typeof body === 'object' ? (body as ApiResult) : null; if (resp.ok && obj) return obj; if (obj) { - return { ...obj, success: false, message: obj.message || `${action} failed: server returned HTTP ${resp.status}` }; + // PHP handlers often put their readable text in `error` only ("Another + // save of this site is still in progress..."): prefer it over the status. + const text = (typeof obj.message === 'string' && obj.message.trim()) + || (typeof obj.error === 'string' && obj.error.trim()) + || `${action} failed: server returned HTTP ${resp.status}`; + return { ...obj, success: false, message: text }; } return { success: false, @@ -357,40 +394,49 @@ export function useWhpApi() { extras: projectExtras.all(), }); - const resp = await fetch(`${whpConfig.apiUrl}?action=save`, { - method: 'POST', - headers: { - 'Content-Type': 'application/json', - 'X-CSRF-Token': whpConfig.csrfToken, - }, - body: JSON.stringify({ - ...payload, - // Present on every save so the server can detect a newer copy; left - // out only when load never reported a rev (an older backend). - ...(baseRev !== undefined ? { base_rev: baseRev } : {}), - ...(forceEmpty ? { force_empty: true } : {}), - ...(forceOverwrite ? { force_overwrite: true } : {}), - }), - }); + let result: ApiResult; + try { + result = await withTimeout('Save', async (signal) => { + const resp = await fetch(`${whpConfig.apiUrl}?action=save`, { + signal, + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'X-CSRF-Token': whpConfig.csrfToken, + }, + body: JSON.stringify({ + ...payload, + // Present on every save so the server can detect a newer copy; left + // out only when load never reported a rev (an older backend). + ...(baseRev !== undefined ? { base_rev: baseRev } : {}), + ...(forceEmpty ? { force_empty: true } : {}), + ...(forceOverwrite ? { force_overwrite: true } : {}), + }), + }); - // 409 empty_overwrite_refused is an expected outcome the UI handles, so - // return it as a structured result instead of throwing. - if (resp.status === 409) { - const body = await resp.json().catch(() => null); - if (body?.error === 'empty_overwrite_refused') { - return { - success: false, - error: 'empty_overwrite_refused', - message: body.message || 'This would replace your saved site with an empty page.', - }; - } - // Opens the conflict modal (here, so the save inside publish raises it too). - const message = body?.message || 'This site was changed in another tab or device since you opened it.'; - setConflict(message); - return { ...(body ?? {}), success: false, error: 'conflict', message }; + // 409 empty_overwrite_refused is an expected outcome the UI handles, so + // return it as a structured result instead of throwing. + if (resp.status === 409) { + const body = await resp.json().catch(() => null); + if (body?.error === 'empty_overwrite_refused') { + return { + success: false, + error: 'empty_overwrite_refused', + message: body.message || 'This would replace your saved site with an empty page.', + }; + } + // Opens the conflict modal (here, so the save inside publish raises it too). + const message = body?.message || 'This site was changed in another tab or device since you opened it.'; + setConflict(message); + return { ...(body ?? {}), success: false, error: 'conflict', message }; + } + + return readApiResult(resp, 'Save'); + }); + } catch (e) { + if (e instanceof ApiTimeoutError) return timeoutResult(e); + throw e; } - - const result = await readApiResult(resp, 'Save'); // The one-shot force_empty is spent only once a save actually went through. if (result.success) { if (forceEmpty) forceEmptyNextSave.current = false; @@ -466,33 +512,44 @@ export function useWhpApi() { } // Then publish from staging to live - const resp = await fetch( - `${whpConfig.apiUrl}?action=publish&site_id=${whpConfig.siteId}`, - { - method: 'POST', - headers: { - 'Content-Type': 'application/json', - 'X-CSRF-Token': whpConfig.csrfToken, - }, - body: JSON.stringify({ site_id: whpConfig.siteId }), - }, - ); - return readApiResult(resp, 'Publish'); + try { + return await withTimeout('Publish', async (signal) => { + const resp = await fetch( + `${whpConfig.apiUrl}?action=publish&site_id=${whpConfig.siteId}`, + { + signal, + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'X-CSRF-Token': whpConfig.csrfToken, + }, + body: JSON.stringify({ site_id: whpConfig.siteId }), + }, + ); + return readApiResult(resp, 'Publish'); + }); + } catch (e) { + if (e instanceof ApiTimeoutError) return timeoutResult(e); + throw e; + } }, [isWHP, whpConfig, loadState, save]); const load = useCallback(async () => { if (!isWHP || !whpConfig) return null; - const resp = await fetch( - `${whpConfig.apiUrl}?action=load&site_id=${whpConfig.siteId}`, - ); - if (!resp.ok) throw new Error(`Load failed: HTTP ${resp.status}`); - let data; - try { - data = await resp.json(); - } catch { - throw new Error('Load failed: response was not valid JSON'); - } + // A timeout rejects with ApiTimeoutError (an Error with a readable message). + const data = await withTimeout('Load', async (signal) => { + const resp = await fetch( + `${whpConfig.apiUrl}?action=load&site_id=${whpConfig.siteId}`, + { signal }, + ); + if (!resp.ok) throw new Error(`Load failed: HTTP ${resp.status}`); + try { + return await resp.json(); + } catch { + throw new Error('Load failed: response was not valid JSON'); + } + }); if (!data || data.success !== true) { throw new Error(`Load failed: ${data?.message || data?.error || 'server reported failure'}`); }