a desktop audit was not a desktop audit
`formFactor: 'desktop'` changes which thresholds a metric is scored against and NOTHING else. Throttling and the user agent keep their defaults, which are mobile — so every desktop run was a desktop-sized window on a 4x-slowed CPU over slow 4G, announced as a moto-g. It read 86 where the same page reads 100. That gap was reported repeatedly as network latency to the host. It was not latency; it was this. Measured after the fix: desktop 100, LCP 0.4s against 2.2s. Nine more from the same review: runtimeError was never checked, so a page that failed to load reported "nothing failing" — the most reassuring possible output for a site that is down --json exited 0 before the gate, so the documented non-zero exit was true for a person reading the table and false for the only caller that could act on it scoreDisplayMode 'error' was filtered out while still nulling its category, so a category could read 0 with an empty findings list a missing metric serialised as 0, which a script reads as a perfect measurement `source` is a string on some audits and an object on others; String() on the object printed [object Object] the watchdog's surrender was permanent — the health branch sat below the check, so once it stopped trying it also stopped noticing, and a server fixed by hand left it asleep until the platform restarted a failed restart leaves no process at all, because startServer deletes before it starts. status null then reads as "somebody stopped it", which the watchdog leaves alone — one bad restart would have turned it off and taken the server with it, silently the skill claimed the panel uses this engine. Nothing but the CLI does and gave `bun run build:site` with no cwd, which only works from the website checkout — api/static-site.ts documents that and the skill did not Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5
+33
-10
@@ -78,28 +78,34 @@ let timer: ReturnType<typeof setInterval> | null = null;
|
||||
async function check(): Promise<void> {
|
||||
state.lastCheckedAt = new Date().toISOString();
|
||||
|
||||
if (state.surrendered) return;
|
||||
|
||||
// Inside the cooldown: it was just restarted and is probably still starting.
|
||||
if (state.lastRestartAt && Date.now() - Date.parse(state.lastRestartAt) < COOLDOWN_MS) return;
|
||||
|
||||
const servers = await readServers().catch(() => null);
|
||||
const dev = servers?.find((one: ServerStatus) => one.name === 'development');
|
||||
if (!dev || dev.port === null) return;
|
||||
|
||||
/*
|
||||
Only the lie. `online` and answering is fine; anything else is somebody's decision.
|
||||
── Answering again clears everything, INCLUDING having given up ──
|
||||
|
||||
`reachable` is null when there is no port to try, which is not a failure — it is a server that has
|
||||
never been configured, and starting that would be inventing an intention nobody had.
|
||||
This branch used to sit below the `surrendered` check, which made surrender permanent: once it
|
||||
stopped trying it also stopped noticing, so a server somebody fixed by hand left a watchdog that
|
||||
would never watch again until the whole platform restarted. Reading the state first and clearing
|
||||
it here is what makes giving up a pause rather than a death.
|
||||
|
||||
`online` and answering is fine; anything else is somebody's decision — see the note at the top.
|
||||
`reachable` is null when there is no port to try, which is a server nobody has configured.
|
||||
*/
|
||||
if (dev.status !== 'online' || dev.reachable !== false) {
|
||||
if (state.strikes) console.log('[fofos-admin] dev server answering again');
|
||||
if (state.strikes || state.surrendered) console.log('[fofos-admin] dev server answering again');
|
||||
state.strikes = 0;
|
||||
state.consecutiveRestarts = 0;
|
||||
state.surrendered = false;
|
||||
return;
|
||||
}
|
||||
|
||||
if (state.surrendered) return;
|
||||
|
||||
// Inside the cooldown: it was just restarted and is probably still starting.
|
||||
if (state.lastRestartAt && Date.now() - Date.parse(state.lastRestartAt) < COOLDOWN_MS) return;
|
||||
|
||||
state.strikes += 1;
|
||||
console.log(`[fofos-admin] dev server is online but not answering (${state.strikes}/${STRIKES})`);
|
||||
if (state.strikes < STRIKES) return;
|
||||
@@ -122,7 +128,24 @@ async function check(): Promise<void> {
|
||||
output: error instanceof Error ? error.message : String(error),
|
||||
}));
|
||||
|
||||
if (!result.ok) console.warn('[fofos-admin] the restart failed:', result.output.slice(0, 400));
|
||||
/*
|
||||
A failed start is the dangerous outcome, not the loud one.
|
||||
|
||||
`startServer` DELETES the pm2 entry before starting it. If the start then fails there is no
|
||||
process at all — and `status: null` reads, one tick later, as "a server somebody stopped", which
|
||||
this deliberately leaves alone. So a single bad restart would turn the watchdog off and take the
|
||||
server with it, silently.
|
||||
|
||||
Counted as a failure so `GIVE_UP_AFTER` still bounds the retries, and said loudly, because a
|
||||
development server that no longer exists is not something to find out about tomorrow.
|
||||
*/
|
||||
if (!result.ok) {
|
||||
console.error(
|
||||
'[fofos-admin] the dev server FAILED to start and no process is left — `pm2 start` was preceded ' +
|
||||
'by a delete. Start it from the panel, or `pm2 logs`:',
|
||||
result.output.slice(0, 400),
|
||||
);
|
||||
}
|
||||
|
||||
if (state.consecutiveRestarts >= GIVE_UP_AFTER) {
|
||||
state.surrendered = true;
|
||||
|
||||
+72
-5
@@ -33,7 +33,8 @@ const CATEGORIES = ['performance', 'accessibility', 'best-practices', 'seo'] as
|
||||
|
||||
export type FormFactor = 'mobile' | 'desktop';
|
||||
|
||||
export type AuditMetric = { id: string; label: string; value: number; display: string };
|
||||
/** `value` is null when the metric did not run — which is not the same as it being zero. */
|
||||
export type AuditMetric = { id: string; label: string; value: number | null; display: string };
|
||||
|
||||
export type AuditFinding = {
|
||||
id: string;
|
||||
@@ -200,12 +201,48 @@ export async function audit(
|
||||
logLevel: 'error',
|
||||
onlyCategories: [...CATEGORIES],
|
||||
formFactor,
|
||||
// Lighthouse will not apply mobile emulation to a desktop form factor or the reverse, and
|
||||
// getting this wrong is the classic way to produce a number that means nothing.
|
||||
/*
|
||||
── `formFactor` alone does NOT make a desktop audit ──
|
||||
|
||||
It changes which of Lighthouse's own thresholds a metric is scored against and nothing else.
|
||||
The THROTTLING and the USER AGENT keep their defaults, which are mobile: a 4x CPU slowdown and
|
||||
slow 4G, announced as a moto-g. So a `--desktop` run was a desktop-sized window crawling on an
|
||||
emulated phone, and it read 86 where the same page read 100 in a real desktop Lighthouse.
|
||||
|
||||
That number was believed for a while and blamed on network latency. It was not latency; it was
|
||||
this. All four have to agree or the run is about nothing.
|
||||
|
||||
The numbers are Lighthouse's own — `desktopDense4G` from its constants: 40 ms RTT, 10 Mbps, no
|
||||
CPU slowdown. Copied rather than imported because the constants module is not part of its
|
||||
public surface and pinning to it would break on an upgrade that moved the file.
|
||||
*/
|
||||
screenEmulation:
|
||||
formFactor === 'desktop'
|
||||
? { mobile: false, width: 1350, height: 940, deviceScaleFactor: 1, disabled: false }
|
||||
: { mobile: true, width: 412, height: 823, deviceScaleFactor: 1.75, disabled: false },
|
||||
throttling:
|
||||
formFactor === 'desktop'
|
||||
? {
|
||||
rttMs: 40,
|
||||
throughputKbps: 10 * 1024,
|
||||
cpuSlowdownMultiplier: 1,
|
||||
requestLatencyMs: 0,
|
||||
downloadThroughputKbps: 0,
|
||||
uploadThroughputKbps: 0,
|
||||
}
|
||||
: {
|
||||
rttMs: 150,
|
||||
throughputKbps: 1.6 * 1024,
|
||||
cpuSlowdownMultiplier: 4,
|
||||
requestLatencyMs: 150 * 3.75,
|
||||
downloadThroughputKbps: 1.6 * 1024 * 0.9,
|
||||
uploadThroughputKbps: 750 * 0.9,
|
||||
},
|
||||
emulatedUserAgent:
|
||||
formFactor === 'desktop'
|
||||
? 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) ' +
|
||||
'Chrome/131.0.0.0 Safari/537.36'
|
||||
: undefined,
|
||||
throttlingMethod: 'simulate',
|
||||
// Never. It ships to Sentry, and a build tool that phones home about this site is not something
|
||||
// anybody asked for. Off by default in lighthouse too; said here so it stays off.
|
||||
@@ -213,6 +250,17 @@ export async function audit(
|
||||
} as any);
|
||||
|
||||
if (!run?.lhr) throw new Error('lighthouse returned nothing');
|
||||
|
||||
/*
|
||||
A run that could not load the page still returns an `lhr`.
|
||||
|
||||
Every category comes back null, every audit is filtered out, and the report reads "nothing
|
||||
failing" — the most reassuring possible output for a site that is down. `runtimeError` is the one
|
||||
field that says so.
|
||||
*/
|
||||
if (run.lhr.runtimeError?.code && run.lhr.runtimeError.code !== 'NO_ERROR') {
|
||||
throw new Error(`${run.lhr.runtimeError.code}: ${run.lhr.runtimeError.message ?? 'the page did not load'}`);
|
||||
}
|
||||
const lhr = run.lhr;
|
||||
|
||||
const scores: Record<string, number | null> = {};
|
||||
@@ -226,7 +274,9 @@ export async function audit(
|
||||
return {
|
||||
id,
|
||||
label: found?.title ?? id,
|
||||
value: typeof found?.numericValue === 'number' ? found.numericValue : 0,
|
||||
// NULL, not 0. A metric that did not run is not a metric that scored perfectly, and a `--json`
|
||||
// consumer reading 0 for Total Blocking Time would believe the best possible news.
|
||||
value: typeof found?.numericValue === 'number' ? found.numericValue : null,
|
||||
display: found?.displayValue ?? '—',
|
||||
};
|
||||
});
|
||||
@@ -240,6 +290,12 @@ export async function audit(
|
||||
*/
|
||||
const findings: AuditFinding[] = Object.values(lhr.audits)
|
||||
.filter((one) => {
|
||||
/*
|
||||
`error` is a failure and is listed. It nulls the whole category score, so filtering it out
|
||||
alongside the diagnostics meant a category could read 0 with an empty findings list — the
|
||||
one case where "nothing failing" is exactly wrong.
|
||||
*/
|
||||
if (one.scoreDisplayMode === 'error') return true;
|
||||
if (one.scoreDisplayMode === 'informative' || one.scoreDisplayMode === 'manual') return false;
|
||||
if (one.scoreDisplayMode === 'notApplicable') return false;
|
||||
return typeof one.score === 'number' && one.score < 0.9;
|
||||
@@ -261,8 +317,19 @@ export async function audit(
|
||||
if (typeof item.wastedMs === 'number') parts.push(`${Math.round(item.wastedMs)} ms`);
|
||||
const node = item.node as { nodeLabel?: string } | undefined;
|
||||
if (node?.nodeLabel) parts.push(node.nodeLabel);
|
||||
/*
|
||||
`source` is a string on some audits and a `source-location` object on others —
|
||||
`deprecations` and the insight audits are the ones that bite. `String()` on the object
|
||||
prints `[object Object]`, which is worse than printing nothing.
|
||||
*/
|
||||
const source = item.source as { url?: unknown } | string | undefined;
|
||||
const from =
|
||||
typeof source === 'string' ? source
|
||||
: source && typeof source.url === 'string' ? source.url
|
||||
: '';
|
||||
|
||||
return {
|
||||
url: typeof item.url === 'string' ? item.url : String(item.source ?? ''),
|
||||
url: typeof item.url === 'string' ? item.url : from,
|
||||
detail: parts.join(', '),
|
||||
};
|
||||
});
|
||||
|
||||
+21
-4
@@ -32,9 +32,20 @@ const factors: FormFactor[] = args.includes('--mobile')
|
||||
? ['desktop']
|
||||
: ['mobile', 'desktop'];
|
||||
|
||||
/** Lighthouse's own bands, so a number reads as good or not without looking them up. */
|
||||
/**
|
||||
* Lighthouse's own bands, so a number reads as good or not without looking them up.
|
||||
*
|
||||
* `✓` is 90 and up, which is what Lighthouse calls green — NOT the same as the exit gate below, which
|
||||
* is 100. The two disagreeing is deliberate and worth saying out loud: green is Lighthouse's word for
|
||||
* good, and this tool's exit code answers a different question, which is whether there is anything
|
||||
* left to do.
|
||||
*/
|
||||
const band = (score: number | null) => (score === null ? ' ? ' : score >= 90 ? ' ✓ ' : score >= 50 ? ' ~ ' : ' ✗ ');
|
||||
|
||||
/** Every category on every form factor at 100 — see the note on `band`. */
|
||||
const perfect = (results: { scores: Record<string, number | null> }[]) =>
|
||||
results.every((one) => Object.values(one.scores).every((score) => score === 100));
|
||||
|
||||
const results = [];
|
||||
for (const formFactor of factors) {
|
||||
// To stderr, so `--json` on stdout stays a clean pipe while the wait is still narrated.
|
||||
@@ -43,7 +54,14 @@ for (const formFactor of factors) {
|
||||
|
||||
if (json) {
|
||||
console.log(JSON.stringify(results, null, 2));
|
||||
process.exit(0);
|
||||
/*
|
||||
The same gate as below, not `exit(0)`.
|
||||
|
||||
`--json` is the mode a script uses, and a script is exactly the caller that reads an exit code.
|
||||
Exiting 0 here meant the documented "non-zero when anything is short of 100" was true for a person
|
||||
watching the table and false for the only consumer that could act on it.
|
||||
*/
|
||||
process.exit(perfect(results) ? 0 : 1);
|
||||
}
|
||||
|
||||
for (const result of results) {
|
||||
@@ -74,5 +92,4 @@ for (const result of results) {
|
||||
}
|
||||
|
||||
// A non-zero exit when anything is short of 100, so this can gate a build without being parsed.
|
||||
const perfect = results.every((one) => Object.values(one.scores).every((score) => score === 100));
|
||||
process.exit(perfect ? 0 : 1);
|
||||
process.exit(perfect(results) ? 0 : 1);
|
||||
|
||||
@@ -6,8 +6,8 @@ version: 1
|
||||
---
|
||||
|
||||
`bin/lighthouse.ts` runs Lighthouse against a URL in a real Chrome and prints what failed, with the
|
||||
resources each finding names. It is the same engine the panel uses (`api/lighthouse.ts`), so a number
|
||||
you get here is the number the panel would show.
|
||||
resources each finding names. The engine is `api/lighthouse.ts`, which is where the panel will read
|
||||
from when it grows a screen for this — today the CLI is the only caller.
|
||||
|
||||
## Running it
|
||||
|
||||
@@ -34,8 +34,14 @@ static files under `dist/`, served by nginx behind a proxy, with different cachi
|
||||
compression and a trailing-slash redirect. **They are not the same page** and their numbers do not
|
||||
agree.
|
||||
|
||||
So: `bun run build:site` first, then audit `https://web.fofosfluffybastards.com/...`. A score from the
|
||||
dev server is not wrong, it is about something nobody visits.
|
||||
So: build first, then audit the public URL. The build has to run in the website checkout and with the
|
||||
site's own environment — see `api/static-site.ts`, which documents both:
|
||||
|
||||
```bash
|
||||
cd ~/officerdev/data/*/FofosWebsite/website && bun run build:site
|
||||
```
|
||||
|
||||
A score from the dev server is not wrong, it is about something nobody visits.
|
||||
|
||||
## Reading the output
|
||||
|
||||
@@ -78,10 +84,17 @@ Performance is computed from these and nothing else. Insights are diagnostics; t
|
||||
|
||||
## Traps
|
||||
|
||||
**A desktop number from another machine includes that machine's latency.** Auditing from officer-dev
|
||||
against fofos crosses a network the owner's browser does not. Compare like with like: two runs from
|
||||
the same place, before and after a change. An absolute score from somewhere else is a different
|
||||
question.
|
||||
**A desktop audit is desktop in four ways, and `formFactor` is only one of them.** Setting
|
||||
`formFactor: 'desktop'` changes which thresholds a metric is scored against and NOTHING else — the
|
||||
throttling and the user agent keep their defaults, which are mobile. A run configured that way is a
|
||||
desktop-sized window crawling on an emulated phone, and it reads about 86 where the same page reads
|
||||
100. `api/lighthouse.ts` sets `screenEmulation`, `throttling` and `emulatedUserAgent` together for
|
||||
this reason; if you write your own runner, set all four or the number is about nothing.
|
||||
|
||||
**A number from another machine includes that machine's network.** Compare like with like: two runs
|
||||
from the same place, before and after a change. An absolute score from somewhere else is a different
|
||||
question — though check the paragraph above first, because that explains far more of a gap than
|
||||
latency usually does.
|
||||
|
||||
**Run-to-run variance is real** — a point or two either way on the same build. Chase a finding, not a
|
||||
number. If a score moves and no finding changed, nothing happened.
|
||||
|
||||
Reference in New Issue
Block a user