Skip to content

Commit 4f8797b

Browse files
authored
refactor(db): DRY up database layer with shared helpers and lint enforcement (#550)
## Summary DRY up the `src/lib/db/` layer by extracting shared helpers, removing duplicated SQL patterns, and adding GritQL lint rules to prevent regression. ## Changes ### New helpers in `src/lib/db/utils.ts` - **`getMetadata(db, keys)`** — batch read from metadata table (`SELECT ... WHERE key IN (...)`) - **`setMetadata(db, entries)`** — batch write in a single transaction - **`clearMetadata(db, keys)`** — batch delete (`DELETE ... WHERE key IN (...)`) - **`touchCacheEntry(table, keyColumn, keyValue)`** — shared `last_accessed` timestamp update ### Refactored DB modules - `install-info.ts` — 4 individual queries → 1 batch call per operation - `version-check.ts`, `release-channel.ts` — same metadata helper adoption - `dsn-cache.ts`, `project-cache.ts` — private `touchCacheEntry` → shared helper - `project-cache.ts` — deduplicated 4 get/set functions via `getByKey`/`setByKey` helpers - `project-aliases.ts` — manual `BEGIN`/`COMMIT`/`ROLLBACK` → `db.transaction()()` ### De-async synchronous functions Removed `async`/`Promise<>` from 27 DB functions that only do synchronous SQLite operations, plus ~370 `await` removals at call sites. Narrowed the biome `useAwait: "off"` override to only files with genuinely async operations. ### GritQL lint rules (3 new) - `no-raw-metadata-queries.grit` — enforces `getMetadata`/`setMetadata`/`clearMetadata` - `no-manual-transactions.grit` — enforces `db.transaction()()` - `no-inline-touch-cache.grit` — enforces shared `touchCacheEntry()` ### Documentation - AGENTS.md — expanded SQL Utilities section, replaced stale "Async Config Functions" with accurate "DB and Config Function Signatures"
1 parent 7da52e0 commit 4f8797b

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

75 files changed

+933
-844
lines changed

AGENTS.md

Lines changed: 52 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -368,27 +368,35 @@ if (result.success) {
368368

369369
### SQL Utilities
370370

371-
Use the `upsert()` helper from `src/lib/db/utils.ts` to reduce SQL boilerplate:
371+
Use helpers from `src/lib/db/utils.ts` to reduce SQL boilerplate:
372372

373373
```typescript
374-
import { upsert, runUpsert } from "../db/utils.js";
374+
import { upsert, runUpsert, getMetadata, setMetadata, clearMetadata, touchCacheEntry } from "../db/utils.js";
375375

376-
// Generate UPSERT statement
377-
const { sql, values } = upsert("table", { id: 1, name: "foo" }, ["id"]);
378-
db.query(sql).run(...values);
379-
380-
// Or use convenience wrapper
376+
// UPSERT (insert or update on conflict)
381377
runUpsert(db, "table", { id: 1, name: "foo" }, ["id"]);
382378

383-
// Exclude columns from update
379+
// With excludeFromUpdate (columns only set on INSERT)
384380
const { sql, values } = upsert(
385381
"users",
386382
{ id: 1, name: "Bob", created_at: now },
387383
["id"],
388384
{ excludeFromUpdate: ["created_at"] }
389385
);
386+
db.query(sql).run(...values);
387+
388+
// METADATA table helpers (for key-value config in the metadata table)
389+
const m = getMetadata(db, [KEY_A, KEY_B]); // SELECT ... WHERE key IN (...)
390+
setMetadata(db, { [KEY_A]: "val1", [KEY_B]: "val2" }); // Batch upsert in transaction
391+
clearMetadata(db, [KEY_A, KEY_B]); // DELETE ... WHERE key IN (...)
392+
393+
// Cache entry touch (update last_accessed timestamp)
394+
touchCacheEntry("dsn_cache", "directory", directoryPath);
390395
```
391396

397+
**Enforced by lint:** GritQL rules ban raw metadata queries, inline `last_accessed` UPDATEs,
398+
and manual `BEGIN`/`COMMIT`/`ROLLBACK` transactions outside `utils.ts` and `migration.ts`.
399+
392400
### Error Handling
393401

394402
All CLI errors extend the `CliError` base class from `src/lib/errors.ts`:
@@ -460,16 +468,23 @@ entity-aware suggestions (e.g., "This looks like a span ID").
460468
- `trace/view.ts` — issue short ID → issue's trace redirect
461469
- `hex-id.ts` — entity-aware error hints in `validateHexId`/`validateSpanId`
462470
463-
### Async Config Functions
471+
### DB and Config Function Signatures
464472
465-
All config operations are async. Always await:
473+
Database operations in `src/lib/db/` are **synchronous** (SQLite is synchronous in Bun).
474+
Functions that only do SQLite work return values directly — no `async`/`await` needed:
466475
467476
```typescript
468-
const token = await getAuthToken();
469-
const isAuth = await isAuthenticated();
470-
await setAuthToken(token, expiresIn);
477+
const org = getDefaultOrganization(); // returns string | undefined
478+
const info = getUserInfo(); // returns UserInfo | undefined
479+
setDefaults("my-org", "my-project"); // returns void
480+
const token = getAuthToken(); // returns string | undefined
471481
```
472482
483+
Exceptions that ARE async (they do file I/O or network calls):
484+
- `clearAuth()` — awaits cache directory cleanup
485+
- `getCachedDetection()`, `getCachedProjectRoot()`, `setCachedProjectRoot()` — await `stat()` for mtime validation
486+
- `refreshToken()`, `performTokenRefresh()` — await HTTP calls
487+
473488
### Imports
474489
475490
- Use `.js` extension for local imports (ESM requirement)
@@ -832,6 +847,30 @@ mock.module("./some-module", () => ({
832847
<!-- lore:019cb950-9b7b-731a-9832-b7f6cfb6a6a2 -->
833848
* **Self-hosted OAuth device flow requires Sentry 26.1.0+ and SENTRY\_CLIENT\_ID**: Self-hosted OAuth device flow requires Sentry 26.1.0+ and both \`SENTRY\_URL\` and \`SENTRY\_CLIENT\_ID\` env vars. Users must create a public OAuth app in Settings → Developer Settings. The client ID is NOT optional for self-hosted. Fallback for older instances: \`sentry auth login --token\`. \`getSentryUrl()\` and \`getClientId()\` in \`src/lib/oauth.ts\` read lazily (not at module load) so URL parsing from arguments can set \`SENTRY\_URL\` after import.
834849
850+
<!-- lore:019ce2be-39f1-7ad9-a4c5-4506b62f689c -->
851+
* **api-client.ts split into domain modules under src/lib/api/**: The original monolithic \`src/lib/api-client.ts\` (1,977 lines) was split into 12 focused domain modules under \`src/lib/api/\`: infrastructure.ts (shared helpers, types, raw requests), organizations.ts, projects.ts, teams.ts, repositories.ts, issues.ts, events.ts, traces.ts, logs.ts, seer.ts, trials.ts, users.ts. The original \`api-client.ts\` was converted to a ~100-line barrel re-export file preserving all existing import paths. The \`biome.jsonc\` override for \`noBarrelFile\` already includes \`api-client.ts\`. When adding new API functions, place them in the appropriate domain module under \`src/lib/api/\`, not in the barrel file.
852+
853+
<!-- lore:019d0b69-1430-74f0-8e9a-426a5c7b321d -->
854+
* **Bun compiled binary sourcemap options and size impact**: Binary build (\`script/build.ts\`) uses two steps: (1) \`Bun.build()\` produces \`dist-bin/bin.js\` + \`.map\` with \`sourcemap: "linked"\` and minification. (2) \`Bun.build()\` with \`compile: true\` produces native binary — no sourcemap embedded. Bun's compiled binaries use \`/$bunfs/root/bin.js\` as the virtual path in stack traces. Sourcemap upload must use \`--url-prefix '/$bunfs/root/'\` so Sentry can match frames. The upload runs \`sentry-cli sourcemaps inject dist-bin/\` first (adds debug IDs), then uploads both JS and map. Bun's compile step strips comments (including \`//# debugId=\`), but debug ID matching still works via the injected runtime snippet + URL prefix matching. Size: +0.04 MB gzipped vs +2.30 MB for inline sourcemaps. Without \`SENTRY\_AUTH\_TOKEN\`, upload is skipped gracefully.
855+
856+
<!-- lore:019cb8ea-c6f0-75d8-bda7-e32b4e217f92 -->
857+
* **CLI telemetry DSN is public write-only — safe to embed in install script**: The CLI's Sentry DSN (\`SENTRY\_CLI\_DSN\` in \`src/lib/constants.ts\`) is a public write-only ingest key already baked into every binary. Safe to hardcode in install scripts. Opt-out: \`SENTRY\_CLI\_NO\_TELEMETRY=1\`.
858+
859+
<!-- lore:019c978a-18b5-7a0d-a55f-b72f7789bdac -->
860+
* **cli.sentry.dev is served from gh-pages branch via GitHub Pages**: \`cli.sentry.dev\` is served from gh-pages branch via GitHub Pages. Craft's gh-pages target runs \`git rm -r -f .\` before extracting docs — persist extra files via \`postReleaseCommand\` in \`.craft.yml\`. Install script supports \`--channel nightly\`, downloading from the \`nightly\` release tag directly. version.json is only used by upgrade/version-check flow.
861+
862+
<!-- lore:019cbe93-19b8-7776-9705-20bbde226599 -->
863+
* **Nightly delta upgrade buildNightlyPatchGraph fetches ALL patch tags — O(N) HTTP calls**: Delta upgrade in \`src/lib/delta-upgrade.ts\` supports stable (GitHub Releases) and nightly (GHCR) channels. \`filterAndSortChainTags\` filters \`patch-\*\` tags by version range using \`Bun.semver.order()\`. GHCR uses \`fetchWithRetry\` (10s timeout + 1 retry; blobs 30s) with optional \`signal?: AbortSignal\` combined via \`AbortSignal.any()\`. \`isExternalAbort(error, signal)\` skips retries for external aborts — critical for background prefetch. Patches cached to \`~/.sentry/patch-cache/\` (file-based, 7-day TTL). \`loadCachedChain\` stitches patches for multi-hop offline upgrades.
864+
865+
<!-- lore:2c3eb7ab-1341-4392-89fd-d81095cfe9c4 -->
866+
* **npm bundle requires Node.js >= 22 due to node:sqlite polyfill**: The npm package (dist/bin.cjs) requires Node.js >= 22 because the bun:sqlite polyfill uses \`node:sqlite\`. A runtime version guard in the esbuild banner catches this early. When writing esbuild banner strings in TS template literals, double-escape: \`\\\\\\\n\` in TS → \`\\\n\` in output → newline at runtime. Single \`\\\n\` produces a literal newline inside a JS string, causing SyntaxError.
867+
868+
<!-- lore:019c972c-9f0f-75cd-9e24-9bdbb1ac03d6 -->
869+
* **Numeric issue ID resolution returns org:undefined despite API success**: Numeric issue ID resolution in \`resolveNumericIssue()\`: (1) try DSN/env/config for org, (2) if found use \`getIssueInOrg(org, id)\` with region routing, (3) else fall back to unscoped \`getIssue(id)\`, (4) extract org from \`issue.permalink\` via \`parseSentryUrl\` as final fallback. \`parseSentryUrl\` handles path-based (\`/organizations/{org}/...\`) and subdomain-style URLs. \`matchSubdomainOrg()\` filters region subdomains by requiring slug length > 2. Self-hosted uses path-based only.
870+
871+
<!-- lore:019ce0bb-f35d-7380-b661-8dc56f9938cf -->
872+
* **Seer trial prompt uses middleware layering in bin.ts error handling chain**: Error recovery middlewares in \`bin.ts\` are layered: \`main() → executeWithAutoAuth() → executeWithSeerTrialPrompt() → runCommand()\`. Seer trial prompts (for \`no\_budget\`/\`not\_enabled\`) caught by inner wrapper; auth errors bubble to outer. Auth retry goes through full chain. Trial API: \`GET /api/0/customers/{org}/\`\`productTrials\[]\` (prefer \`seerUsers\`, fallback \`seerAutofix\`). Start: \`PUT /api/0/customers/{org}/product-trial/\`. SaaS-only; self-hosted 404s gracefully. \`ai\_disabled\` excluded. \`startSeerTrial\` accepts \`category\` from trial object — don't hardcode.
873+
835874
<!-- lore:019d0b16-977c-7e49-b06d-523b7782692f -->
836875
* **Sentry CLI fuzzy matching coverage map across subsystems**: Fuzzy matching exists in: (1) Stricli built-in Damerau-Levenshtein for subcommand/flag typos within known route groups, (2) custom \`fuzzyMatch()\` in \`complete.ts\` for dynamic tab-completion using Levenshtein+prefix+contains scoring, (3) custom \`levenshtein()\` in \`platforms.ts\` for platform name suggestions, (4) plural alias detection in \`app.ts\`, (5) \`resolveCommandPath()\` in \`introspect.ts\` uses \`fuzzyMatch()\` from \`fuzzy.ts\` for top-level and subcommand typos — covering both \`sentry \<typo>\` and \`sentry help \<typo>\`. Static shell tab-completion uses shell-native prefix matching (compgen/\`\_describe\`/fish \`-a\`).
837876

biome.jsonc

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,10 @@
33
"extends": ["ultracite/core"],
44
"plugins": [
55
"./lint-rules/no-stdout-write-in-commands.grit",
6-
"./lint-rules/no-process-stdout-in-commands.grit"
6+
"./lint-rules/no-process-stdout-in-commands.grit",
7+
"./lint-rules/no-raw-metadata-queries.grit",
8+
"./lint-rules/no-manual-transactions.grit",
9+
"./lint-rules/no-inline-touch-cache.grit"
710
],
811
"files": {
912
"includes": ["!docs", "!test/init-eval/templates"]
@@ -48,8 +51,13 @@
4851
}
4952
},
5053
{
51-
// SQLite operations are sync but we keep async signatures for API compatibility
52-
"includes": ["src/lib/db/**/*.ts"],
54+
// A few DB modules still have genuinely async functions (stat() for mtime validation,
55+
// clearResponseCache for auth cleanup). The useAwait rule is off for these specific files.
56+
"includes": [
57+
"src/lib/db/dsn-cache.ts",
58+
"src/lib/db/project-root-cache.ts",
59+
"src/lib/db/auth.ts"
60+
],
5361
"linter": {
5462
"rules": {
5563
"suspicious": {
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
file($name, $body) where {
2+
$name <: not r".*utils\.ts$",
3+
$body <: contains `$db.query($sql).run($args)` as $match where {
4+
$sql <: r".*UPDATE .* SET last_accessed.*"
5+
},
6+
register_diagnostic(span=$match, message="Use touchCacheEntry() from db/utils.js instead of inline last_accessed UPDATE.")
7+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
file($name, $body) where {
2+
$name <: not r".*migration\.ts$",
3+
$name <: not r".*node-polyfills\.ts$",
4+
$body <: contains `$db.exec($sql)` as $match where {
5+
$sql <: or {
6+
r".*BEGIN.*",
7+
r".*COMMIT.*",
8+
r".*ROLLBACK.*"
9+
}
10+
},
11+
register_diagnostic(span=$match, message="Use db.transaction()() instead of manual BEGIN/COMMIT/ROLLBACK.")
12+
}
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
file($name, $body) where {
2+
$name <: not r".*utils\.ts$",
3+
$name <: not r".*migration\.ts$",
4+
$body <: contains or {
5+
`$db.query($sql)` as $match where {
6+
$sql <: or {
7+
r".*SELECT value FROM metadata.*",
8+
r".*DELETE FROM metadata.*"
9+
}
10+
},
11+
`runUpsert($db, "metadata", $data, $keys)` as $match
12+
},
13+
register_diagnostic(span=$match, message="Use getMetadata/setMetadata/clearMetadata from db/utils.js instead of raw metadata queries.")
14+
}

src/bin.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ async function runCompletion(completionArgs: string[]): Promise<void> {
3939
// Disable telemetry so db/index.ts skips the @sentry/node-core lazy-require (~280ms)
4040
process.env.SENTRY_CLI_NO_TELEMETRY = "1";
4141
const { handleComplete } = await import("./lib/complete.js");
42-
await handleComplete(completionArgs);
42+
handleComplete(completionArgs);
4343
}
4444

4545
/**

src/commands/auth/login.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ export const loginCommand = buildCommand({
139139
output: { human: formatLoginResult },
140140
async *func(this: SentryContext, flags: LoginFlags) {
141141
// Check if already authenticated and handle re-authentication
142-
if (await isAuthenticated()) {
142+
if (isAuthenticated()) {
143143
const shouldProceed = await handleExistingAuth(flags.force);
144144
if (!shouldProceed) {
145145
return;

src/commands/auth/logout.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ export const logoutCommand = buildCommand({
3838
flags: {},
3939
},
4040
async *func(this: SentryContext) {
41-
if (!(await isAuthenticated())) {
41+
if (!isAuthenticated()) {
4242
return yield new CommandOutput({
4343
loggedOut: false,
4444
message: "Not currently authenticated.",

src/commands/auth/status.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -104,9 +104,9 @@ function collectTokenInfo(
104104
/**
105105
* Collect default org/project into the data structure.
106106
*/
107-
async function collectDefaults(): Promise<AuthStatusData["defaults"]> {
108-
const org = await getDefaultOrganization();
109-
const project = await getDefaultProject();
107+
function collectDefaults(): AuthStatusData["defaults"] {
108+
const org = getDefaultOrganization();
109+
const project = getDefaultProject();
110110

111111
if (!(org || project)) {
112112
return;
@@ -160,7 +160,7 @@ export const statusCommand = buildCommand({
160160
applyFreshFlag(flags);
161161

162162
const auth = getAuthConfig();
163-
const authenticated = await isAuthenticated();
163+
const authenticated = isAuthenticated();
164164
const fromEnv = auth ? isEnvSource(auth.source) : false;
165165

166166
if (!authenticated) {
@@ -186,7 +186,7 @@ export const statusCommand = buildCommand({
186186
configPath: fromEnv ? undefined : getDbPath(),
187187
user,
188188
token: collectTokenInfo(auth, flags["show-token"]),
189-
defaults: await collectDefaults(),
189+
defaults: collectDefaults(),
190190
verification: await verifyCredentials(),
191191
};
192192

src/commands/auth/whoami.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ export const whoamiCommand = buildCommand({
4646
async *func(this: SentryContext, flags: WhoamiFlags) {
4747
applyFreshFlag(flags);
4848

49-
if (!(await isAuthenticated())) {
49+
if (!isAuthenticated()) {
5050
throw new AuthError("not_authenticated");
5151
}
5252

0 commit comments

Comments
 (0)