Skip to content

Commit 32ffe32

Browse files
Haiderclaude
andcommitted
fix(dbt-tools): bubble real errors and redact SQL across all execDbt* paths
Pulls #943, #944, and #945 into this PR — each was tagged as out-of-scope in the earlier rounds but they share so much of the same fix surface (extractDbtError / fallbackExitMessage already in place) that fixing them in-PR is cleaner than spawning three follow-up PRs. Closes #943 — execDbtCompile / execDbtCompileInline used to swallow the real dbt error with `try { … } catch { lines = [] }`. They now capture the run error, skip the success-only tiers when the primary run failed (same anti-spurious-data reasoning as execDbtShow), and throw via a new shared `bubbleDbtError(label, primary, plain)` helper that combines extractDbtError + fallbackExitMessage with the same SQL-safety, ANSI-stripping, and category-prefix-dedup guarantees. Closes #944 — execDbtShow Tier 1 used to assign a truthy non-array `data.preview` straight into `rows`, returning `{ data: {} }` to callers and crashing downstream `.map` / `.length`. The else-branch now guards with `Array.isArray(preview)` and emits empty rows for any unexpected shape (object, number, etc.). Closes #945 — execDbtCompileInline's final throw used `e.message` directly, which is Node's "Command failed: <dbt-path> compile --inline '<entire user SQL>' …" — leaking PII / secrets templated into the inline query. The new bubbleDbtError helper routes through fallbackExitMessage, which redacts the command line for both exit-code and signal/timeout rejections. Tests: 41 pass / 0 fail (was 33). New coverage: - execDbtShow non-array preview shape (object + numeric) - execDbtCompile error bubbling (real stderr, structured JSON event, category-prefix dedup) - execDbtCompileInline error bubbling - execDbtCompileInline SQL-redaction canaries for both exit-code and signal-kill failure modes Refs #932 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 44d9215 commit 32ffe32

2 files changed

Lines changed: 205 additions & 42 deletions

File tree

packages/dbt-tools/src/dbt-cli.ts

Lines changed: 92 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -285,17 +285,20 @@ export async function execDbtShow(sql: string, limit?: number) {
285285
(previewLine as any).result?.preview ??
286286
(previewLine as any).result?.rows
287287

288-
// Guard JSON.parse — fall through to Tier 2 on malformed strings
288+
// The previewLine match upstream only checked for truthiness, so a
289+
// future dbt version emitting `data.preview = {}` or `= 42` would
290+
// flow into `rows` unchecked and the downstream `data: rows` field
291+
// would crash callers that do `.map` / `.length`. Guard explicitly
292+
// for the three shapes we accept (parsed JSON array, native array,
293+
// malformed) and emit an empty result for anything else.
289294
let rows: Record<string, unknown>[]
290295
if (typeof preview === "string") {
291296
const parsed = safeJsonParse(preview)
292-
if (Array.isArray(parsed)) {
293-
rows = parsed
294-
} else {
295-
rows = [] // Malformed — will fall through below
296-
}
297-
} else {
297+
rows = Array.isArray(parsed) ? parsed : []
298+
} else if (Array.isArray(preview)) {
298299
rows = preview
300+
} else {
301+
rows = []
299302
}
300303

301304
// Return the result — even if empty. An empty preview means the query returned
@@ -464,52 +467,88 @@ function fallbackExitMessage(primary?: ExecFileError, plain?: ExecFileError): st
464467
return "dbt failed (no exit code reported)"
465468
}
466469

