From 73ad29bc9cfdfaa947eb7a9bd4a220001fedb05f Mon Sep 17 00:00:00 2001 From: UtkarshBhardwaj007 Date: Tue, 21 Apr 2026 12:43:58 +0100 Subject: [PATCH] fix(deploy): opt out of bulletin-deploy memory-report on Bun MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `dot deploy` crashed on Bun-compiled binaries with "node:v8 getHeapSpaceStatistics is not yet implemented in Bun" when run from a cwd whose git remote matched an internal Parity org. The old `BULLETIN_DEPLOY_TELEMETRY=0` assignment lived below the ES-module imports in `src/index.ts`, so bulletin-deploy's `telemetry.ts` had already computed its `DISABLED` gate before the opt-out ran. Sentry loaded, `withDeploySpan`'s finally fired `maybeWriteMemoryReport`, which calls `v8.getHeapSpaceStatistics()` — unimplemented in Bun. Move the opt-out into `src/bootstrap.ts`, imported as the very first import in `src/index.ts` so its side effects run before any sibling import's top level. Also force `BULLETIN_DEPLOY_MEM_REPORT=0` by default — that gate is checked at call time inside `maybeWriteMemoryReport`, so it stops the v8 call even if a user explicitly opts telemetry back in. Both vars still honour an explicit user override. --- .../opt-out-bulletin-deploy-mem-report.md | 5 +++ CLAUDE.md | 2 +- src/bootstrap.test.ts | 43 +++++++++++++++++++ src/bootstrap.ts | 41 ++++++++++++++++++ src/index.ts | 13 ++---- 5 files changed, 94 insertions(+), 10 deletions(-) create mode 100644 .changeset/opt-out-bulletin-deploy-mem-report.md create mode 100644 src/bootstrap.test.ts create mode 100644 src/bootstrap.ts diff --git a/.changeset/opt-out-bulletin-deploy-mem-report.md b/.changeset/opt-out-bulletin-deploy-mem-report.md new file mode 100644 index 0000000..c14a17a --- /dev/null +++ b/.changeset/opt-out-bulletin-deploy-mem-report.md @@ -0,0 +1,5 @@ +--- +"playground-cli": patch +--- + +Fix `dot deploy` crashing on Bun-compiled binaries with `node:v8 getHeapSpaceStatistics is not yet implemented in Bun.` when running from an internal Parity repo. Move the `bulletin-deploy` telemetry opt-out into a dedicated `src/bootstrap.ts` side-effect module imported before any other module, and additionally force `BULLETIN_DEPLOY_MEM_REPORT=0` so bulletin-deploy's diagnostic memory-report path can never reach Bun's unimplemented `v8.getHeapSpaceStatistics`. Explicit `BULLETIN_DEPLOY_TELEMETRY=1` / `BULLETIN_DEPLOY_MEM_REPORT=1` overrides are preserved. diff --git a/CLAUDE.md b/CLAUDE.md index 4e8561f..23959c9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -20,7 +20,7 @@ These are things that aren't self-evident from reading the code and have bitten - **Bun compiled-binary stdin quirk** — Ink's `useInput` silently drops every keystroke (arrows, Enter, Ctrl+C) in `bun build --compile` binaries unless `process.stdin.on('readable', …)` is touched before Ink's `render()`. We install a no-op `readable` listener at the top of `src/index.ts` as a warm-up. Do NOT remove it until Bun's compiled-binary TTY stdin behaves like Node's. Symptom if this breaks: TUI renders but nothing responds, including Ctrl+C. - **`bulletin-deploy` is pinned to an explicit version, not `latest`.** We're on `0.7.0` stable today. The `latest` npm dist-tag is a moving target and previously pointed at 0.6.8, which has a WebSocket heartbeat bug (default 40s < chunk timeout 60s) that tears down uploads mid-flight as `WS halt (3)`. Keep the pin explicit so we never silently slide onto a broken `latest`. When upgrading, read the release notes for any public-API changes to `deploy()`, `DotNS` methods, or the `DeployOptions` we rely on (`jsMerkle`, `signer`, `signerAddress`, `mnemonic`, `rpc`, `attributes`). Note: 0.7.0 removed the `playground?: boolean` `DeployOption` (registry publishing now lives here in `src/utils/deploy/playground.ts`), which is a no-op for us since we never passed that flag. - **Throttle TUI info updates** — bulletin-deploy logs per-chunk and builds (vite/next) stream thousands of lines/sec. Calling `setState` on every log event floods React's reconciler with so much backpressure the process can balloon past 20 GB and freeze the OS. `RunningStage` coalesces "latest info" updates to ≤10/sec via a ref + timer and caps line length at 160 chars. Any new hot-path event sink should do the same; don't hook raw per-line streams directly into Ink state. -- **Process-guard safety net** (`src/utils/process-guard.ts`) — deploy pipelines open several long-lived WebSockets + child processes and any one of them can keep the event loop alive after the TUI visibly finishes, turning `dot` into a zombie that accumulates retry buffers indefinitely (seen climbing past 25 GB). We defend in depth: (1) `installSignalHandlers()` catches SIGINT/TERM/HUP + `unhandledRejection` and forces cleanup + exit within 3 s; (2) `scheduleHardExit()` installs an `unref`'d timer that kills the process if the event loop doesn't drain within a grace period; (3) `startMemoryWatchdog()` aborts if RSS exceeds 4 GB — a generous cap because legit deploys on Bun SEA binaries routinely touch 1–1.5 GB from runtime-metadata decoding + Bun's JSC heap + Ink yoga. Do NOT re-add a per-window growth detector: we tried 300 MB / 3 s and it false-positived on the single-burst metadata-loading spike, aborting deploys that would have succeeded. Set `DOT_MEMORY_TRACE=1` to stream per-sample RSS/heap/external stats — useful when diagnosing a real leak report. `BULLETIN_DEPLOY_TELEMETRY` is also forced to `"0"` at CLI entry — Sentry buffers breadcrumbs in-memory. Any new long-running command should register a cleanup hook via `onProcessShutdown()`. +- **Process-guard safety net** (`src/utils/process-guard.ts`) — deploy pipelines open several long-lived WebSockets + child processes and any one of them can keep the event loop alive after the TUI visibly finishes, turning `dot` into a zombie that accumulates retry buffers indefinitely (seen climbing past 25 GB). We defend in depth: (1) `installSignalHandlers()` catches SIGINT/TERM/HUP + `unhandledRejection` and forces cleanup + exit within 3 s; (2) `scheduleHardExit()` installs an `unref`'d timer that kills the process if the event loop doesn't drain within a grace period; (3) `startMemoryWatchdog()` aborts if RSS exceeds 4 GB — a generous cap because legit deploys on Bun SEA binaries routinely touch 1–1.5 GB from runtime-metadata decoding + Bun's JSC heap + Ink yoga. Do NOT re-add a per-window growth detector: we tried 300 MB / 3 s and it false-positived on the single-burst metadata-loading spike, aborting deploys that would have succeeded. Set `DOT_MEMORY_TRACE=1` to stream per-sample RSS/heap/external stats — useful when diagnosing a real leak report. `BULLETIN_DEPLOY_TELEMETRY` and `BULLETIN_DEPLOY_MEM_REPORT` are both forced to `"0"` from `src/bootstrap.ts`, which is the FIRST import in `src/index.ts`. Import ordering matters: bulletin-deploy's `telemetry.ts` computes its `DISABLED` flag at module load, so a plain `process.env.X = "0"` executed later in `index.ts` is too late — ES-module import hoisting has already evaluated the Sentry gate. The fix relies on ordered sibling evaluation (ECMA-262 §16.2 `InnerModuleEvaluation` walks `[[RequestedModules]]` in source order), which both Node's ESM loader and `bun build --compile` preserve today; a hypothetical future parallelising bundler would break this assumption and the Sentry gate would race the opt-out again. `src/bootstrap.test.ts` asserts the structural invariant that bootstrap is the first import in `src/index.ts`. The memory-report opt-out is belt-and-suspenders: that gate is checked at call time inside `maybeWriteMemoryReport`, and stops bulletin-deploy from calling `v8.getHeapSpaceStatistics()` (unimplemented in Bun, crashes our compiled binary with "node:v8 getHeapSpaceStatistics is not yet implemented in Bun") even if a user explicitly re-enables telemetry. Any new long-running command should register a cleanup hook via `onProcessShutdown()`. - **Parser MUST NOT emit an event per log line.** `DeployLogParser.feed()` is called for every console line bulletin-deploy prints — hundreds per deploy on the happy path, thousands if retries fire. We intentionally emit events ONLY for phase-banner matches and `[N/M]` chunk progress. Everything else returns `null`. Adding a catch-all `info` emit turns the parser into a firehose that allocates ~200 bytes × thousands of lines, and was a measurable contributor to chunk-upload memory pressure. ## Repo conventions diff --git a/src/bootstrap.test.ts b/src/bootstrap.test.ts new file mode 100644 index 0000000..f859f86 --- /dev/null +++ b/src/bootstrap.test.ts @@ -0,0 +1,43 @@ +import { readFileSync } from "node:fs"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +describe("bootstrap", () => { + beforeEach(() => { + delete process.env.BULLETIN_DEPLOY_TELEMETRY; + delete process.env.BULLETIN_DEPLOY_MEM_REPORT; + vi.resetModules(); + }); + + it("forces BULLETIN_DEPLOY_TELEMETRY=0 when the user has not set it", async () => { + await import("./bootstrap.js"); + expect(process.env.BULLETIN_DEPLOY_TELEMETRY).toBe("0"); + }); + + it("forces BULLETIN_DEPLOY_MEM_REPORT=0 when the user has not set it", async () => { + await import("./bootstrap.js"); + expect(process.env.BULLETIN_DEPLOY_MEM_REPORT).toBe("0"); + }); + + it("preserves an explicit BULLETIN_DEPLOY_TELEMETRY opt-in", async () => { + process.env.BULLETIN_DEPLOY_TELEMETRY = "1"; + await import("./bootstrap.js"); + expect(process.env.BULLETIN_DEPLOY_TELEMETRY).toBe("1"); + }); + + it("preserves an explicit BULLETIN_DEPLOY_MEM_REPORT opt-in", async () => { + process.env.BULLETIN_DEPLOY_MEM_REPORT = "1"; + await import("./bootstrap.js"); + expect(process.env.BULLETIN_DEPLOY_MEM_REPORT).toBe("1"); + }); + + // Load-bearing structural invariant: if bootstrap stops being the first + // import of `src/index.ts`, the bulletin-deploy import chain will evaluate + // its `DISABLED` gate before our env vars are set — which is the exact bug + // this module exists to prevent. A Biome reorder / careless rebase could + // silently break the fix; this test nails the ordering down. + it("is the first import in src/index.ts", () => { + const src = readFileSync("src/index.ts", "utf-8"); + const firstImport = src.match(/^import\s+(?:[^;]+\s+from\s+)?["']([^"']+)["'];?$/m); + expect(firstImport?.[1]).toBe("./bootstrap.js"); + }); +}); diff --git a/src/bootstrap.ts b/src/bootstrap.ts new file mode 100644 index 0000000..7e7f367 --- /dev/null +++ b/src/bootstrap.ts @@ -0,0 +1,41 @@ +// Env-var opt-outs that MUST be applied before any `bulletin-deploy` module +// evaluates. ES-module top-level evaluation is dependency-first + ordered +// across siblings of the same parent, so importing this module as the very +// first statement in `src/index.ts` guarantees its side effects run before +// the `bulletin-deploy` import chain initialises its Sentry / memory-report +// gates. Plain `process.env.X = "0"` later in `index.ts` is too late — +// import hoisting would have already loaded Sentry and wired up the +// threshold-triggered memory report. +// +// Why we opt out by default: +// 1. Sentry buffers breadcrumbs + spans in-memory while it tries to reach +// its endpoint; on a flaky or long-running deploy this has been seen to +// balloon `dot`'s RSS. +// 2. bulletin-deploy's memory-report path calls `v8.getHeapSpaceStatistics()`, +// which Bun has not implemented. Our CLI ships as a Bun-compiled binary, +// so reaching that code path kills the deploy with +// "node:v8 getHeapSpaceStatistics is not yet implemented in Bun." +// +// Both gates honour an explicit user override: if the caller sets either +// env var before invoking `dot`, we leave their choice alone. That lets +// Parity folks opt back in with `BULLETIN_DEPLOY_TELEMETRY=1 dot deploy` +// when they want to help debug a deploy. + +// Forces ESM module semantics on this otherwise-import-free file. Without it, +// TS classifies the file as a script and `await import("./bootstrap.js")` +// resolves to `{ default: undefined }` with no export binding — TS2306 at +// every callsite in `bootstrap.test.ts`. Keep. +export {}; + +if (process.env.BULLETIN_DEPLOY_TELEMETRY === undefined) { + process.env.BULLETIN_DEPLOY_TELEMETRY = "0"; +} + +// Defence-in-depth: the memory-report env gate lives inside +// `maybeWriteMemoryReport`, checked at call time rather than module load. +// Even if a user explicitly opts telemetry back in, this stops the +// Bun-incompatible `v8.getHeapSpaceStatistics` call from firing in our +// Bun-compiled binary. +if (process.env.BULLETIN_DEPLOY_MEM_REPORT === undefined) { + process.env.BULLETIN_DEPLOY_MEM_REPORT = "0"; +} diff --git a/src/index.ts b/src/index.ts index 15750c6..0ac494f 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,5 +1,9 @@ #!/usr/bin/env node +// MUST be the first import — sets env vars that gate bulletin-deploy's +// Sentry + memory-report paths before their modules evaluate. See +// `src/bootstrap.ts` for the rationale. +import "./bootstrap.js"; import { Command } from "commander"; import pkg from "../package.json" with { type: "json" }; import { initCommand } from "./commands/init/index.js"; @@ -26,15 +30,6 @@ if (process.stdin.isTTY) { process.stdin.unref(); } -// Opt out of bulletin-deploy's Sentry telemetry unless the user has -// explicitly opted in. Sentry buffers breadcrumbs + spans in-memory while -// it tries to reach its endpoint — on a flaky or long-running deploy this -// has been observed to balloon the process. Users can re-enable by setting -// `BULLETIN_DEPLOY_TELEMETRY=1` before invoking `dot deploy`. -if (process.env.BULLETIN_DEPLOY_TELEMETRY === undefined) { - process.env.BULLETIN_DEPLOY_TELEMETRY = "0"; -} - // Install SIGINT/SIGTERM/SIGHUP + unhandledRejection handlers so a force-quit // or a stray async error can't turn `dot` into a zombie that grows memory // indefinitely.