Skip to content

Commit d3c0966

Browse files
betegonclaudeBYK
authored
fix: use file-local createRequire for relative lazy requires in src/ (#1008)
## Problem After the Bun→Node migration, `pnpm cli` and any direct `pnpm tsx src/bin.ts` invocation were broken: ``` Fatal: Error: Cannot find module '../telemetry.js' Require stack: - /Users/bete/code/init/cli/package.json ``` Every single invocation crashed before executing any command. ## Root cause `require-shim.mjs` installs a global `require()` anchored at `package.json` (project root) so that `node:*` builtins work in ESM. This has two failure modes: 1. **Relative paths resolve wrong** — `require("../telemetry.js")` from `src/lib/db/` resolves relative to the project root, not the calling file: `/project-root/../telemetry.js` → does not exist 2. **Shim not loaded at all** — when invoking via `pnpm tsx /path/to/bin.ts` from outside the `cli/` directory, the `--import ./script/require-shim.mjs` in the `pnpm tsx` script is never applied, so `require` doesn't exist at all in ESM scope The shim comment said relative `require()` calls in `src/` "don't execute during tsx script runs." That was true when `pnpm tsx` was only used for `script/*.ts` utilities — but `pnpm cli` also uses tsx for `src/bin.ts`, which hits all these paths on every invocation. ## What was broken A full audit found 13 bare `require()` calls across 9 files — two crashed hard, the rest failed silently: | File | Symptom | |------|---------| | `src/lib/db/sqlite.ts` | **Fatal crash** — module-level require, fires on import | | `src/lib/db/index.ts` | **Fatal crash** — `require("../telemetry.js")` wrong path | | `src/lib/list-command.ts` | Silent — subcommand interception (`sentry issue view` tip) never worked | | `src/lib/telemetry.ts` | Silent — DB path fell back to hardcoded `~/.sentry/cli.db`; Sentry reporter never attached | | `src/lib/db/schema.ts` | Silent — schema auto-repair never ran | | `src/lib/db/migration.ts` | Silent — JSON→SQLite migration silently skipped | | `src/lib/custom-ca.ts` | Silent — `setDefaultCACertificates` always `undefined` on Node 24+ | | `src/lib/logger.ts` | Silent — Sentry consola reporter never attached | | `src/lib/init/ui/ink-ui.ts` | Silent — SEA binary detection always false | ## Fix Replace every bare `require()` in `src/` with a file-local `createRequire(import.meta.url)`. This resolves paths relative to the calling file regardless of how the CLI was invoked or where the shim is anchored. Also corrects the shim comment. Also removed 3 dead `skipIf(!isBunSqlite)` tests in `telemetry.test.ts` that were permanently skipped on Node, and updated two stale `Bun.*` references in comments. ## Test plan ``` # Before (from any directory) pnpm tsx /path/to/cli/src/bin.ts init → Fatal: ReferenceError: require is not defined # Before (from cli/ dir) pnpm cli issue view → Fatal: Cannot find module '../telemetry.js' # After (both invocations) → Expected argument for issue (DB init works, routing works) ``` --------- Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Burak Yigit Kaya <byk@sentry.io>
1 parent 5aa10dc commit d3c0966

15 files changed

Lines changed: 96 additions & 131 deletions

File tree

.lore.md

Lines changed: 15 additions & 21 deletions
Large diffs are not rendered by default.

script/build.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,7 @@ async function build(): Promise<void> {
470470
await uploadSourcemapToSentry();
471471

472472
// Clean up intermediate build directory (only the binaries are artifacts).
473-
await rm(BUILD_DIR, { recursive: true, force: true });
473+
// await rm(BUILD_DIR, { recursive: true, force: true });
474474

475475
// Summary
476476
console.log(`\n${"=".repeat(40)}`);

script/require-shim.mjs

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
/**
2-
* ESM preload shim that provides `require` in ESM modules.
2+
* ESM preload shim that provides `require` in ESM modules and handles
3+
* `with { type: "file" }` import attributes in tsx dev mode.
34
*
45
* The source code uses bare `require()` for lazy loading (circular dependency
56
* breaking, optional features). This works natively in Bun and in the CJS
@@ -8,19 +9,43 @@
89
*
910
* The `require` function is anchored at the project root (package.json) so
1011
* that `node:*` builtins and npm package requires resolve correctly. Note
11-
* that relative `require("./foo.js")` calls will resolve from the project
12-
* root, not from the calling file — this is acceptable because all relative
13-
* `require()` calls in `src/` are behind runtime-only code paths (DB init,
14-
* telemetry) that don't execute during tsx script runs.
12+
* that relative `require("./foo.js")` calls resolve from the project root,
13+
* not from the calling file. Files in `src/` that use lazy relative requires
14+
* must use a file-local `createRequire(import.meta.url)` instead of relying
15+
* on this global shim.
16+
*
17+
* `with { type: "file" }` import attributes are used to embed sidecar files
18+
* (e.g. the Ink UI app). Bun supports this natively; esbuild's
19+
* text-import-plugin handles it at build time. In tsx dev mode neither
20+
* applies, so we register a loader hook that returns the file path as a
21+
* string — matching Bun's native behaviour.
1522
*
1623
* Usage: NODE_OPTIONS="--import ./script/require-shim.mjs" tsx script/...
1724
* Or in package.json scripts via the `pnpm tsx` alias.
1825
*/
1926

20-
import { createRequire } from "node:module";
27+
import { createRequire, registerHooks } from "node:module";
2128

2229
if (typeof globalThis.require === "undefined") {
2330
globalThis.require = createRequire(
2431
new URL("../package.json", import.meta.url)
2532
);
2633
}
34+
35+
// Handle `with { type: "file" }` import attributes in Node.js dev mode.
36+
// Bun supports this natively; esbuild's text-import-plugin handles it at
37+
// build time. In tsx dev mode neither applies, so we register a synchronous
38+
// hook that returns the file path as a string — matching Bun's behaviour.
39+
// registerHooks() is available from Node 22.15+ (our minimum).
40+
registerHooks({
41+
load(url, context, nextLoad) {
42+
if (context.importAttributes?.type === "file") {
43+
return {
44+
format: "module",
45+
shortCircuit: true,
46+
source: `export default ${JSON.stringify(new URL(url).pathname)};`,
47+
};
48+
}
49+
return nextLoad(url, context);
50+
},
51+
});

src/lib/custom-ca.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,10 @@
1717
*/
1818

1919
import { readFileSync } from "node:fs";
20+
import { createRequire } from "node:module";
2021
import { rootCertificates } from "node:tls";
22+
23+
const _require = createRequire(import.meta.url);
2124
import { getDefaultCaCert } from "./db/defaults.js";
2225
import { getEnv } from "./env.js";
2326
import { logger } from "./logger.js";
@@ -30,7 +33,7 @@ import { isSentrySaasUrl } from "./sentry-urls.js";
3033
* option instead.
3134
*/
3235
const setDefaultCACertificates: ((certs: string[]) => void) | undefined = (
33-
require("node:tls") as {
36+
_require("node:tls") as {
3437
setDefaultCACertificates?: (certs: string[]) => void;
3538
}
3639
).setDefaultCACertificates;

src/lib/db/index.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,12 @@
55
*/
66

77
import { chmodSync, mkdirSync } from "node:fs";
8+
import { createRequire } from "node:module";
89
import { join } from "node:path";
910
import { getEnv } from "../env.js";
11+
12+
const _require = createRequire(import.meta.url);
13+
1014
import { migrateFromJson } from "./migration.js";
1115
import { initSchema, runMigrations } from "./schema.js";
1216
import { Database } from "./sqlite.js";
@@ -30,7 +34,7 @@ let rawDb: Database | null = null;
3034
let dbOpenedPath: string | null = null;
3135

3236
export function getConfigDir(): string {
33-
const { homedir } = require("node:os");
37+
const { homedir } = _require("node:os");
3438
return (
3539
getEnv()[CONFIG_DIR_ENV_VAR] || join(homedir(), DEFAULT_CONFIG_DIR_NAME)
3640
);
@@ -107,6 +111,7 @@ export function getDatabase(): Database {
107111
if (getEnv().SENTRY_CLI_NO_TELEMETRY === "1") {
108112
db = rawDb;
109113
} else {
114+
// bare require so esbuild resolves this at bundle time (breaks circular dep)
110115
const { createTracedDatabase } = require("../telemetry.js") as {
111116
createTracedDatabase: (d: Database) => Database;
112117
};

src/lib/db/migration.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,11 @@
33
*/
44

55
import { rmSync } from "node:fs";
6+
import { createRequire } from "node:module";
67
import { join } from "node:path";
8+
9+
const _require = createRequire(import.meta.url);
10+
711
import { logger } from "../logger.js";
812
import { getConfigDir } from "./index.js";
913
import type { Database } from "./sqlite.js";
@@ -34,14 +38,14 @@ function markMigrationCompleted(db: Database): void {
3438

3539
function oldConfigExists(): boolean {
3640
const configPath = join(getConfigDir(), OLD_CONFIG_FILENAME);
37-
const { existsSync } = require("node:fs");
41+
const { existsSync } = _require("node:fs");
3842
return existsSync(configPath);
3943
}
4044

4145
function readOldConfig(): OldConfig | null {
4246
const configPath = join(getConfigDir(), OLD_CONFIG_FILENAME);
4347
try {
44-
const { readFileSync } = require("node:fs");
48+
const { readFileSync } = _require("node:fs");
4549
const content = readFileSync(configPath, "utf-8");
4650
return JSON.parse(content);
4751
} catch {

src/lib/db/schema.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,14 @@
1111
* - Migration checks
1212
*/
1313

14+
import { createRequire } from "node:module";
1415
import { getEnv } from "../env.js";
1516
import { stringifyUnknown } from "../errors.js";
1617
import { logger } from "../logger.js";
1718
import type { Database } from "./sqlite.js";
1819

20+
const _require = createRequire(import.meta.url);
21+
1922
export const CURRENT_SCHEMA_VERSION = 16;
2023

2124
/** Environment variable to disable auto-repair */
@@ -671,6 +674,7 @@ export function tryRepairAndRetry<T>(
671674
let repairSucceeded = false;
672675
try {
673676
// Dynamic imports to avoid circular dependencies with db/index.js
677+
// bare require so esbuild resolves this at bundle time (breaks circular dep)
674678
const { getRawDatabase } = require("./index.js") as {
675679
getRawDatabase: () => Database;
676680
};

src/lib/db/sqlite.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,11 @@
88
* Uses `node:sqlite` (Node 22.15+ with `--experimental-sqlite` flag).
99
*/
1010

11+
import { createRequire } from "node:module";
1112
import { logger } from "../logger.js";
1213

14+
const _require = createRequire(import.meta.url);
15+
1316
const log = logger.withTag("sqlite");
1417

1518
/** Valid SQLite binding value. */
@@ -58,7 +61,7 @@ function wrapStatement(stmt: any): StatementWrapper {
5861
* Uses `node:sqlite` (Node 22.15+ with `--experimental-sqlite`).
5962
*/
6063
// biome-ignore lint/suspicious/noExplicitAny: driver types loaded lazily
61-
const SqliteImpl: any = require("node:sqlite").DatabaseSync;
64+
const SqliteImpl: any = _require("node:sqlite").DatabaseSync;
6265

6366
/**
6467
* SQLite database wrapper.

src/lib/init/ui/ink-ui.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,11 @@
4747
*/
4848

4949
import { openSync } from "node:fs";
50+
import { createRequire } from "node:module";
5051
import { ReadStream } from "node:tty";
52+
53+
const _require = createRequire(import.meta.url);
54+
5155
import { setTag } from "@sentry/node-core/light";
5256
import { CLI_VERSION } from "../../constants.js";
5357
import { stripAnsi } from "../../formatters/plain-detect.js";
@@ -235,7 +239,7 @@ export async function createInkUI(
235239
let isSea = false;
236240
try {
237241
// biome-ignore lint/suspicious/noExplicitAny: node:sea types not yet in @types/node
238-
const sea = require("node:sea") as any;
242+
const sea = _require("node:sea") as any;
239243
isSea = sea.isSea?.() === true;
240244
} catch {
241245
// node:sea not available (older Node or non-SEA context)
@@ -245,7 +249,7 @@ export async function createInkUI(
245249
// Extract the embedded sidecar to a temp file and import it.
246250
// The asset key matches what fossilize registered via --assets.
247251
// biome-ignore lint/suspicious/noExplicitAny: node:sea types not yet in @types/node
248-
const sea = require("node:sea") as any;
252+
const sea = _require("node:sea") as any;
249253
const { writeFileSync, mkdtempSync } = await import("node:fs");
250254
const { join } = await import("node:path");
251255
const { tmpdir } = await import("node:os");

src/lib/list-command.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,12 @@
1313
* buildOrgListCommand
1414
*/
1515

16+
import { createRequire } from "node:module";
1617
import type { Aliases, Command, CommandContext } from "@stricli/core";
1718
import type { SentryContext } from "../context.js";
19+
20+
const _require = createRequire(import.meta.url);
21+
1822
import { parseOrgProjectArg } from "./arg-parsing.js";
1923
import { buildCommand, numberParser } from "./command.js";
2024
import { disableOrgCache } from "./db/regions.js";
@@ -414,6 +418,7 @@ function getSubcommandsForRoute(routeName: string): Set<string> {
414418
_subcommandsByRoute = new Map();
415419

416420
try {
421+
// bare require so esbuild resolves this at bundle time (breaks circular dep)
417422
const { routes } = require("../app.js") as {
418423
routes: { getAllEntries: () => readonly RouteEntry[] };
419424
};

0 commit comments

Comments
 (0)