470+
/**
471+
* Build a user-facing error message from a failed `dbt <cmd>` invocation.
472+
*
473+
* Used by every execDbt* function so all three share the same error UX:
474+
* - structured `level: "error"` event from JSON logs > primary stderr >
475+
* plain-text stderr > redacted exit-status fallback
476+
* - ANSI escapes stripped
477+
* - inline SQL / command-line redacted from any err.message-derived path
478+
* - no doubled prefix when dbt's own category prefix is already present
479+
*/
480+
function bubbleDbtError(label: string, primary?: ExecFileError, plain?: ExecFileError): string {
481+
const errorLogLines = primary?.stdout ? parseJsonLines(primary.stdout.toString()) : []
482+
const real = extractDbtError(errorLogLines, primary, plain)
483+
if (real) {
484+
const hasDbtCategoryPrefix = /^(Compilation|Database|Runtime|Parsing|Validation|Dependency)\s+Error\b/.test(
485+
real,
486+
)
487+
return hasDbtCategoryPrefix ? real : `${label}: ${real}`
488+
}
489+
return `${label}: ${fallbackExitMessage(primary, plain) ?? "unknown error"}`
490+
}
491+
467492
/**
468493
* Compile a model via `dbt compile --select <model>` and return compiled SQL.
469494
*/
470495
export async function execDbtCompile(model: string): Promise<{ sql: string }> {
471496
const args = ["compile", "--select", model, "--output", "json", "--log-format", "json"]
472497

473-
let lines: Record<string, unknown>[]
498+
let lines: Record<string, unknown>[] = []
499+
let primaryRunError: ExecFileError | undefined
474500
try {
475501
const { stdout } = await run(args)
476502
lines = parseJsonLines(stdout)
477-
} catch {
478-
lines = []
503+
} catch (e) {
504+
primaryRunError = toExecFileError(e)
479505
}
480506

481-
// --- Tier 1: known field paths ---
482-
const sql = findCompiledSql(lines)
483-
if (sql) return { sql }
507+
// Skip success-only tiers when the primary run failed (same anti-spurious-
508+
// data reasoning as execDbtShow).
509+
if (!primaryRunError) {
510+
// --- Tier 1: known field paths ---
511+
const sql = findCompiledSql(lines)
512+
if (sql) return { sql }
484513

485-
// --- Tier 2: heuristic deep scan ---
486-
for (const line of lines) {
487-
const found = deepFind(line, (val) => looksLikeSql(val))
488-
if (found) return { sql: found as string }
514+
// --- Tier 2: heuristic deep scan ---
515+
for (const line of lines) {
516+
const found = deepFind(line, (val) => looksLikeSql(val))
517+
if (found) return { sql: found as string }
518+
}
489519
}
490520

491-
// --- Tier 3: read compiled SQL from manifest.json (more reliable than stdout) ---
492-
// dbt compile writes compiled_code to target/manifest.json even when stdout is logs.
493-
// We run compile without JSON flags so it writes to manifest, then read the artifact.
521+
// --- Manifest fallback ---
522+
// dbt compile writes compiled_code to target/manifest.json even when stdout
523+
// is just logs. Re-run plain (no JSON flags) so the artifact is fresh, then
524+
// read it back. We tolerate a failure here (a prior successful compile may
525+
// have left a usable manifest) but capture the error for later bubbling.
526+
let manifestRunError: ExecFileError | undefined
494527
try {
495528
await run(["compile", "--select", model])
496-
} catch {
497-
// Compile may fail (e.g., dbt not found, project errors) — continue to manifest check
498-
// since a prior successful compile may have left a usable manifest
529+
} catch (e) {
530+
manifestRunError = toExecFileError(e)
499531
}
500532
const fromManifest = readCompiledFromManifest(model)
501533
if (fromManifest) return { sql: fromManifest }
502534

503-
// Last resort: return stdout (may contain logs mixed with SQL)
535+
// --- Last resort: plain compile, return raw stdout ---
536+
let plainRunError: ExecFileError | undefined
504537
try {
505538
const { stdout: plainOut } = await run(["compile", "--select", model])
506539
return { sql: plainOut.trim() }
507540
} catch (e) {
508-
throw new Error(
509-
`Could not compile model '${model}' in any format (JSON, heuristic, or manifest). ` +
510-
`Last error: ${e instanceof Error ? e.message : String(e)}`,
511-
)
541+
plainRunError = toExecFileError(e)
542+
}
543+
544+
// If dbt actually failed at any tier, surface the real dbt error via the
545+
// shared helper so the message is SQL-safe, ANSI-stripped, and consistent
546+
// with execDbtShow's error UX.
547+
if (primaryRunError || plainRunError || manifestRunError) {
548+
throw new Error(bubbleDbtError("dbt compile failed", primaryRunError, plainRunError ?? manifestRunError))
512549
}
550+
551+
throw new Error(`Could not compile model '${model}' in any format (JSON, heuristic, or manifest).`)
513552
}
514553

515554
/**
@@ -521,34 +560,45 @@ export async function execDbtCompileInline(
521560
): Promise<{ sql: string }> {
522561
const args = ["compile", "--inline", sql, "--output", "json", "--log-format", "json"]
523562

524-
let lines: Record<string, unknown>[]
563+
let lines: Record<string, unknown>[] = []
564+
let primaryRunError: ExecFileError | undefined
525565
try {
526566
const { stdout } = await run(args)
527567
lines = parseJsonLines(stdout)
528-
} catch {
529-
lines = []
568+
} catch (e) {
569+
primaryRunError = toExecFileError(e)
530570
}
531571

532-
// --- Tier 1: known field paths ---
533-
const compiled = findCompiledSql(lines)
534-
if (compiled) return { sql: compiled }
572+
// Skip success-only tiers when the primary run failed.
573+
if (!primaryRunError) {
574+
// --- Tier 1: known field paths ---
575+
const compiled = findCompiledSql(lines)
576+
if (compiled) return { sql: compiled }
535577

536-
// --- Tier 2: heuristic deep scan ---
537-
for (const line of lines) {
538-
const found = deepFind(line, (val) => looksLikeSql(val))
539-
if (found) return { sql: found as string }
578+
// --- Tier 2: heuristic deep scan ---
579+
for (const line of lines) {
580+
const found = deepFind(line, (val) => looksLikeSql(val))
581+
if (found) return { sql: found as string }
582+
}
540583
}
541584

542585
// --- Tier 3: plain text fallback ---
586+
let plainRunError: ExecFileError | undefined
543587
try {
544588
const { stdout: plainOut } = await run(["compile", "--inline", sql])
545589
return { sql: plainOut.trim() }
546590
} catch (e) {
547-
throw new Error(
548-
`Could not compile inline SQL in any format (JSON, heuristic, or plain text). ` +
549-
`Last error: ${e instanceof Error ? e.message : String(e)}`,
550-
)
591+
plainRunError = toExecFileError(e)
551592
}
593+
594+
// bubbleDbtError redacts inline SQL from any err.message fallback — critical
595+
// here because we're spawning `dbt compile --inline <user SQL>` and Node's
596+
// rejection message embeds the full command line (with the SQL) verbatim.
597+
if (primaryRunError || plainRunError) {
598+
throw new Error(bubbleDbtError("dbt compile inline failed", primaryRunError, plainRunError))
599+
}
600+
601+
throw new Error("Could not compile inline SQL in any format (JSON, heuristic, or plain text).")
552602
}
553603

554604
/** Shared: extract compiled SQL from known dbt JSON output formats. */

packages/dbt-tools/test/dbt-cli.test.ts

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -421,6 +421,33 @@ describe("execDbtShow", () => {
421421

422422
await expect(execDbtShow("SELECT 1")).rejects.toThrow(/Database Error: connection refused/)
423423
})
424+
425+
test("does NOT return malformed data when data.preview is a truthy non-array", async () => {
426+
// Regression for #944: the previewLine match only checks truthiness, so a
427+
// future dbt version emitting `data.preview = {}` would flow into `rows`
428+
// and the downstream `data: rows` field would crash callers that do
429+
// `.map` / `.length`. Treat unexpected shapes as empty rows instead.
430+
const jsonLines = [JSON.stringify({ data: { preview: {} } })].join("\n")
431+
mockExecFile.mockImplementation((_cmd: string, _args: string[], _opts: any, cb: Function) => {
432+
cb(null, jsonLines, "")
433+
})
434+
435+
const result = await execDbtShow("SELECT 1")
436+
expect(Array.isArray(result.data)).toBe(true)
437+
expect(result.data).toEqual([])
438+
expect(result.columnNames).toEqual([])
439+
})
440+
441+
test("also treats numeric data.preview as empty (defence-in-depth)", async () => {
442+
const jsonLines = [JSON.stringify({ data: { preview: 42 } })].join("\n")
443+
mockExecFile.mockImplementation((_cmd: string, _args: string[], _opts: any, cb: Function) => {
444+
cb(null, jsonLines, "")
445+
})
446+
447+
const result = await execDbtShow("SELECT 1")
448+
expect(Array.isArray(result.data)).toBe(true)
449+
expect(result.data).toEqual([])
450+
})
424451
})
425452

