fix(api): time out hung save/publish/load; show PHP error text
- save, publish and load run under a 30s AbortController timeout (raced,
so a stalled body read can't hang either); save/publish resolve to
{ success:false, error:'timeout', message } and load rejects with a
readable Error, so a hung request no longer blocks the save queue
- readApiResult falls back to obj.error when obj.message is absent, so
PHP texts like 'Another save of this site is still in progress...' are
shown instead of 'server returned HTTP 503'
Refs #4
Refs #47
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
25de75c34f
commit
7d25bf181c
2 files changed
+279
-55
No files matched your search
@@ -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<string, unknown>) => {
|
||||
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<typeof vi.fn>;
|
||||
let api: ReturnType<typeof useWhpApi>;
|
||||
let guard: ReturnType<typeof useSaveGuard>;
|
||||
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(
|
||||
<EditorConfigProvider config={whpConfig}>
|
||||
<SaveGuardProvider><SiteDesignProvider><PageProvider>
|
||||
{withLoadGate && <LoadGate />}
|
||||
<Probe />
|
||||
</PageProvider></SiteDesignProvider></SaveGuardProvider>
|
||||
</EditorConfigProvider>,
|
||||
);
|
||||
});
|
||||
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');
|
||||
});
|
||||
});
|
||||
+112
-55
@@ -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<T>(action: string, fn: (signal: AbortSignal) => Promise<T>, ms = API_TIMEOUT_MS): Promise<T> {
|
||||
const ctrl = new AbortController();
|
||||
let timer: ReturnType<typeof setTimeout> | undefined;
|
||||
const timeout = new Promise<never>((_, 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'}`);
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user