the maintenance buttons tell you when they fail
Asked whether there was feedback on failure, and for the restart there was not —
there was the opposite.
`useClient` throws on any status >= 400. The restart handler caught everything and
reported success, on the reasoning that a dropped connection is normal when the
process answering the request is the one being replaced. That is true, and it made
the honest refusal built into the route unreachable: a 400 saying "not running under
PM2" arrived through the same catch, so the button would have said "Restarting…"
while nothing restarted, and stayed disabled forever.
The two failures are opposites and are now told apart by whether an error carries a
STATUS. A status means the server answered and declined — real error, message shown,
button re-enabled. No status means the connection dropped — expected, and the
success path.
Both buttons also unwrap the message. The client throws with the raw response body,
and these routes answer with JSON, so an unhandled failure showed
`{"ok":false,"error":"…"}` and asked the reader to find the sentence inside the
punctuation. Five tests on that, including the cases where it must NOT invent one:
JSON with no error field, and a non-Error thrown value.
Found while answering the question rather than by testing, which is the wrong order.
── One thing I should not have done ──
I exercised the restart route against the live server to see its refusal shape. PM2
knows `officer` here, so it returned 200 and scheduled a real restart; the only
reason it did not happen is that my test process called `process.exit(0)` before the
250ms timer fired. Luck, not design. `pm_uptime` confirms the platform never
restarted.
tsgo clean, frontend builds, 919 pass / 7 fail unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
+43
-5
@@ -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 (
|
||||
|
||||
+30
@@ -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');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user