426453
// ---------------------------------------------------------------------------
@@ -513,6 +540,45 @@ describe("execDbtCompile", () => {
513540
const result = await execDbtCompile("my_model")
514541
expect(result.sql).toBe("SELECT * FROM final_model")
515542
})
543+
544+
// --- Real dbt error bubbling (#943) ---
545+
546+
test("surfaces real dbt stderr when compile fails", async () => {
547+
// Pre-fix: catch { lines = [] } swallowed the real error and the final
548+
// throw embedded Node's generic "Command failed: ..." message.
549+
mockExecFile.mockImplementation((_cmd: string, _args: string[], _opts: any, cb: Function) => {
550+
const err: any = new Error("Command failed: dbt compile --select foo ...")
551+
err.code = 1
552+
cb(err, "", "Compilation Error: Model 'foo' depends on a node named 'bar' which was not found")
553+
})
554+
555+
await expect(execDbtCompile("foo")).rejects.toThrow(/Compilation Error.*Model 'foo'.*depends on a node named 'bar'/)
556+
})
557+
558+
test("prefers structured JSON error event over raw stderr", async () => {
559+
const errorLog = JSON.stringify({
560+
info: { level: "error", msg: "Database Error: relation does not exist" },
561+
})
562+
mockExecFile.mockImplementation((_cmd: string, _args: string[], _opts: any, cb: Function) => {
563+
const err: any = new Error("Command failed")
564+
err.code = 1
565+
cb(err, errorLog, "exit status 1")
566+
})
567+
568+
await expect(execDbtCompile("foo")).rejects.toThrow(/Database Error: relation does not exist/)
569+
})
570+
571+
test("does not double the 'dbt compile failed:' prefix on dbt category errors", async () => {
572+
mockExecFile.mockImplementation((_cmd: string, _args: string[], _opts: any, cb: Function) => {
573+
const err: any = new Error("Command failed")
574+
err.code = 1
575+
cb(err, "", "Compilation Error: model not found")
576+
})
577+
578+
const caught = (await execDbtCompile("foo").catch((e) => e)) as Error
579+
expect(caught.message).toBe("Compilation Error: model not found")
580+
expect(caught.message).not.toMatch(/dbt compile failed: Compilation Error/)
581+
})
516582
})
517583

