From 5c3db0cb9ee90a7f94461c01eace26dbc44b735b Mon Sep 17 00:00:00 2001 From: Eric J Date: Mon, 3 Aug 2026 13:49:12 -0700 Subject: [PATCH 1/2] fix(preview): stop the container from being billed 24/7 The preview container was provisioned around the clock on ~7 requests/day. Measured over the Jul 12 - Aug 3 billing period: ~1 instance awake every hour without exception, 1,540 GiB-hours of memory, 2.2% CPU utilization, and 14 worker requests across the last 45 hours of it. At the published $0.0000025/GiB-s that is ~$0.86/day, or ~$28/month, against a $5 plan. Three causes, all fixed here. 1. It never slept. @cloudflare/containers proxies the container through a TransformStream and decrements its inflight-request counter only when that body finishes piping; isActivityExpired() returns false while the counter is nonzero, so `sleepAfter = "20s"` never fires. The 502 path dropped the container's error body without reading it, leaving the counter pinned above zero permanently. One failed render was enough to keep the instance awake forever. Drain it. 2. instance_type was 4x oversized. Memory bills on provisioned size for the whole awake window, so this is the dominant lever. containersMetricsAdaptiveGroups over 20 days: peak 603 MiB, idle ~205 MiB, peak disk 522 MiB. `basic` (1 GiB / 4 GB) clears the peak with ~40% headroom; `lite` (256 MiB) would not. 3. max_instances was 3. The worker only ever addresses pool-0, so the other two were unreachable - but they capped nothing if an instance ever leaked. The new test asserts the error path consumes the container body. It fails on the pre-fix worker (bodyUsed false), so it is not vacuous. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_018AB7J1qK6kSBnJJmgDqMst --- preview-service/worker/src/index.ts | 9 ++++++++ preview-service/worker/test/worker.spec.ts | 24 ++++++++++++++++++++++ preview-service/worker/wrangler.jsonc | 17 +++++++++++++-- 3 files changed, 48 insertions(+), 2 deletions(-) diff --git a/preview-service/worker/src/index.ts b/preview-service/worker/src/index.ts index f5a0740f..baf5c931 100644 --- a/preview-service/worker/src/index.ts +++ b/preview-service/worker/src/index.ts @@ -76,6 +76,15 @@ export default { }), ); if (!renderRes.ok) { + // Drain the body before dropping this response. @cloudflare/containers + // proxies the container through a TransformStream and decrements its + // inflight-request counter only when that stream finishes piping. A + // container that still looks busy never reaches `sleepAfter` (see + // isActivityExpired in the package), so it stays provisioned - and billed + // for its full instance_type memory - around the clock. One un-drained + // error body pins the instance awake indefinitely. + const detail = await renderRes.text().catch(() => ""); + console.error(`container render failed (${renderRes.status}): ${detail.slice(0, 200)}`); return new Response("render failed", { status: 502, headers: corsHeaders(env) }); } const png = await renderRes.arrayBuffer(); diff --git a/preview-service/worker/test/worker.spec.ts b/preview-service/worker/test/worker.spec.ts index e3267df5..b7738a43 100644 --- a/preview-service/worker/test/worker.spec.ts +++ b/preview-service/worker/test/worker.spec.ts @@ -40,6 +40,30 @@ describe("worker /preview", () => { expect(buf[0]).toBe(0x89); }); + it("drains the container response body when a render fails", async () => { + // Regression guard for a cost bug, not a correctness one: the response is + // discarded either way. @cloudflare/containers only decrements its + // inflight-request counter when the proxied body finishes streaming, and a + // container with a nonzero counter never reaches `sleepAfter`. Dropping one + // error body left a 4 GiB instance provisioned 24/7 on ~7 requests/day, + // which is where the bill went. Assert the body is consumed. + const failing = new Response("boom", { status: 500 }); + const fakeEnv = { + ...env, + PREVIEW_CONTAINER: { + idFromName: () => "pool-0", + get: () => ({ fetch: async () => failing }), + }, + } as unknown as typeof env; + + const ctx = createExecutionContext(); + const res = await worker.fetch(post({ ...body, seed: 987654 }), fakeEnv, ctx); + await waitOnExecutionContext(ctx); + + expect(res.status).toBe(502); + expect(failing.bodyUsed).toBe(true); + }); + it("rejects disallowed origins", async () => { const ctx = createExecutionContext(); const res = await worker.fetch(post(body, "https://evil.example"), env, ctx); diff --git a/preview-service/worker/wrangler.jsonc b/preview-service/worker/wrangler.jsonc index 12659be8..6b5993f4 100644 --- a/preview-service/worker/wrangler.jsonc +++ b/preview-service/worker/wrangler.jsonc @@ -12,8 +12,21 @@ { "class_name": "PreviewContainer", "image": "../container/Dockerfile", - "instance_type": "standard-1", - "max_instances": 3, + // Memory is billed on PROVISIONED size for the whole time an instance is + // awake, not on what the process touches - so instance_type is the single + // biggest cost lever here, and it was 4x oversized. Measured over 20 days + // of production (containersMetricsAdaptiveGroups): peak 603 MiB, idle + // ~205 MiB, peak disk 522 MiB. `basic` is 1 GiB / 4 GB, ~40% memory + // headroom over the observed peak. `size` is pinned to 1024 in + // src/schema.ts, so no request can enlarge a render past that peak. + // Re-measure before shrinking further; the next size down is `lite` + // (256 MiB), which the 603 MiB peak rules out. + "instance_type": "basic", + // The worker only ever addresses idFromName("pool-0"), so a second + // instance is unreachable by design - but max_instances is what caps the + // blast radius if that ever changes or an instance leaks. At 3 this bill + // could silently triple. + "max_instances": 1, }, ], "durable_objects": { From d8564a698f36ce6f196c6bed72b4f865293eb048 Mon Sep 17 00:00:00 2001 From: Eric J Date: Mon, 3 Aug 2026 13:58:58 -0700 Subject: [PATCH 2/2] feat(scripts): loud preflight for the scripts that need Docker A stopped daemon surfaced as a wrangler build error several screens deep that never says "start Docker". This turns it into a red banner naming the exact command for whichever runtime you actually have installed. Wired into `preview:dev` and `preview:deploy` only - the two that build the Factorio image. Deliberately NOT into `preview:test` or `verify`: CLAUDE.md documents that those run on CI runners with no container runtime at all, and a daemon dependency there would break that property. It reports rather than acts. The runtime is the developer's choice, not the repo's - OrbStack, Docker Desktop, colima and podman all satisfy the Dockerfile, and hardcoding `orb start` would strand everyone else. Auto-start is available behind an explicit FMW_AUTO_START_DOCKER=1 opt-in, which polls for the daemon rather than trusting the start command's exit code. Exit 0 daemon ready / 1 CLI present but no daemon / 2 no CLI at all, and the no-CLI banner says INSTALL rather than START. Colour is decoration, never the message: NO_COLOR and non-TTY drop the escapes, and the glyph and wording still carry it. All three paths were run by hand (DOCKER_HOST to a dead socket, and PATH stripped of docker). Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_018AB7J1qK6kSBnJJmgDqMst --- package.json | 5 +- scripts/require-docker.ts | 260 +++++++++++++++++++++++++++++++++++++ test/requireDocker.spec.ts | 150 +++++++++++++++++++++ 3 files changed, 413 insertions(+), 2 deletions(-) create mode 100644 scripts/require-docker.ts create mode 100644 test/requireDocker.spec.ts diff --git a/package.json b/package.json index 4d2da58f..e6c2095c 100644 --- a/package.json +++ b/package.json @@ -18,11 +18,12 @@ "verify:shard": "vp test", "verify": "pnpm run verify:lint && vp run --cache test && pnpm run preview:test", "localpreview": "pnpm run preview:dev", - "preview:dev": "concurrently -k -n worker,app -c blue,green \"pnpm run preview:worker\" \"pnpm run preview:app\"", + "require:docker": "node scripts/require-docker.ts", + "preview:dev": "pnpm run require:docker && concurrently -k -n worker,app -c blue,green \"pnpm run preview:worker\" \"pnpm run preview:app\"", "preview:worker": "pnpm --filter @fmw/preview-worker exec wrangler dev --var ALLOWED_ORIGIN:http://localhost:5173", "preview:app": "VITE_PREVIEW_SERVICE_URL=http://localhost:8787 vp dev --port 5173 --strictPort", "preview:test": "pnpm --filter @fmw/preview-worker test && pnpm --filter @fmw/preview-container test", - "preview:deploy": "pnpm --filter @fmw/preview-worker run deploy", + "preview:deploy": "pnpm run require:docker && pnpm --filter @fmw/preview-worker run deploy", "deploy:app": "pnpm run verify && pnpm build && pnpm --filter @fmw/preview-worker exec wrangler pages deploy dist --cwd ../.. --project-name factoriomapwebui --branch main --commit-dirty=true", "deploy": "pnpm run deploy:app" }, diff --git a/scripts/require-docker.ts b/scripts/require-docker.ts new file mode 100644 index 00000000..5aec8ac0 --- /dev/null +++ b/scripts/require-docker.ts @@ -0,0 +1,260 @@ +/** + * Preflight for the two scripts that genuinely need a container runtime: + * `preview:dev` (wrangler dev builds the Factorio image) and `preview:deploy` + * (wrangler deploy builds and pushes it). Without this, a stopped daemon + * surfaces as a wrangler build error several screens deep that does not say + * "start Docker". + * + * Exit 0 = a daemon answered; 1 = the CLI exists but no daemon answered + * (actionable - start it); 2 = no docker CLI at all (install something). + * + * Deliberately NOT wired into `preview:test` or `verify`. Those run on CI + * runners with no container runtime at all and must keep working there - see + * CLAUDE.md. Adding a daemon dependency to them would break that property. + * + * It reports rather than acts, because the runtime is the developer's choice, + * not the repo's: OrbStack, Docker Desktop, colima and podman all satisfy the + * Dockerfile and only the human knows which they installed. Set + * `FMW_AUTO_START_DOCKER=1` to opt into having it run the start command for + * you. + * + * Node runs this `.ts` directly (type stripping); there is deliberately no + * compile step and no `tsc` involved - see CLAUDE.md. + */ + +import { spawnSync } from "node:child_process"; +import { existsSync } from "node:fs"; +import { join, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; + +/** How long to wait on a probe before calling the daemon unreachable. */ +const PROBE_TIMEOUT_MS = 10_000; +/** How long to wait for a daemon to answer after an opt-in auto-start. */ +const START_TIMEOUT_MS = 90_000; +/** Inner width of the banner box, in characters. */ +const BOX_WIDTH = 70; + +export type ProbeResult = + | { status: "ok"; version: string } + | { status: "no-daemon"; detail: string } + | { status: "no-cli"; detail: string }; + +export interface Runtime { + /** Display name, e.g. "OrbStack". */ + name: string; + /** Shell command that starts it. */ + start: string; + /** True when this one looks installed on the current machine. */ + installed: boolean; +} + +// ============================================================================ +// Colour +// ============================================================================ + +const CODES = { + reset: "\x1b[0m", + bold: "\x1b[1m", + dim: "\x1b[2m", + red: "\x1b[31m", + green: "\x1b[32m", + yellow: "\x1b[33m", + cyan: "\x1b[36m", +} as const; + +export type Style = keyof Omit; + +/** + * Wrap text in ANSI styles when colour is on. + * + * Colour is a decoration, never the message: every banner below also carries a + * glyph and a word, so piping to a file or running under NO_COLOR loses nothing + * but the paint. + */ +export function paint(text: string, styles: Style[], enabled: boolean): string { + if (!enabled || styles.length === 0) return text; + return styles.map((s) => CODES[s]).join("") + text + CODES.reset; +} + +/** Honour the NO_COLOR convention, and never emit escapes into a pipe. */ +export function colorEnabled(env: NodeJS.ProcessEnv, isTty: boolean): boolean { + if (env.NO_COLOR !== undefined && env.NO_COLOR !== "") return false; + if (env.FORCE_COLOR !== undefined && env.FORCE_COLOR !== "" && env.FORCE_COLOR !== "0") + return true; + return isTty; +} + +// ============================================================================ +// Banner +// ============================================================================ + +/** + * Render a heavy box around `title`. + * + * Padding is computed from the PLAIN text and the styling applied afterwards, + * so the right-hand border stays flush - measuring a string that already has + * escape codes in it is how boxes come out ragged. + */ +export function buildBox(title: string, styles: Style[], enabled: boolean): string[] { + const bar = "═".repeat(BOX_WIDTH); + const padded = ` ${title} `.padEnd(BOX_WIDTH, " "); + return [ + paint(`╔${bar}╗`, styles, enabled), + paint(`║${padded}║`, styles, enabled), + paint(`╚${bar}╝`, styles, enabled), + ]; +} + +/** Clip a one-line detail so it cannot wrap the banner into unreadability. */ +export function truncate(text: string, max: number): string { + const flat = text.replace(/\s+/g, " ").trim(); + return flat.length <= max ? flat : `${flat.slice(0, max - 1)}…`; +} + +/** + * Turn a failed probe into the exact lines a human needs, loudest first. + * + * Split out from `main` so `test/requireDocker.spec.ts` can assert the advice + * without a container runtime anywhere near the test runner. + */ +export function describeFailure( + probe: Exclude, + runtimes: Runtime[], + enabled: boolean, +): { code: number; lines: string[] } { + const installed = runtimes.filter((r) => r.installed); + const noCli = probe.status === "no-cli"; + const title = noCli ? "✖ NO CONTAINER RUNTIME FOUND" : "✖ DOCKER IS NOT RUNNING"; + + const lines = [ + "", + ...buildBox(title, ["bold", "red"], enabled), + "", + noCli + ? ` This build needs a Docker-compatible CLI and none is on your PATH.` + : ` A ${paint("docker", ["cyan"], enabled)} CLI is installed, but no daemon answered.`, + // Truncated: docker's own connect errors run past 150 characters, and a + // wrapped wall of dim text buries the actionable part below it. + ` ${paint(truncate(probe.detail, 110), ["dim"], enabled)}`, + "", + paint(noCli ? ` ▶ INSTALL ONE OF:` : ` ▶ START IT WITH:`, ["bold", "yellow"], enabled), + "", + ]; + + const suggestions = installed.length > 0 ? installed : runtimes; + for (const r of suggestions) { + const mark = r.installed ? paint("●", ["green"], enabled) : paint("○", ["dim"], enabled); + const note = r.installed ? "" : paint(" (not detected)", ["dim"], enabled); + lines.push( + ` ${mark} ${r.name.padEnd(16)} ${paint(r.start, ["bold", "cyan"], enabled)}${note}`, + ); + } + + lines.push( + "", + paint( + noCli + ? ` Then re-run. Any Docker-compatible runtime satisfies the Dockerfile.` + : ` Then re-run. Or set FMW_AUTO_START_DOCKER=1 to have this start it for you.`, + ["dim"], + enabled, + ), + "", + ); + return { code: noCli ? 2 : 1, lines }; +} + +/** The one-line all-clear. Quiet on purpose - noise on success trains people to ignore it. */ +export function describeSuccess(version: string, enabled: boolean): string { + return `${paint("✔", ["bold", "green"], enabled)} Docker daemon ready ${paint(`(${version})`, ["dim"], enabled)}`; +} + +// ============================================================================ +// Probing +// ============================================================================ + +/** Ask the daemon for its version. Anything other than a clean answer is a failure. */ +export function probeDocker(timeoutMs = PROBE_TIMEOUT_MS): ProbeResult { + const res = spawnSync("docker", ["info", "--format", "{{.ServerVersion}}"], { + encoding: "utf8", + timeout: timeoutMs, + }); + if (res.error) { + const code = (res.error as NodeJS.ErrnoException).code; + if (code === "ENOENT") return { status: "no-cli", detail: "`docker` is not on your PATH." }; + return { status: "no-daemon", detail: `docker info failed: ${res.error.message}` }; + } + if (res.status === 0 && res.stdout.trim()) { + return { status: "ok", version: res.stdout.trim() }; + } + const stderr = (res.stderr || "").trim().split("\n")[0] || "docker info returned no version."; + return { status: "no-daemon", detail: stderr }; +} + +/** + * Which runtimes look installed here. Order is the order they get suggested in. + * + * PATH is scanned directly rather than shelling out to `command -v`: spawning + * with `shell: true` AND an args array is deprecated in Node 26 (DEP0190) and + * prints a warning that would land in the middle of the banner. + */ +export function detectRuntimes(): Runtime[] { + const dirs = (process.env.PATH ?? "").split(":").filter(Boolean); + const onPath = (bin: string): boolean => dirs.some((d) => existsSync(join(d, bin))); + return [ + { name: "OrbStack", start: "orb start", installed: onPath("orb") }, + { + name: "Docker Desktop", + start: "open -a Docker", + installed: existsSync("/Applications/Docker.app"), + }, + { name: "colima", start: "colima start", installed: onPath("colima") }, + { name: "podman", start: "podman machine start", installed: onPath("podman") }, + ]; +} + +// ============================================================================ +// Entrypoint +// ============================================================================ + +async function main(): Promise { + const enabled = colorEnabled(process.env, process.stdout.isTTY === true); + + let probe = probeDocker(); + if (probe.status === "ok") { + console.log(describeSuccess(probe.version, enabled)); + return 0; + } + + const runtimes = detectRuntimes(); + const auto = process.env.FMW_AUTO_START_DOCKER; + const target = runtimes.find((r) => r.installed); + + if (auto === "1" && target && probe.status !== "no-cli") { + console.log( + paint(`⏳ Starting ${target.name} (FMW_AUTO_START_DOCKER=1) ...`, ["yellow"], enabled), + ); + spawnSync(target.start, { shell: true, stdio: "inherit", timeout: START_TIMEOUT_MS }); + // The start command returning does not mean the daemon is accepting + // connections yet, so poll rather than trusting its exit code. + const deadline = Date.now() + START_TIMEOUT_MS; + while (Date.now() < deadline) { + probe = probeDocker(5_000); + if (probe.status === "ok") { + console.log(describeSuccess(probe.version, enabled)); + return 0; + } + spawnSync("sleep", ["2"]); + } + } + + const { code, lines } = describeFailure(probe, runtimes, enabled); + for (const line of lines) console.error(line); + return code; +} + +// Only run when executed directly - `test/requireDocker.spec.ts` imports the +// pure helpers from here and must not shell out to docker. +if (process.argv[1] && resolve(process.argv[1]) === fileURLToPath(import.meta.url)) { + process.exitCode = await main(); +} diff --git a/test/requireDocker.spec.ts b/test/requireDocker.spec.ts new file mode 100644 index 00000000..20230e12 --- /dev/null +++ b/test/requireDocker.spec.ts @@ -0,0 +1,150 @@ +import { describe, it, expect } from "vite-plus/test"; + +import { + buildBox, + colorEnabled, + describeFailure, + describeSuccess, + paint, + truncate, + type Runtime, +} from "../scripts/require-docker.ts"; + +/** Strip ANSI so a rendered line can be measured and read as plain text. */ +// eslint-disable-next-line no-control-regex +const plain = (s: string): string => s.replace(/\x1b\[[0-9;]*m/g, ""); + +const RUNTIMES: Runtime[] = [ + { name: "OrbStack", start: "orb start", installed: false }, + { name: "Docker Desktop", start: "open -a Docker", installed: false }, +]; + +describe("colorEnabled", () => { + it("follows the TTY when nothing overrides it", () => { + expect(colorEnabled({}, true)).toBe(true); + expect(colorEnabled({}, false)).toBe(false); + }); + + it("honours NO_COLOR even on a TTY", () => { + expect(colorEnabled({ NO_COLOR: "1" }, true)).toBe(false); + }); + + it("honours FORCE_COLOR off a TTY, but not when set to 0", () => { + expect(colorEnabled({ FORCE_COLOR: "1" }, false)).toBe(true); + expect(colorEnabled({ FORCE_COLOR: "0" }, false)).toBe(false); + }); +}); + +describe("paint", () => { + it("is a no-op when colour is disabled", () => { + expect(paint("hi", ["red", "bold"], false)).toBe("hi"); + }); + + it("wraps and resets when enabled", () => { + const out = paint("hi", ["red"], true); + expect(out.startsWith("\x1b[31m")).toBe(true); + expect(out.endsWith("\x1b[0m")).toBe(true); + expect(plain(out)).toBe("hi"); + }); +}); + +describe("buildBox", () => { + // The border is measured against the PLAIN text; if padding is ever computed + // on an already-styled string the right edge goes ragged, and only a coloured + // run would show it. Assert both. + it("keeps the right border flush, coloured or not", () => { + for (const enabled of [false, true]) { + const lines = buildBox("✖ DOCKER IS NOT RUNNING", ["bold", "red"], enabled); + // Measured in UTF-16 units, the same unit `padEnd` counts in - so this + // asserts exactly what the renderer did. Every glyph in the box is BMP. + const widths = lines.map((l) => plain(l).length); + expect(new Set(widths).size).toBe(1); + expect(lines).toHaveLength(3); + expect(plain(lines[1])).toContain("DOCKER IS NOT RUNNING"); + } + }); +}); + +describe("truncate", () => { + it("leaves a short string alone but collapses its whitespace", () => { + expect(truncate("a b\n c", 40)).toBe("a b c"); + }); + + it("clips to the limit, ellipsis included", () => { + const out = truncate("x".repeat(50), 10); + expect(out).toHaveLength(10); + expect(out.endsWith("…")).toBe(true); + }); +}); + +describe("describeFailure", () => { + it("exits 1 for a stopped daemon and 2 when there is no CLI", () => { + expect(describeFailure({ status: "no-daemon", detail: "x" }, RUNTIMES, false).code).toBe(1); + expect(describeFailure({ status: "no-cli", detail: "x" }, RUNTIMES, false).code).toBe(2); + }); + + it("suggests only the installed runtime when one is detected", () => { + const installed = RUNTIMES.map((r) => ({ ...r, installed: r.name === "OrbStack" })); + const text = describeFailure({ status: "no-daemon", detail: "x" }, installed, false) + .lines.map(plain) + .join("\n"); + expect(text).toContain("orb start"); + expect(text).not.toContain("open -a Docker"); + }); + + it("falls back to listing every runtime when none is detected", () => { + const text = describeFailure({ status: "no-daemon", detail: "x" }, RUNTIMES, false) + .lines.map(plain) + .join("\n"); + expect(text).toContain("orb start"); + expect(text).toContain("open -a Docker"); + expect(text).toContain("(not detected)"); + }); + + it("clips a long detail so it cannot bury the instructions", () => { + const long = "Cannot connect to the Docker daemon at unix:///x.sock. ".repeat(5); + const text = describeFailure({ status: "no-daemon", detail: long }, RUNTIMES, false) + .lines.map(plain) + .join("\n"); + expect(text).toContain("Cannot connect to the Docker daemon"); + expect(text).toContain("…"); + for (const line of text.split("\n")) expect(line.length).toBeLessThanOrEqual(120); + }); + + it("says install, not start, when there is no CLI to start", () => { + const text = describeFailure({ status: "no-cli", detail: "x" }, RUNTIMES, false) + .lines.map(plain) + .join("\n"); + expect(text).toContain("INSTALL ONE OF:"); + expect(text).not.toContain("START IT WITH:"); + // Auto-start cannot help when nothing is installed - don't advertise it. + expect(text).not.toContain("FMW_AUTO_START_DOCKER"); + }); + + it("surfaces the underlying detail and the auto-start opt-out", () => { + const text = describeFailure( + { status: "no-daemon", detail: "Cannot connect to the Docker daemon" }, + RUNTIMES, + false, + ) + .lines.map(plain) + .join("\n"); + expect(text).toContain("Cannot connect to the Docker daemon"); + expect(text).toContain("FMW_AUTO_START_DOCKER=1"); + }); + + it("stays readable with colour stripped - the glyph and words carry it", () => { + const { lines } = describeFailure({ status: "no-daemon", detail: "x" }, RUNTIMES, true); + const text = lines.map(plain).join("\n"); + expect(text).toContain("✖"); + expect(text).toContain("DOCKER IS NOT RUNNING"); + expect(text).toContain("START IT WITH:"); + }); +}); + +describe("describeSuccess", () => { + it("is one quiet line naming the version", () => { + const line = plain(describeSuccess("29.4.0", true)); + expect(line).toBe("✔ Docker daemon ready (29.4.0)"); + }); +});