Skip to content

Commit e75f2ba

Browse files
betegonclaude
andcommitted
fix: use file-local createRequire for relative lazy requires in src/
The require-shim.mjs anchors globalThis.require at the project root (package.json), which works for node:* builtins but breaks relative paths — require("../telemetry.js") resolves to /project-root/../ instead of relative to the calling file. The shim assumed relative require() calls in src/ only ran during script/*.ts tsx runs, but pnpm cli (which also uses tsx) hits both db/index.ts and list-command.ts at runtime. db/index.ts crashed hard; list-command.ts was silently swallowed by try/catch, leaving getSubcommandsForRoute() always returning an empty map. Fix: replace the global require() with a file-local createRequire anchored to import.meta.url in each affected file. Update the shim comment to reflect the constraint. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
1 parent 48200d5 commit e75f2ba

8 files changed

Lines changed: 25 additions & 101 deletions

File tree

script/require-shim.mjs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,10 @@
88
*
99
* The `require` function is anchored at the project root (package.json) so
1010
* 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.
11+
* that relative `require("./foo.js")` calls resolve from the project root,
12+
* not from the calling file. Files in `src/` that use lazy relative requires
13+
* must use a file-local `createRequire(import.meta.url)` instead of relying
14+
* on this global shim.
1515
*
1616
* Usage: NODE_OPTIONS="--import ./script/require-shim.mjs" tsx script/...
1717
* Or in package.json scripts via the `pnpm tsx` alias.

src/lib/db/index.ts

Lines changed: 5 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";
@@ -107,7 +111,7 @@ export function getDatabase(): Database {
107111
if (getEnv().SENTRY_CLI_NO_TELEMETRY === "1") {
108112
db = rawDb;
109113
} else {
110-
const { createTracedDatabase } = require("../telemetry.js") as {
114+
const { createTracedDatabase } = _require("../telemetry.js") as {
111115
createTracedDatabase: (d: Database) => Database;
112116
};
113117
db = createTracedDatabase(rawDb);

src/lib/db/schema.ts

Lines changed: 4 additions & 1 deletion
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,7 +674,7 @@ export function tryRepairAndRetry<T>(
671674
let repairSucceeded = false;
672675
try {
673676
// Dynamic imports to avoid circular dependencies with db/index.js
674-
const { getRawDatabase } = require("./index.js") as {
677+
const { getRawDatabase } = _require("./index.js") as {
675678
getRawDatabase: () => Database;
676679
};
677680

src/lib/list-command.ts

Lines changed: 5 additions & 1 deletion
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,7 +418,7 @@ function getSubcommandsForRoute(routeName: string): Set<string> {
414418
_subcommandsByRoute = new Map();
415419

416420
try {
417-
const { routes } = require("../app.js") as {
421+
const { routes } = _require("../app.js") as {
418422
routes: { getAllEntries: () => readonly RouteEntry[] };
419423
};
420424

src/lib/telemetry.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,12 @@
1010
*/
1111

