From f41042744607d663c8d700573d982663596cf51d Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Fri, 30 Oct 2026 21:57:11 +0000 Subject: [PATCH 1/2] perf(cli): Give `__complete` its own light entry point Shell completion runs on every TAB press, but `sentry __complete` paid for most of the CLI before printing anything. The npm bin required the whole 7.6 MB `dist/index.cjs` (~340 ms per call), and the completion engine's static imports reached `@sentry/node` anyway: `db/auth.ts` and the cache modules imported telemetry, and `logger.ts` imported `createConsolaReporter`. Its eager graph was 659 modules. `src/bin.ts` and `dist/bin.cjs` now check for `__complete` first and call `runCompletion` in `lib/complete.ts` without loading `cli.ts`; the npm package ships it as a separate `dist/complete.cjs` (322 KB). The database modules get `withDbSpan` and `recordCacheHit` from `db/index.ts`, which requires telemetry lazily like the existing tracing proxy and skips it with telemetry off, and `initSentry` hands the logger its Sentry reporter. The eager graph is now 40 modules. Median on Node 24 (`issue list ""`): npm 356 -> 67 ms, SEA binary 145 -> 97 ms, bare node 26 ms. Completion output is unchanged. `test/script/cli-startup.test.ts` bundles `lib/complete.ts` and fails on any package outside an allowlist or more than 50 source modules, and the npm e2e test checks `__complete` never loads `index.cjs`. Co-Authored-By: BYK --- packages/cli/AGENTS.md | 15 ++ packages/cli/package.json | 1 + packages/cli/script/bundle.ts | 37 ++++- packages/cli/src/bin.ts | 22 ++- packages/cli/src/cli.ts | 39 +----- packages/cli/src/lib/complete.ts | 24 +++- packages/cli/src/lib/db/auth.ts | 3 +- packages/cli/src/lib/db/dsn-cache.ts | 3 +- packages/cli/src/lib/db/index.ts | 34 +++-- packages/cli/src/lib/db/issue-org-cache.ts | 3 +- packages/cli/src/lib/db/project-cache.ts | 3 +- packages/cli/src/lib/db/project-root-cache.ts | 3 +- packages/cli/src/lib/db/regions.ts | 3 +- packages/cli/src/lib/db/repo-cache.ts | 3 +- packages/cli/src/lib/db/user.ts | 3 +- packages/cli/src/lib/logger.ts | 11 +- packages/cli/src/lib/telemetry.ts | 2 +- packages/cli/test/e2e/bundle.test.ts | 29 +++- packages/cli/test/lib/logger.test.ts | 7 +- packages/cli/test/script/cli-startup.test.ts | 131 ++++++++++++------ 20 files changed, 255 insertions(+), 121 deletions(-) diff --git a/packages/cli/AGENTS.md b/packages/cli/AGENTS.md index b212ea208..82778a6ff 100644 --- a/packages/cli/AGENTS.md +++ b/packages/cli/AGENTS.md @@ -434,6 +434,21 @@ reads, SQLite, no network); ones that need a parser (`arg-parsing.ts`, the search grammar) import it when called, so the fast path only loads it for the arguments that need it. +The fast path runs on every TAB press, so it loads only `src/lib/complete.ts` +and what that module imports. `src/bin.ts` and the npm `dist/bin.cjs` check +for `__complete` first and call `runCompletion` without loading `cli.ts`; +the npm package ships it as the separate `dist/complete.cjs` bundle. That +graph must stay free of `@sentry/node`, telemetry, the API client, and the +command tree: the database modules get spans and cache metrics through +`withDbSpan` and `recordCacheHit` from `src/lib/db/index.ts`, which require +telemetry lazily and skip it when telemetry is off (`runCompletion` turns it +off), and `logger.ts` takes the Sentry reporter from `initSentry`. +`test/script/cli-startup.test.ts` bundles the graph and fails on any package +not in its `COMPLETION_PACKAGES` list, naming the importer, or on more source +modules than `COMPLETION_MODULE_BUDGET`. Defer a heavy dependency with a +dynamic `import()` in the completer that needs it rather than adding it to +the list. + ### Multiple Values in Arguments Split by argument type — do not mix the conventions: diff --git a/packages/cli/package.json b/packages/cli/package.json index 5e22afa97..4c5150f99 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -26,6 +26,7 @@ }, "files": [ "dist/bin.cjs", + "dist/complete.cjs", "dist/index.cjs", "dist/index.mjs", "dist/index.d.cts", diff --git a/packages/cli/script/bundle.ts b/packages/cli/script/bundle.ts index d6624cfca..5473deedf 100644 --- a/packages/cli/script/bundle.ts +++ b/packages/cli/script/bundle.ts @@ -257,6 +257,25 @@ const plugins: Plugin[] = [ requireAliasPlugin, ]; +const TELEMETRY_REQUIRE_RE = /^\.\.\/telemetry\.js$/; + +/** + * Leave telemetry (and `@sentry/node`) out of `dist/complete.cjs`. + * `runCompletion` turns telemetry off, so the lazy `require` of it in + * `src/lib/db/index.ts` never runs there; bundling it would only make every + * TAB press parse the SDK. + */ +const omitTelemetryPlugin: Plugin = { + name: "omit-telemetry", + setup(b) { + b.onResolve({ filter: TELEMETRY_REQUIRE_RE }, (args) => + args.kind === "require-call" + ? { path: args.path, external: true } + : undefined, + ); + }, +}; + if (process.env.SENTRY_AUTH_TOKEN) { console.log(" Sentry auth token found, source maps will be uploaded"); } else { @@ -343,6 +362,18 @@ const result = await build({ }, }); +// `sentry __complete` runs on every TAB press, so `dist/bin.cjs` loads this +// small bundle of `runCompletion` instead of the whole CLI in `index.cjs`. +const completeResult = await build({ + ...commonBuildOptions, + entryPoints: ["./src/lib/complete.ts"], + format: "cjs", + outfile: "./dist/complete.cjs", + inject: ["./script/import-meta-url.js"], + define: { ...commonDefine, "import.meta.url": "import_meta_url" }, + plugins: [...commonBuildOptions.plugins, omitTelemetryPlugin], +}); + const esmResult = await build({ ...commonBuildOptions, format: "esm", @@ -360,7 +391,7 @@ const esmResult = await build({ const BIN_WRAPPER = `#!/usr/bin/env node {let v=process.versions.node.split(".").map(Number);if(v[0]<18){console.error("Error: sentry requires Node.js 18 or later (found "+process.version+").\\n\\nEither upgrade Node.js, or install the standalone binary instead:\\n curl -fsSL https://cli.sentry.dev/install | bash\\n");process.exit(1)}} {let e=process.emit;process.emit=function(n,...a){return n==="warning"?!1:e.apply(this,[n,...a])}} -require('./index.cjs')._cli().catch(()=>{process.exitCode=1}); +process.argv[2]==="__complete"?require('./complete.cjs').runCompletion(process.argv.slice(3)).catch(()=>{process.exitCode=0}):require('./index.cjs')._cli().catch(()=>{process.exitCode=1}); `; await writeFile("./dist/bin.cjs", BIN_WRAPPER); @@ -440,11 +471,15 @@ console.log(" -> dist/node-sqlite3-wasm.wasm (WASM SQLite fallback)"); const bundleSizeKB = ( (result.metafile?.outputs["dist/index.cjs"]?.bytes ?? 0) / 1024 ).toFixed(1); +const completeBundleSizeKB = ( + (completeResult.metafile?.outputs["dist/complete.cjs"]?.bytes ?? 0) / 1024 +).toFixed(1); const esmBundleSizeKB = ( (esmResult.metafile?.outputs["dist/index.mjs"]?.bytes ?? 0) / 1024 ).toFixed(1); console.log(`\n -> dist/index.cjs (${bundleSizeKB} KB)`); console.log(` -> dist/index.mjs (${esmBundleSizeKB} KB)`); +console.log(` -> dist/complete.cjs (${completeBundleSizeKB} KB)`); console.log(`\n${"=".repeat(40)}`); console.log("Bundle complete!"); diff --git a/packages/cli/src/bin.ts b/packages/cli/src/bin.ts index e7821f6ec..01291a6ce 100755 --- a/packages/cli/src/bin.ts +++ b/packages/cli/src/bin.ts @@ -21,7 +21,6 @@ process.emit = ((event: string, ...args: unknown[]) => { return _origEmit(event, ...args); }) as typeof process.emit; -import { startCli } from "./cli.js"; import { wrapCall } from "./lib/react-native/wrap-call.js"; // React Native Xcode build wrapper: when the RN build script invokes us in @@ -50,7 +49,20 @@ function handleStreamError(err: NodeJS.ErrnoException): void { process.stdout.on("error", handleStreamError); process.stderr.on("error", handleStreamError); -// startCli handles its own errors — this is a safety net for truly unexpected rejections -startCli().catch(() => { - process.exitCode = 1; -}); +// Shell completion runs on every TAB press: load only the completion engine, +// never the full CLI (`@sentry/node`, the command tree). +if (process.argv[2] === "__complete") { + import("./lib/complete.js") + .then(({ runCompletion }) => runCompletion(process.argv.slice(3))) + .catch(() => { + // Completions should never crash — silently return no results + process.exitCode = 0; + }); +} else { + // startCli handles its own errors — this is a safety net for truly unexpected rejections + import("./cli.js") + .then(({ startCli }) => startCli()) + .catch(() => { + process.exitCode = 1; + }); +} diff --git a/packages/cli/src/cli.ts b/packages/cli/src/cli.ts index ecb59d39e..b3f27257b 100644 --- a/packages/cli/src/cli.ts +++ b/packages/cli/src/cli.ts @@ -1,9 +1,7 @@ /** - * CLI runner with fast-path dispatch. - * - * Shell completion (`__complete`) is dispatched before any heavy imports - * to avoid loading `@sentry/node` (~100ms). All other commands go through - * the full CLI with telemetry, middleware, and error recovery. + * CLI runner: every command goes through the full CLI with telemetry, + * middleware, and error recovery. Shell completion (`__complete`) never gets + * here: the bin entry points run `lib/complete.ts` without loading this module. * * Extracted from `bin.ts` so the logic is testable and reusable without * top-level side effects. `bin.ts` remains a thin wrapper that registers @@ -26,11 +24,9 @@ async function preloadProjectContext(cwd: string): Promise { // Snapshot env-token host BEFORE anything mutates env.SENTRY_HOST/URL // (the .sentryclirc shim or the default-URL fallback below). Pins the // env-token's trust scope to the user's shell, not a repo-local file. - // Dynamic import: env-token-host chains into db/auth → telemetry → @sentry/node const { captureEnvTokenHost } = await import("./lib/env-token-host.js"); captureEnvTokenHost(); - // Dynamic import keeps the heavy DSN/DB modules out of the completion fast-path const [{ findProjectRoot }, { setCachedProjectRoot }] = await Promise.all([ import("./lib/dsn/project-root.js"), import("./lib/db/project-root-cache.js"), @@ -67,20 +63,6 @@ async function preloadProjectContext(cwd: string): Promise { } } -/** - * Fast-path: shell completion. - * - * Dispatched before importing the full CLI to avoid loading @sentry/node, - * @stricli/core, and other heavy dependencies. Only loads the lightweight - * completion engine and SQLite cache modules. - */ -export async function runCompletion(completionArgs: string[]): Promise { - // Disable telemetry so db/index.ts skips the @sentry/node lazy-require (~100ms) - getEnv().SENTRY_CLI_NO_TELEMETRY = "1"; - const { handleComplete } = await import("./lib/complete.js"); - await handleComplete(completionArgs); -} - /** * Flags whose values must never be sent to telemetry. * Superset of `SENSITIVE_FLAGS` in `telemetry.ts` — includes `auth-token` @@ -310,9 +292,6 @@ export async function runMcpCommand(cliArgs: string[]): Promise { /** * Full CLI execution with telemetry, middleware, and error recovery. - * - * All heavy imports are loaded here (not at module top level) so the - * `__complete` fast-path can skip them entirely. */ export async function runCli(cliArgs: string[]): Promise { if (await runMcpCommand(cliArgs)) { @@ -748,8 +727,8 @@ export async function runCli(cliArgs: string[]): Promise { /** * Top-level CLI dispatch. * - * Reads `process.argv`, dispatches to the completion fast-path or the full - * CLI runner, and handles fatal errors. Called from `bin.ts`. + * Reads `process.argv`, runs the full CLI, and handles fatal errors. + * Called from `bin.ts`. */ export async function startCli(): Promise { const args = process.argv.slice(2); @@ -759,14 +738,6 @@ export async function startCli(): Promise { // timestamps (and all other Date output) appear in the wrong timezone. initTimezone(); - // Completions are a fast-path (~1ms) — skip .sentryclirc I/O. - if (args[0] === "__complete") { - return await runCompletion(args.slice(1)).catch(() => { - // Completions should never crash — silently return no results - process.exitCode = 0; - }); - } - // Walk up from CWD once to find project root AND .sentryclirc config. // Caches both so later findProjectRoot / loadSentryCliRc calls are hits. // Most failures here are non-fatal (unreadable rc file, missing project diff --git a/packages/cli/src/lib/complete.ts b/packages/cli/src/lib/complete.ts index 7725bf358..43ddd7934 100644 --- a/packages/cli/src/lib/complete.ts +++ b/packages/cli/src/lib/complete.ts @@ -37,6 +37,7 @@ import { queueCompletionTelemetry } from "./db/completion-telemetry.js"; import { getProjectAliases } from "./db/project-aliases.js"; import { getCachedProjectsForOrg } from "./db/project-cache.js"; import { getCachedOrganizations } from "./db/regions.js"; +import { getEnv } from "./env.js"; import { ValidationError } from "./errors.js"; import { fuzzyMatch } from "./fuzzy.js"; import type { CommandCompletions } from "./introspect.js"; @@ -65,15 +66,28 @@ type Completion = { }; /** - * Main entry point for `sentry __complete`. + * Run `sentry __complete`. * - * Called from the cli.ts fast-path. Parses the shell words to determine - * what kind of completion is needed, queries the cache, and writes - * results to stdout. + * `src/bin.ts` and the npm package's `dist/bin.cjs` (`dist/complete.cjs`) + * call this before anything else of the CLI loads, so a TAB press loads only + * this module's imports. `test/script/cli-startup.test.ts` keeps them free of + * `@sentry/node`, the API client, and the command tree. + * + * @param args - The words after `__complete` + */ +export async function runCompletion(args: string[]): Promise { + // Keeps the database modules from requiring telemetry and `@sentry/node` + getEnv().SENTRY_CLI_NO_TELEMETRY = "1"; + await handleComplete(args); +} + +/** + * Complete the shell words in `args`: parses them to determine what kind of + * completion is needed, queries the cache, and writes results to stdout. * * @param args - The words after `__complete` (COMP_WORDS[1:] from the shell) */ -export async function handleComplete(args: string[]): Promise { +async function handleComplete(args: string[]): Promise { const startMs = performance.now(); // The last word is the partial being completed (may be empty) diff --git a/packages/cli/src/lib/db/auth.ts b/packages/cli/src/lib/db/auth.ts index a41d54c6a..2f1507c75 100644 --- a/packages/cli/src/lib/db/auth.ts +++ b/packages/cli/src/lib/db/auth.ts @@ -40,7 +40,6 @@ import { AuthError, ConfigError } from "../errors.js"; import { logger } from "../logger.js"; import { parseBoolValue } from "../parse-bool.js"; import { normalizeHttpOrigin } from "../sentry-urls.js"; -import { withDbSpan } from "../telemetry.js"; import { type AccountRow, accountIdentity, @@ -64,7 +63,7 @@ import { } from "./accounts.js"; import { getAccountScope, resetAccountScopes } from "./account-scope.js"; import { hashIdentity } from "./identity-hash.js"; -import { getDatabase } from "./index.js"; +import { getDatabase, withDbSpan } from "./index.js"; import { clearCompletionCache } from "./completion-cache.js"; import { clearAllIssueOrgCache } from "./issue-org-cache.js"; import { clearTrustedHostState } from "./regions.js"; diff --git a/packages/cli/src/lib/db/dsn-cache.ts b/packages/cli/src/lib/db/dsn-cache.ts index 63f57bb4b..0af266d63 100644 --- a/packages/cli/src/lib/db/dsn-cache.ts +++ b/packages/cli/src/lib/db/dsn-cache.ts @@ -13,8 +13,7 @@ import type { DetectedDsn, ResolvedProjectInfo, } from "../dsn/types.js"; -import { recordCacheHit } from "../telemetry.js"; -import { getDatabase, maybeCleanupCaches } from "./index.js"; +import { getDatabase, maybeCleanupCaches, recordCacheHit } from "./index.js"; import { safeParseJson } from "./json.js"; import { runUpsert, touchCacheEntry } from "./utils.js"; diff --git a/packages/cli/src/lib/db/index.ts b/packages/cli/src/lib/db/index.ts index 929946cde..070b88634 100644 --- a/packages/cli/src/lib/db/index.ts +++ b/packages/cli/src/lib/db/index.ts @@ -224,16 +224,7 @@ export function getDatabase(): Database { } // Wrap with tracing proxy for automatic query instrumentation. - // Lazy-require telemetry to avoid top-level import of @sentry/node (~100ms). - // Shell completions set SENTRY_CLI_NO_TELEMETRY=1 to skip this entirely. - if (getEnv().SENTRY_CLI_NO_TELEMETRY === "1") { - db = rawDb; - } else { - const { createTracedDatabase } = _require("../telemetry.js") as { - createTracedDatabase: (d: Database) => Database; - }; - db = createTracedDatabase(rawDb); - } + db = lazyTelemetry()?.createTracedDatabase(rawDb) ?? rawDb; dbOpenedPath = dbPath; registerExitHandler(); @@ -246,6 +237,29 @@ export function getDatabase(): Database { } } +/** + * The telemetry module, required lazily so the database modules never import + * `@sentry/node` (~100ms). `undefined` when telemetry is off: the `__complete` + * fast path sets `SENTRY_CLI_NO_TELEMETRY=1` and never loads it. + */ +function lazyTelemetry(): typeof import("../telemetry.js") | undefined { + if (getEnv().SENTRY_CLI_NO_TELEMETRY === "1") { + return; + } + return _require("../telemetry.js") as typeof import("../telemetry.js"); +} + +/** Run a database operation in a `db.operation` span (telemetry's `withDbSpan`). */ +export function withDbSpan(operation: string, fn: () => T): T { + const telemetry = lazyTelemetry(); + return telemetry ? telemetry.withDbSpan(operation, fn) : fn(); +} + +/** Record a cache hit or miss (telemetry's `recordCacheHit`). */ +export function recordCacheHit(cacheName: string, hit: boolean): void { + lazyTelemetry()?.recordCacheHit(cacheName, hit); +} + /** Close the database connection (used for testing). */ export function closeDatabase(): void { if (db) { diff --git a/packages/cli/src/lib/db/issue-org-cache.ts b/packages/cli/src/lib/db/issue-org-cache.ts index a501567de..3e10710b0 100644 --- a/packages/cli/src/lib/db/issue-org-cache.ts +++ b/packages/cli/src/lib/db/issue-org-cache.ts @@ -24,8 +24,7 @@ * deletion, which we already handle via 404 eviction. */ -import { recordCacheHit } from "../telemetry.js"; -import { getDatabase } from "./index.js"; +import { getDatabase, recordCacheHit } from "./index.js"; import { runUpsert } from "./utils.js"; type IssueOrgRow = { diff --git a/packages/cli/src/lib/db/project-cache.ts b/packages/cli/src/lib/db/project-cache.ts index b81082354..16f254b62 100644 --- a/packages/cli/src/lib/db/project-cache.ts +++ b/packages/cli/src/lib/db/project-cache.ts @@ -3,8 +3,7 @@ */ import type { CachedProject } from "../../types/index.js"; -import { recordCacheHit } from "../telemetry.js"; -import { getDatabase, maybeCleanupCaches } from "./index.js"; +import { getDatabase, maybeCleanupCaches, recordCacheHit } from "./index.js"; import { runUpsert, touchCacheEntry } from "./utils.js"; type ProjectCacheRow = { diff --git a/packages/cli/src/lib/db/project-root-cache.ts b/packages/cli/src/lib/db/project-root-cache.ts index af30afb4d..55b75c817 100644 --- a/packages/cli/src/lib/db/project-root-cache.ts +++ b/packages/cli/src/lib/db/project-root-cache.ts @@ -10,8 +10,7 @@ import { stat } from "node:fs/promises"; import type { ProjectRootReason } from "../dsn/project-root.js"; -import { recordCacheHit } from "../telemetry.js"; -import { getDatabase, maybeCleanupCaches } from "./index.js"; +import { getDatabase, maybeCleanupCaches, recordCacheHit } from "./index.js"; import { runUpsert } from "./utils.js"; /** Cache TTL in milliseconds (24 hours) */ diff --git a/packages/cli/src/lib/db/regions.ts b/packages/cli/src/lib/db/regions.ts index 4fa60b2ab..5f8f20399 100644 --- a/packages/cli/src/lib/db/regions.ts +++ b/packages/cli/src/lib/db/regions.ts @@ -13,13 +13,12 @@ import { DEFAULT_SENTRY_URL, getConfiguredSentryUrl } from "../constants.js"; import { logger } from "../logger.js"; import { normalizeHttpOrigin } from "../sentry-urls.js"; -import { recordCacheHit } from "../telemetry.js"; import { getAccountIdForIdentity, getCredentialContext, getIdentityFingerprint, } from "./auth.js"; -import { getDatabase } from "./index.js"; +import { getDatabase, recordCacheHit } from "./index.js"; import { runUpsert } from "./utils.js"; const log = logger.withTag("db.regions"); diff --git a/packages/cli/src/lib/db/repo-cache.ts b/packages/cli/src/lib/db/repo-cache.ts index 287089a37..55eb58222 100644 --- a/packages/cli/src/lib/db/repo-cache.ts +++ b/packages/cli/src/lib/db/repo-cache.ts @@ -11,8 +11,7 @@ */ import type { SentryRepository } from "../../types/index.js"; -import { recordCacheHit } from "../telemetry.js"; -import { getDatabase } from "./index.js"; +import { getDatabase, recordCacheHit } from "./index.js"; import { runUpsert } from "./utils.js"; /** diff --git a/packages/cli/src/lib/db/user.ts b/packages/cli/src/lib/db/user.ts index 657a90720..22feaaa73 100644 --- a/packages/cli/src/lib/db/user.ts +++ b/packages/cli/src/lib/db/user.ts @@ -6,14 +6,13 @@ * `getEffectiveAccountRow`), for display and to set Sentry user context. */ -import { withDbSpan } from "../telemetry.js"; import { deleteAccountRows, isAccountOnServer, listAccountRows, } from "./accounts.js"; import { getEffectiveAccountRow } from "./auth.js"; -import { getDatabase } from "./index.js"; +import { getDatabase, withDbSpan } from "./index.js"; export type UserInfo = { userId: string; diff --git a/packages/cli/src/lib/logger.ts b/packages/cli/src/lib/logger.ts index 569b56c7b..be4c598fe 100644 --- a/packages/cli/src/lib/logger.ts +++ b/packages/cli/src/lib/logger.ts @@ -59,8 +59,7 @@ * @module */ -import { createConsolaReporter } from "@sentry/node"; -import type { ConsolaInstance } from "consola"; +import type { ConsolaInstance, ConsolaReporter } from "consola"; import { createConsola } from "consola"; import { getEnv } from "./env.js"; @@ -261,6 +260,8 @@ let sentryReporterAttached = false; * * Must be called after `Sentry.init()` completes so that `createConsolaReporter()` * can find the active client. Safe to call multiple times — subsequent calls are no-ops. + * The caller creates the reporter so this module never imports `@sentry/node`: + * the `__complete` fast path loads the logger through the database modules. * * The reporter auto-forwards all consola log messages to Sentry structured logs, * mapping consola types to Sentry severity levels: @@ -270,14 +271,12 @@ let sentryReporterAttached = false; * * Tags are preserved as `consola.tag` attributes in Sentry. */ -export function attachSentryReporter(): void { +export function attachSentryReporter(reporter: ConsolaReporter): void { if (sentryReporterAttached) { return; } - // Imported statically: the bundler leaves a runtime `require()` unresolved, - // which would either fail or load a host application's `@sentry/node`. - logger.addReporter(createConsolaReporter()); + logger.addReporter(reporter); sentryReporterAttached = true; } diff --git a/packages/cli/src/lib/telemetry.ts b/packages/cli/src/lib/telemetry.ts index f4a878f7b..9a473c8d9 100644 --- a/packages/cli/src/lib/telemetry.ts +++ b/packages/cli/src/lib/telemetry.ts @@ -787,7 +787,7 @@ export function initSentry( } // Wire up consola → Sentry log forwarding now that the client is active - attachSentryReporter(); + attachSentryReporter(Sentry.createConsolaReporter()); // End healthy sessions and flush events when the event loop drains. // The SDK's processSessionIntegration starts a session during init and diff --git a/packages/cli/test/e2e/bundle.test.ts b/packages/cli/test/e2e/bundle.test.ts index 2ff76095b..1c7442d6f 100644 --- a/packages/cli/test/e2e/bundle.test.ts +++ b/packages/cli/test/e2e/bundle.test.ts @@ -8,7 +8,7 @@ import { spawn } from "node:child_process"; import { existsSync } from "node:fs"; import { readFile } from "node:fs/promises"; -import { join } from "node:path"; +import { basename, join } from "node:path"; import { pathToFileURL } from "node:url"; import { beforeAll, describe, expect, test } from "vitest"; import { BUNDLE_BIN_PATH, ensureBundleBuilt } from "./bundle-setup.js"; @@ -188,6 +188,33 @@ describe("npm bundle", () => { expect(output).not.toContain("MODULE_NOT_FOUND"); }, 15_000); + test("__complete runs from dist/complete.cjs without loading the CLI", async () => { + // Print the modules the run loaded, so the test sees what a TAB press pays for + const { stdout, stderr, exitCode } = await spawnCollect( + "node", + [ + "-e", + `process.on("exit", () => process.stderr.write(JSON.stringify(Object.keys(require.cache)))); + process.argv.splice(1, 0, ${JSON.stringify(BUNDLE_BIN_PATH)}); + require(process.argv[1]);`, + "__complete", + "issue", + "list", + "--period", + "", + ], + { cwd: ROOT_DIR }, + ); + + expect(exitCode).toBe(0); + expect(stdout).toContain("24h\tLast 24 hours"); + const loaded = (JSON.parse(stderr) as string[]).map((path) => + basename(path), + ); + expect(loaded).toContain("complete.cjs"); + expect(loaded).not.toContain("index.cjs"); + }, 15_000); + test("Ink sidecar can be imported and exports mountApp", async () => { // The Ink sidecar (dist/ink-app.js) is a pre-bundled self-contained ESM // module that ships with the npm package. Verify it exists, can be imported diff --git a/packages/cli/test/lib/logger.test.ts b/packages/cli/test/lib/logger.test.ts index 6a7044f02..06ee25eb3 100644 --- a/packages/cli/test/lib/logger.test.ts +++ b/packages/cli/test/lib/logger.test.ts @@ -5,6 +5,7 @@ * attachSentryReporter, and the logger instance configuration. */ +import { createConsolaReporter } from "@sentry/node"; import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; import { attachSentryReporter, @@ -271,13 +272,13 @@ describe("attachSentryReporter", () => { test("can be called without error", () => { // attachSentryReporter is idempotent and safe to call even when // Sentry is not initialized (it catches errors internally) - expect(() => attachSentryReporter()).not.toThrow(); + expect(() => attachSentryReporter(createConsolaReporter())).not.toThrow(); }); test("is idempotent (second call is a no-op)", () => { // First call may or may not succeed depending on Sentry state - attachSentryReporter(); + attachSentryReporter(createConsolaReporter()); // Second call should be a no-op due to internal guard - expect(() => attachSentryReporter()).not.toThrow(); + expect(() => attachSentryReporter(createConsolaReporter())).not.toThrow(); }); }); diff --git a/packages/cli/test/script/cli-startup.test.ts b/packages/cli/test/script/cli-startup.test.ts index 1b998f8fe..6f55bd444 100644 --- a/packages/cli/test/script/cli-startup.test.ts +++ b/packages/cli/test/script/cli-startup.test.ts @@ -1,4 +1,4 @@ -/** Keep Sentry SDK dependencies out of the completion startup path. */ +/** Keep Sentry SDK dependencies out of the CLI's startup paths. */ import { join } from "node:path"; import { build } from "esbuild"; @@ -6,47 +6,100 @@ import { expect, test } from "vitest"; const ANY_IMPORT = /.*/; -test.each(["cli.ts", "index.ts"])( +/** Hostname and origin predicates are pure and have no startup side effects. */ +const PURE_SENTRY_IMPORTS = [ + "@sentry/toolkit-core/sentry-host", + "@sentry/toolkit-core/sentry-origin", +]; + +/** + * Every package `sentry __complete` may load. It runs on each TAB press, so a + * new one must be cheap to load: never `@sentry/node`, the API client, or + * anything that boots the command tree. + */ +const COMPLETION_PACKAGES = new Set([ + ...PURE_SENTRY_IMPORTS, + "@sentry/toolkit-core/auth-token", + // `errors.ts` (exit codes) + "@stricli/core", + // `logger.ts`, which the database modules log through + "consola", + // `db/accounts.ts` account IDs + "uuidv7", +]); + +/** + * Source modules `sentry __complete` loads before it prints anything. Raise it + * only for a completer that needs the modules. + */ +const COMPLETION_MODULE_BUDGET = 50; + +/** + * Bundle what loading `entry` executes, failing on a package import + * `isAllowed` rejects. Deferred imports do not execute when an entry point + * loads, so they are left out. + */ +function bundleStartup( + entry: string, + isAllowed: (specifier: string) => boolean, +) { + return build({ + entryPoints: [join(import.meta.dirname, "../../src", entry)], + bundle: true, + write: false, + metafile: true, + platform: "node", + format: "esm", + logLevel: "silent", + plugins: [ + { + name: "check-startup-imports", + setup(builder) { + builder.onResolve({ filter: ANY_IMPORT }, (args) => { + if (args.kind === "dynamic-import") { + return { path: args.path, external: true }; + } + const isPackage = !( + args.kind === "entry-point" || + args.path.startsWith(".") || + args.path.startsWith("node:") + ); + if (isPackage && !isAllowed(args.path)) { + return { + errors: [ + { text: `Eager import ${args.path} from ${args.importer}` }, + ], + }; + } + return; + }); + }, + }, + ], + }); +} + +test.each(["bin.ts", "cli.ts", "index.ts"])( "%s does not eagerly import the Sentry SDK", async (entry) => { + // Traverse the pure predicates' source to catch future SDK imports. await expect( - build({ - entryPoints: [join(import.meta.dirname, "../../src", entry)], - bundle: true, - write: false, - platform: "node", - format: "esm", - logLevel: "silent", - plugins: [ - { - name: "check-startup-imports", - setup(builder) { - builder.onResolve({ filter: ANY_IMPORT }, (args) => { - // Deferred imports do not execute when an entry point loads. - if (args.kind === "dynamic-import") { - return { path: args.path, external: true }; - } - // Hostname and origin predicates are pure and have no startup - // side effects. Traverse their source to catch future SDK imports. - if ( - args.path.startsWith("@sentry/") && - args.path !== "@sentry/toolkit-core/sentry-host" && - args.path !== "@sentry/toolkit-core/sentry-origin" - ) { - return { - errors: [ - { - text: `Eager SDK dependency ${args.path} from ${args.importer}`, - }, - ], - }; - } - return; - }); - }, - }, - ], - }), + bundleStartup( + entry, + (specifier) => + !specifier.startsWith("@sentry/") || + PURE_SENTRY_IMPORTS.includes(specifier), + ), ).resolves.toMatchObject({ errors: [] }); }, ); + +test("the __complete fast path loads only light modules", async () => { + const { metafile } = await bundleStartup("lib/complete.ts", (specifier) => + COMPLETION_PACKAGES.has(specifier), + ); + const sourceModules = Object.keys(metafile.inputs).filter( + (input) => !input.includes("node_modules"), + ); + expect(sourceModules.length).toBeLessThanOrEqual(COMPLETION_MODULE_BUDGET); +}); From fd2a64a76d8df5102fa59a60da9476c59458a0d6 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Sat, 31 Oct 2026 14:05:39 +0000 Subject: [PATCH 2/2] ref(cli): Gate telemetry out of the completion bundle with a build define Replace the esbuild resolve plugin that externalized the lazy telemetry require with a SENTRY_CLI_COMPLETION_BUILD define. The complete.cjs build sets it to true, so esbuild drops the require and @sentry/node from that bundle. Source runs keep the SENTRY_CLI_NO_TELEMETRY check. --- packages/cli/AGENTS.md | 4 +++- packages/cli/script/bundle.ts | 29 ++++++++--------------------- packages/cli/src/lib/db/index.ts | 13 ++++++++++++- 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/packages/cli/AGENTS.md b/packages/cli/AGENTS.md index 82778a6ff..c9da617ab 100644 --- a/packages/cli/AGENTS.md +++ b/packages/cli/AGENTS.md @@ -442,7 +442,9 @@ graph must stay free of `@sentry/node`, telemetry, the API client, and the command tree: the database modules get spans and cache metrics through `withDbSpan` and `recordCacheHit` from `src/lib/db/index.ts`, which require telemetry lazily and skip it when telemetry is off (`runCompletion` turns it -off), and `logger.ts` takes the Sentry reporter from `initSentry`. +off; `dist/complete.cjs` is built with `SENTRY_CLI_COMPLETION_BUILD` set to +`true`, so the bundle leaves telemetry out), and `logger.ts` takes the Sentry +reporter from `initSentry`. `test/script/cli-startup.test.ts` bundles the graph and fails on any package not in its `COMPLETION_PACKAGES` list, naming the importer, or on more source modules than `COMPLETION_MODULE_BUDGET`. Defer a heavy dependency with a diff --git a/packages/cli/script/bundle.ts b/packages/cli/script/bundle.ts index 5473deedf..02c76fd45 100644 --- a/packages/cli/script/bundle.ts +++ b/packages/cli/script/bundle.ts @@ -257,25 +257,6 @@ const plugins: Plugin[] = [ requireAliasPlugin, ]; -const TELEMETRY_REQUIRE_RE = /^\.\.\/telemetry\.js$/; - -/** - * Leave telemetry (and `@sentry/node`) out of `dist/complete.cjs`. - * `runCompletion` turns telemetry off, so the lazy `require` of it in - * `src/lib/db/index.ts` never runs there; bundling it would only make every - * TAB press parse the SDK. - */ -const omitTelemetryPlugin: Plugin = { - name: "omit-telemetry", - setup(b) { - b.onResolve({ filter: TELEMETRY_REQUIRE_RE }, (args) => - args.kind === "require-call" - ? { path: args.path, external: true } - : undefined, - ); - }, -}; - if (process.env.SENTRY_AUTH_TOKEN) { console.log(" Sentry auth token found, source maps will be uploaded"); } else { @@ -334,6 +315,9 @@ const commonDefine: Record = { // Tree-shakes the SDK's runtime module hooks, which can't instrument a // bundle and would hook the host app in library mode. __SENTRY_CHANNEL_INJECTION__: "false", + // `true` only in `dist/complete.cjs`: drops the lazy telemetry `require` + // (and `@sentry/node`) from `src/lib/db/index.ts` there. + SENTRY_CLI_COMPLETION_BUILD: "false", }; // In an ESM bundle, `require`, `__dirname`, and `__filename` don't exist, but @@ -370,8 +354,11 @@ const completeResult = await build({ format: "cjs", outfile: "./dist/complete.cjs", inject: ["./script/import-meta-url.js"], - define: { ...commonDefine, "import.meta.url": "import_meta_url" }, - plugins: [...commonBuildOptions.plugins, omitTelemetryPlugin], + define: { + ...commonDefine, + SENTRY_CLI_COMPLETION_BUILD: "true", + "import.meta.url": "import_meta_url", + }, }); const esmResult = await build({ diff --git a/packages/cli/src/lib/db/index.ts b/packages/cli/src/lib/db/index.ts index 070b88634..21249f619 100644 --- a/packages/cli/src/lib/db/index.ts +++ b/packages/cli/src/lib/db/index.ts @@ -237,13 +237,24 @@ export function getDatabase(): Database { } } +/** + * `true` in the npm package's `dist/complete.cjs` (defined in + * `script/bundle.ts`), so esbuild drops the telemetry `require` below from + * that bundle. Undefined when running from source. + */ +declare const SENTRY_CLI_COMPLETION_BUILD: boolean | undefined; + /** * The telemetry module, required lazily so the database modules never import * `@sentry/node` (~100ms). `undefined` when telemetry is off: the `__complete` * fast path sets `SENTRY_CLI_NO_TELEMETRY=1` and never loads it. */ function lazyTelemetry(): typeof import("../telemetry.js") | undefined { - if (getEnv().SENTRY_CLI_NO_TELEMETRY === "1") { + if ( + (typeof SENTRY_CLI_COMPLETION_BUILD !== "undefined" && + SENTRY_CLI_COMPLETION_BUILD) || + getEnv().SENTRY_CLI_NO_TELEMETRY === "1" + ) { return; } return _require("../telemetry.js") as typeof import("../telemetry.js");