diff --git a/src/apps/officer-web/Screens/Dashboard/Settings/ServerSettings/MaintenanceSection.tsx b/src/apps/officer-web/Screens/Dashboard/Settings/ServerSettings/MaintenanceSection.tsx index 5ac04163..471b5fb9 100644 --- a/src/apps/officer-web/Screens/Dashboard/Settings/ServerSettings/MaintenanceSection.tsx +++ b/src/apps/officer-web/Screens/Dashboard/Settings/ServerSettings/MaintenanceSection.tsx @@ -11,6 +11,24 @@ import { useClient } from 'hooks/useClient'; * other costs you every open connection. Presenting them identically would invite the reflex of pressing * the second when the first would have done. */ +/** + * The message a route meant to send, out of what the client threw. + * + * `useClient` throws with the raw response BODY as its message, and these routes answer with JSON. So the + * useful sentence is one `JSON.parse` inside a string nobody should be asked to read. + */ +export function errorMessage(err: unknown, fallback: string): string { + const raw = err instanceof Error ? err.message : ''; + if (!raw) return fallback; + try { + const parsed = JSON.parse(raw) as { error?: unknown }; + if (typeof parsed.error === 'string' && parsed.error) return parsed.error; + } catch { + // Not JSON — a proxy error page, or a plain string. Whatever it is, it is what the server said. + } + return raw; +} + export const MaintenanceSection = () => { const client = useClient(); const [rebuilding, setRebuilding] = useState(false); @@ -30,7 +48,10 @@ export const MaintenanceSection = () => { // the difference between "it worked" and "why does my change not show". toast.success('Frontend rebuilt — refresh this page to see it'); } catch (err) { - toast.error(err instanceof Error ? err.message : 'Rebuild failed'); + // The client throws on any status >= 400 with the raw response body as its message, so a build + // failure arrives here as the JSON the route sent. Unwrapped, or the toast shows a reader + // `{"ok":false,"error":"…"}` and makes them dig the real message out of punctuation. + toast.error(errorMessage(err, 'Rebuild failed')); } finally { setRebuilding(false); } @@ -41,12 +62,29 @@ export const MaintenanceSection = () => { try { await client.post('/server-settings/restart', {}); toast.success('Restarting — the page will reconnect on its own'); - } catch { - // The restart may well have happened: the process answering this request is the one being replaced, - // so a dropped connection here is the expected shape of success. + } catch (err) { + /** + * Two failures arrive here and they are opposites. + * + * A REFUSAL — the route answering 400 because PM2 does not manage this process — is a real error + * with a status, and nothing is restarting. A DROPPED CONNECTION has no status, and is the expected + * shape of success: the process answering the request is the one being replaced, so it often dies + * before the response lands. + * + * The first version treated both as success. It would have told you "restarting" while refusing to + * restart, and left the button disabled forever — which is precisely the case this screen exists to + * make legible. + */ + const status = (err as { status?: number }).status; + if (typeof status === 'number') { + toast.error(errorMessage(err, 'Restart refused')); + setRestarting(false); + return; + } toast.success('Restarting — the page will reconnect on its own'); } - // Deliberately left true. The button must not become pressable again while the server is coming back. + // Left disabled on the success path, deliberately: the button must not become pressable again while + // the server is coming back. }; return ( diff --git a/src/apps/officer-web/Screens/Dashboard/Settings/ServerSettings/maintenance-error.test.ts b/src/apps/officer-web/Screens/Dashboard/Settings/ServerSettings/maintenance-error.test.ts new file mode 100644 index 00000000..e6b470e4 --- /dev/null +++ b/src/apps/officer-web/Screens/Dashboard/Settings/ServerSettings/maintenance-error.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, test } from 'bun:test'; +import { errorMessage } from './MaintenanceSection'; + +// `useClient` throws on any status >= 400 with the raw response BODY as the message, and these routes +// answer with JSON. Without unwrapping, the toast shows `{"ok":false,"error":"…"}` and asks the reader to +// find the sentence inside the punctuation. + +describe('errorMessage', () => { + test('pulls the message out of a JSON error body', () => { + const err = new Error('{"ok":false,"error":"not running under PM2 — restart it the way you started it"}'); + expect(errorMessage(err, 'fallback')).toBe('not running under PM2 — restart it the way you started it'); + }); + + test('passes a plain-text body through — a proxy error page is still what the server said', () => { + expect(errorMessage(new Error('502 Bad Gateway'), 'fallback')).toBe('502 Bad Gateway'); + }); + + test('JSON without an error field falls back to the raw body rather than inventing one', () => { + expect(errorMessage(new Error('{"ok":false}'), 'fallback')).toBe('{"ok":false}'); + }); + + test('an empty message uses the fallback', () => { + expect(errorMessage(new Error(''), 'Rebuild failed')).toBe('Rebuild failed'); + }); + + test('something that is not an Error at all uses the fallback', () => { + expect(errorMessage('a bare string', 'Rebuild failed')).toBe('Rebuild failed'); + expect(errorMessage(undefined, 'Rebuild failed')).toBe('Rebuild failed'); + }); +});