518584
// ---------------------------------------------------------------------------
@@ -533,6 +599,53 @@ describe("execDbtCompileInline", () => {
533599
const result = await execDbtCompileInline("SELECT * FROM {{ ref('customers') }}")
534600
expect(result.sql).toBe("SELECT id, name FROM raw.customers")
535601
})
602+
603+
// --- Real dbt error bubbling (#943) + SQL redaction (#945) ---
604+
605+
test("surfaces real dbt error when inline compile fails", async () => {
606+
mockExecFile.mockImplementation((_cmd: string, _args: string[], _opts: any, cb: Function) => {
607+
const err: any = new Error("Command failed: dbt compile --inline 'SELECT 1' ...")
608+
err.code = 1
609+
cb(err, "", "Compilation Error: Undefined macro 'unknown_macro'")
610+
})
611+
612+
await expect(execDbtCompileInline("SELECT 1")).rejects.toThrow(/Compilation Error.*Undefined macro/)
613+
})
614+
615+
test("does NOT embed inline SQL into the surfaced error (no Command-failed leak)", async () => {
616+
// Regression for #945: pre-fix the throw used `e.message` directly,
617+
// which is Node's "Command failed: <dbt-path> compile --inline '<entire
618+
// SQL>' …" format — leaking the user's full query into logs and UI.
619+
const sensitiveSql = "SELECT 'PII_TOKEN_compile_xyz' AS secret FROM users WHERE ssn = '111-22-3333'"
620+
mockExecFile.mockImplementation((_cmd: string, _args: string[], _opts: any, cb: Function) => {
621+
const err: any = new Error(
622+
`Command failed: /usr/local/bin/dbt compile --inline ${sensitiveSql} --output json --log-format json`,
623+
)
624+
err.code = 2
625+
cb(err, "", "")
626+
})
627+
628+
const caught = (await execDbtCompileInline(sensitiveSql).catch((e) => e)) as Error
629+
expect(caught.message).not.toContain("PII_TOKEN_compile_xyz")
630+
expect(caught.message).not.toContain("111-22-3333")
631+
expect(caught.message).toMatch(/dbt compile inline failed: dbt exited with status 2/)
632+
})
633+
634+
test("does NOT leak SQL when inline compile is killed by signal / timeout", async () => {
635+
const sensitiveSql = "SELECT 'compile_leak_canary' FROM secrets"
636+
mockExecFile.mockImplementation((_cmd: string, _args: string[], _opts: any, cb: Function) => {
637+
const err: any = new Error(
638+
`Command failed: /usr/local/bin/dbt compile --inline ${sensitiveSql} --output json --log-format json`,
639+
)
640+
err.killed = true
641+
err.signal = "SIGTERM"
642+
cb(err, "", "")
643+
})
644+
645+
const caught = (await execDbtCompileInline(sensitiveSql).catch((e) => e)) as Error
646+
expect(caught.message).not.toContain("compile_leak_canary")
647+
expect(caught.message).toMatch(/dbt compile inline failed: dbt killed by signal SIGTERM/)
648+
})
536649
})
537650

538651
// ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)