Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 61 additions & 4 deletions packages/dbt-tools/src/dbt-cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -222,11 +222,16 @@ export async function execDbtShow(sql: string, limit?: number) {
if (limit !== undefined) args.push("--limit", String(limit))

let lines: Record<string, unknown>[]
// Capture the run() error so we can bubble the real dbt failure up if all
// parse tiers fail; the generic "Could not parse" alone misleads callers
// into treating structural project errors as transient.
let primaryRunError: ExecFileError | undefined
try {
const { stdout } = await run(args)
lines = parseJsonLines(stdout)
} catch {
lines = []
} catch (e) {
primaryRunError = e as ExecFileError
lines = parseJsonLines(primaryRunError.stdout ?? "")
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
}

// --- Tier 1: known field paths ---
Expand Down Expand Up @@ -281,6 +286,7 @@ export async function execDbtShow(sql: string, limit?: number) {
}

// --- Tier 3: plain text fallback (ASCII table) ---
let plainRunError: ExecFileError | undefined
try {
const plainArgs = ["show", "--inline", sql]
if (limit !== undefined) plainArgs.push("--limit", String(limit))
Expand All @@ -295,8 +301,15 @@ export async function execDbtShow(sql: string, limit?: number) {
compiledSql: sql,
}
}
} catch {
// Plain text dbt show also failed — fall through to error below
} catch (e) {
plainRunError = e as ExecFileError
}

// If either run() rejected, dbt actually crashed — surface the real error
// instead of the generic "Could not parse" message.
const realError = extractDbtError(lines, primaryRunError, plainRunError)
if (realError) {
throw new Error(`dbt show failed: ${realError}`)
}

throw new Error(
Expand All @@ -305,6 +318,50 @@ export async function execDbtShow(sql: string, limit?: number) {
)
}

/** Shape of an execFile rejection — carries stdout/stderr alongside message. */
interface ExecFileError extends Error {
stdout?: string
stderr?: string
code?: number | string
}

/**
* Pick the best human-readable error from a failed `dbt show` invocation.
*
* Preference order:
* 1. A structured `level: "error"` event in the JSON log (dbt's own error msg).
* 2. Stderr from the JSON-mode run.
* 3. Stderr from the plain-text-mode run.
* 4. The exception message itself.
*
* Returns undefined if neither run rejected — caller falls back to the generic
* "Could not parse" message, which is correct when dbt exited 0 but emitted
* something we can't decode.
*/
function extractDbtError(
lines: Record<string, unknown>[],
primary?: ExecFileError,
plain?: ExecFileError,
): string | undefined {
if (!primary && !plain) return undefined

const errorEvent = lines.find(
(l: any) => l.info?.level === "error" || l.level === "error",
) as any
const structuredMsg = errorEvent?.info?.msg ?? errorEvent?.msg

const primaryStderr = primary?.stderr?.toString().trim()
const plainStderr = plain?.stderr?.toString().trim()

return (
(typeof structuredMsg === "string" && structuredMsg.length > 0 ? structuredMsg : undefined) ??
(primaryStderr && primaryStderr.length > 0 ? primaryStderr : undefined) ??
(plainStderr && plainStderr.length > 0 ? plainStderr : undefined) ??
primary?.message ??
plain?.message
)
}

/**
* Compile a model via `dbt compile --select <model>` and return compiled SQL.
*/
Expand Down
67 changes: 67 additions & 0 deletions packages/dbt-tools/test/dbt-cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,73 @@ describe("execDbtShow", () => {

await expect(execDbtShow("SELECT 1")).rejects.toThrow("Could not parse dbt show output in any format")
})

// --- Bubble real dbt error instead of generic "Could not parse" ---

test("surfaces real dbt stderr when run fails", async () => {
mockExecFile.mockImplementation((_cmd: string, _args: string[], _opts: any, cb: Function) => {
const err: any = new Error("Command failed: dbt show --inline ...")
err.code = 1
err.stdout = ""
err.stderr =
"Runtime Error: Failed to read package: No dbt_project.yml found at expected path dbt_packages/dbt_utils/dbt_project.yml"
cb(err, err.stdout, err.stderr)
})

await expect(execDbtShow("SELECT 1")).rejects.toThrow(/Failed to read package/)
await expect(execDbtShow("SELECT 1")).rejects.toThrow(/dbt show failed/)
})

test("prefers structured error event in JSON log over raw stderr", async () => {
const errorLog = JSON.stringify({
info: {
level: "error",
msg: "Compilation Error: Model 'foo' depends on a node named 'bar' which was not found",
},
})
mockExecFile.mockImplementation((_cmd: string, _args: string[], _opts: any, cb: Function) => {
const err: any = new Error("Command failed")
err.code = 1
err.stdout = errorLog
err.stderr = "exit status 1"
cb(err, err.stdout, err.stderr)
})

await expect(execDbtShow("SELECT 1")).rejects.toThrow(/Compilation Error.*Model 'foo'/)
})

test("does not surface generic 'Could not parse' when dbt actually crashed", async () => {
mockExecFile.mockImplementation((_cmd: string, _args: string[], _opts: any, cb: Function) => {
const err: any = new Error("Command failed")
err.code = 2
err.stdout = ""
err.stderr = "Database Error: connection refused"
cb(err, err.stdout, err.stderr)
})

await expect(execDbtShow("SELECT 1")).rejects.not.toThrow(/Could not parse dbt show output/)
})

test("preserves generic 'Could not parse' when dbt exited 0 but output unparseable", async () => {
// Existing behavior — dbt didn't crash, we just couldn't decode its output.
mockExecFile.mockImplementation((_cmd: string, _args: string[], _opts: any, cb: Function) => {
cb(null, "some unparseable output", "")
})

await expect(execDbtShow("SELECT 1")).rejects.toThrow("Could not parse dbt show output in any format")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Duplicate test: "preserves generic 'Could not parse' when dbt exited 0 but output unparseable" tests the exact same scenario as the existing "Tier 3: throws with helpful message when all tiers fail" test. Both use an identical mock (cb(null, "some unparseable output", "")) and assert the same rejection string. The new test adds zero coverage and creates maintenance overhead — any change to the generic-error path must be kept in sync across two identical tests.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/dbt-tools/test/dbt-cli.test.ts, line 205:

<comment>Duplicate test: "preserves generic 'Could not parse' when dbt exited 0 but output unparseable" tests the exact same scenario as the existing "Tier 3: throws with helpful message when all tiers fail" test. Both use an identical mock (cb(null, "some unparseable output", "")) and assert the same rejection string. The new test adds zero coverage and creates maintenance overhead — any change to the generic-error path must be kept in sync across two identical tests.</comment>

<file context>
@@ -149,6 +149,73 @@ describe("execDbtShow", () => {
+      cb(null, "some unparseable output", "")
+    })
+
+    await expect(execDbtShow("SELECT 1")).rejects.toThrow("Could not parse dbt show output in any format")
+  })
+
</file context>

})

test("falls back to error message when stderr is empty", async () => {
mockExecFile.mockImplementation((_cmd: string, _args: string[], _opts: any, cb: Function) => {
const err: any = new Error("spawn ENOENT")
err.code = "ENOENT"
err.stdout = ""
err.stderr = ""
cb(err, "", "")
})

await expect(execDbtShow("SELECT 1")).rejects.toThrow(/spawn ENOENT|dbt show failed/)
})
})

// ---------------------------------------------------------------------------
Expand Down
Loading