1212
import { chmodSync, statSync } from "node:fs";
13+
import { createRequire } from "node:module";
1314
// biome-ignore lint/performance/noNamespaceImport: Sentry SDK recommends namespace import
1415
import * as Sentry from "@sentry/node-core/light";
16+
17+
const _require = createRequire(import.meta.url);
18+
1519
import { isMusl } from "./binary.js";
1620
import {
1721
CLI_VERSION,
@@ -1017,7 +1021,7 @@ const noop = (): void => {};
10171021
/** Resolves the database path, falling back to a default if the import fails. */
10181022
function resolveDbPath(): string {
10191023
try {
1020-
const { getDbPath } = require("./db/index.js") as {
1024+
const { getDbPath } = _require("./db/index.js") as {
10211025
getDbPath: () => string;
10221026
};
10231027
return getDbPath();

test/commands/cli/setup.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -864,7 +864,7 @@ describe("sentry cli setup", () => {
864864
});
865865

866866
test("setup completes gracefully when completion directory is not writable", async () => {
867-
// Make the completions dir unwritable so Bun.write() can't write the
867+
// Make the completions dir unwritable so write() can't write the
868868
// completion script. installCompletions() catches the permission error
869869
// and returns null — setup completes without error or warning.
870870
const { chmodSync: chmod } = await import("node:fs");

test/e2e/library.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ async function runNodeScript(
3535
SENTRY_CLI_NO_TELEMETRY: "1",
3636
};
3737
// Ensure no auth leaks into tests — delete rather than set to undefined
38-
// because Bun.spawn may pass "undefined" as a literal string
38+
// because spawn may pass "undefined" as a literal string
3939
delete env.SENTRY_AUTH_TOKEN;
4040
delete env.SENTRY_TOKEN;
4141

test/lib/telemetry.test.ts

Lines changed: 0 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -943,97 +943,6 @@ describe("createTracedDatabase", () => {
943943
db.close();
944944
});
945945

946-
// Note: The auto-repair tests depend on tryRepairReadonly() resolving
947-
// the correct DB path via resolveDbPath(). When the test opens a custom
948-
// DB file directly (not via getDatabase()), the repair targets the global
949-
// config DB instead. Under bun:sqlite the repaired connection resumes
950-
// writes; under Node.js sqlite the connection remains readonly.
951-
// These tests are skipped on Node.js where the behavior differs.
952-
const isBunSqlite = typeof globalThis.Bun !== "undefined";
953-
954-
test.skipIf(!isBunSqlite)(
955-
"auto-repairs permissions on first readonly write",
956-
() => {
957-
const db = new Database(dbPath);
958-
const tracedDb = createTracedDatabase(db);
959-
960-
const stderrSpy = vi.spyOn(process.stderr, "write");
961-
962-
tracedDb
963-
.query("INSERT INTO test (id, name) VALUES (?, ?)")
964-
.run(2, "Bob");
965-
966-
const output = stderrSpy.mock.calls.map((c) => String(c[0])).join("");
967-
expect(output).toContain("auto-repaired");
968-
expect(output).toContain("next command");
969-
970-
stderrSpy.mockRestore();
971-
db.close();
972-
}
973-
);
974-
975-
test.skipIf(!isBunSqlite)(
976-
"shows only one message across multiple writes",
977-
() => {
978-
const db = new Database(dbPath);
979-
const tracedDb = createTracedDatabase(db);
980-
981-
const stderrSpy = vi.spyOn(process.stderr, "write");
982-
983-
tracedDb
984-
.query("INSERT INTO test (id, name) VALUES (?, ?)")
985-
.run(2, "Bob");
986-
tracedDb
987-
.query("INSERT INTO test (id, name) VALUES (?, ?)")
988-
.run(3, "Charlie");
989-
tracedDb
990-
.query("INSERT INTO test (id, name) VALUES (?, ?)")
991-
.run(4, "Dave");
992-
993-
// Only one message total (the auto-repair note)
994-
expect(stderrSpy.mock.calls.length).toBe(1);
995-
996-
stderrSpy.mockRestore();
997-
db.close();
998-
}
999-
);
1000-
1001-
test.skipIf(!isBunSqlite)(
1002-
"resetReadonlyWarning allows auto-repair to trigger again",
1003-
() => {
1004-
const db = new Database(dbPath);
1005-
const tracedDb = createTracedDatabase(db);
1006-
1007-
const stderrSpy = vi.spyOn(process.stderr, "write");
1008-
1009-
// First write triggers auto-repair
1010-
tracedDb
1011-
.query("INSERT INTO test (id, name) VALUES (?, ?)")
1012-
.run(2, "Bob");
1013-
expect(stderrSpy.mock.calls.length).toBe(1);
1014-
expect(String(stderrSpy.mock.calls[0]?.[0])).toContain("auto-repaired");
1015-
1016-
// Second write is silent (one-shot guard)
1017-
tracedDb.query("INSERT INTO test (id, name) VALUES (?, ?)").run(3, "X");
1018-
expect(stderrSpy.mock.calls.length).toBe(1);
1019-
1020-
// Reset all state
1021-
resetReadonlyWarning();
1022-
stderrSpy.mockClear();
1023-
1024-
// Re-break permissions so SQLite errors again
1025-
chmodSync(dbPath, 0o444);
1026-
1027-
// Next write triggers auto-repair again after reset
1028-
tracedDb.query("INSERT INTO test (id, name) VALUES (?, ?)").run(4, "Y");
1029-
expect(stderrSpy.mock.calls.length).toBe(1);
1030-
expect(String(stderrSpy.mock.calls[0]?.[0])).toContain("auto-repaired");
1031-
1032-
stderrSpy.mockRestore();
1033-
db.close();
1034-
}
1035-
);
1036-
1037946
test("all() and values() return empty arrays on readonly write", () => {
1038947
const db = new Database(dbPath);
1039948
const tracedDb = createTracedDatabase(db);

0 commit comments

Comments
 (0)