From f1fa89c696e82546d73e762f0e7c78b404da63dd Mon Sep 17 00:00:00 2001 From: Atila Fassina Date: Tue, 2 Jun 2026 08:58:21 +0200 Subject: [PATCH 1/3] fix(appkit): don't crash typegen when the warehouse is unreachable Type generation threw an uncaught error whenever the SQL warehouse was down. Every DESCRIBE QUERY failed, all queries degraded to an unknown result, and generateFromEntryPoint unconditionally threw an aggregate "Type generation failed" error that escaped uncaught at the Vite plugin (un-awaited generate()) and the CLI (sync cmd.parse()) call sites. Distinguish connectivity failures from genuine SQL errors: - Connectivity (executeStatement rejects): degrade silently. Reuse the last-known-good cached type if present, otherwise emit an unknown result. Never fatal, so a transient outage no longer fails a build. - SQL error (reachable warehouse, DESCRIBE FAILED): surface via a typed TypegenSyntaxError so the existing prod-throws / dev-warns gate applies. Eligible to fail prod builds only. Also stop caching unknown results: only successful describes with a result schema are persisted, so a transient outage never poisons the cache and a fixed query recovers on the next run. PR1 of 2 (user-visible behavior). PR2 will await the Vite buildStart/watcher, use parseAsync().catch() in the CLI, and add degrade/throw regression tests. Co-authored-by: Isaac Signed-off-by: Atila Fassina --- packages/appkit/src/type-generator/index.ts | 67 ++++++--- .../src/type-generator/query-registry.ts | 132 +++++++++++++----- .../tests/generate-queries.test.ts | 24 ++-- packages/appkit/src/type-generator/types.ts | 27 ++++ 4 files changed, 185 insertions(+), 65 deletions(-) diff --git a/packages/appkit/src/type-generator/index.ts b/packages/appkit/src/type-generator/index.ts index c9a528fe7..e0379152a 100644 --- a/packages/appkit/src/type-generator/index.ts +++ b/packages/appkit/src/type-generator/index.ts @@ -9,12 +9,39 @@ import { } from "./migration"; import { generateQueriesFromDescribe } from "./query-registry"; import { generateServingTypes as generateServingTypesImpl } from "./serving/generator"; -import type { QuerySchema } from "./types"; +import type { QuerySchema, QuerySyntaxError } from "./types"; dotenv.config(); const logger = createLogger("type-generator"); +/** + * Thrown when one or more queries fail `DESCRIBE QUERY` against a *reachable* + * warehouse — i.e. genuine SQL errors (bad table, syntax, incompatible type), + * as opposed to a connectivity failure (warehouse unreachable), which degrades + * silently. Whether this is fatal is the caller's decision: the Vite plugin and + * CLI fail the build in production and warn-only in development. + */ +export class TypegenSyntaxError extends Error { + readonly queries: QuerySyntaxError[]; + + constructor(queries: QuerySyntaxError[], warehouseId?: string) { + const names = queries.map((q) => q.name).join(", "); + super( + [ + `Type generation failed: ${queries.length} ${queries.length === 1 ? "query" : "queries"} could not be described: ${names}.`, + `DESCRIBE QUERY failed for these queries — see the error codes above for details.`, + `Common causes: SQL syntax errors, missing tables/views, or warehouse format incompatibilities.`, + warehouseId + ? `To debug: run the failing query directly in a SQL editor against warehouse ${warehouseId}.` + : `To debug: run the failing query directly in a SQL editor.`, + ].join("\n"), + ); + this.name = "TypegenSyntaxError"; + this.queries = queries; + } +} + /** * Generate type declarations for QueryRegistry * Create the d.ts file from the plugin routes and query schemas @@ -64,28 +91,13 @@ export async function generateFromEntryPoint(options: { logger.debug("Starting type generation..."); let queryRegistry: QuerySchema[] = []; - if (queryFolder) - queryRegistry = await generateQueriesFromDescribe( - queryFolder, - warehouseId, - { - noCache, - }, - ); - - const failedQueries = queryRegistry.filter((q) => - q.type.includes("result: unknown"), - ); - if (failedQueries.length > 0) { - const names = failedQueries.map((q) => q.name).join(", "); - throw new Error( - [ - `Type generation failed: ${failedQueries.length} ${failedQueries.length === 1 ? "query" : "queries"} could not be described: ${names}.`, - `DESCRIBE QUERY failed for these queries — see the error codes above for details.`, - `Common causes: SQL syntax errors, missing tables/views, or warehouse format incompatibilities.`, - `To debug: run the failing query directly in a SQL editor against warehouse ${warehouseId}.`, - ].join("\n"), - ); + let syntaxErrors: QuerySyntaxError[] = []; + if (queryFolder) { + const result = await generateQueriesFromDescribe(queryFolder, warehouseId, { + noCache, + }); + queryRegistry = result.schemas; + syntaxErrors = result.syntaxErrors; } const typeDeclarations = generateTypeDeclarations(queryRegistry); @@ -97,6 +109,15 @@ export async function generateFromEntryPoint(options: { await removeOldGeneratedTypes(projectRoot, "appKitTypes.d.ts"); await migrateProjectConfig(projectRoot); + // Types are always written above — including `result: unknown` for any query + // that could not be described — so a transient warehouse outage never blocks a + // build. Only a genuine SQL error against a REACHABLE warehouse is surfaced as + // a throw; the Vite plugin / CLI apply the prod-fails / dev-warns gate. + // Connectivity failures are absent from `syntaxErrors`, so they pass silently. + if (syntaxErrors.length > 0) { + throw new TypegenSyntaxError(syntaxErrors, warehouseId); + } + logger.debug("Type generation complete!"); } diff --git a/packages/appkit/src/type-generator/query-registry.ts b/packages/appkit/src/type-generator/query-registry.ts index 06ee64bac..3ab4c678f 100644 --- a/packages/appkit/src/type-generator/query-registry.ts +++ b/packages/appkit/src/type-generator/query-registry.ts @@ -7,7 +7,9 @@ import { CACHE_VERSION, hashSQL, loadCache, saveCache } from "./cache"; import { Spinner } from "./spinner"; import { type DatabricksStatementExecutionResponse, + type QueryGenerationResult, type QuerySchema, + type QuerySyntaxError, sqlTypeToHelper, sqlTypeToMarker, } from "./types"; @@ -272,7 +274,7 @@ export async function generateQueriesFromDescribe( queryFolder: string, warehouseId: string, options: { noCache?: boolean; concurrency?: number } = {}, -): Promise { +): Promise { const { noCache = false, concurrency: rawConcurrency = 10 } = options; const concurrency = typeof rawConcurrency === "number" && Number.isFinite(rawConcurrency) @@ -314,7 +316,9 @@ export async function generateQueriesFromDescribe( const logEntries: Array<{ queryName: string; status: "HIT" | "MISS"; - failed?: boolean; + // Absent for clean hits/misses. "syntax" = bad SQL on a reachable warehouse; + // "connectivity" = warehouse unreachable; "empty" = described but no columns. + kind?: "syntax" | "connectivity" | "empty"; error?: { code?: string; message: string }; }> = []; @@ -369,20 +373,32 @@ export async function generateQueriesFromDescribe( // Phase 2: Execute all uncached DESCRIBE calls in parallel type DescribeResult = | { + // Described successfully with a result schema — the only case we cache. status: "ok"; index: number; schema: QuerySchema; cacheEntry: { hash: string; type: string; retry: boolean }; } | { - status: "fail"; + // Reachable warehouse ran DESCRIBE and rejected the statement — a + // genuine SQL error. Eligible to fail the build; never cached. + status: "syntax"; index: number; schema: QuerySchema; - cacheEntry: { hash: string; type: string; retry: boolean }; error: { code?: string; message: string }; + } + | { + // DESCRIBE succeeded but returned no columns — soft `unknown`. Not a + // failure, not cached, retried next run. + status: "empty"; + index: number; + schema: QuerySchema; }; const freshResults: Array<{ index: number; schema: QuerySchema }> = []; + // Genuine SQL errors (reachable warehouse). Connectivity failures are NOT + // recorded here — they degrade silently so a transient outage isn't fatal. + const syntaxErrors: QuerySyntaxError[] = []; if (uncachedQueries.length > 0) { let completed = 0; @@ -416,25 +432,31 @@ export async function generateQueriesFromDescribe( ); if (result.status.state === "FAILED") { + // The warehouse was reachable and ran DESCRIBE, but the statement + // failed — a genuine SQL error (bad table, syntax, incompatible type). const sqlError = result.status.error?.message || "Query execution failed"; logger.warn("DESCRIBE failed for %s: %s", queryName, sqlError); const type = generateUnknownResultQuery(sql, queryName); return { - status: "fail", + status: "syntax", index, schema: { name: queryName, type }, - cacheEntry: { hash: sqlHash, type, retry: true }, error: parseError(sqlError), }; } const { type, hasResults } = convertToQueryType(result, sql, queryName); + if (!hasResults) { + // Described, but no result columns. Emit `unknown` and retry next run; + // do not cache (we never persist `result: unknown`). + return { status: "empty", index, schema: { name: queryName, type } }; + } return { status: "ok", index, schema: { name: queryName, type }, - cacheEntry: { hash: sqlHash, type, retry: !hasResults }, + cacheEntry: { hash: sqlHash, type, retry: false }, }; }; @@ -450,27 +472,50 @@ export async function generateQueriesFromDescribe( if (entry.status === "fulfilled") { const res = entry.value; freshResults.push({ index: res.index, schema: res.schema }); - cache.queries[queryName] = res.cacheEntry; - logEntries.push({ - queryName, - status: "MISS", - failed: res.status === "fail", - error: res.status === "fail" ? res.error : undefined, - }); + + if (res.status === "ok") { + // Only a successful describe with a result schema is cached. + cache.queries[queryName] = res.cacheEntry; + logEntries.push({ queryName, status: "MISS" }); + } else if (res.status === "syntax") { + // Genuine SQL error — record it for the caller's prod/dev gate. + // Not cached: re-described next run so a fixed query recovers. + syntaxErrors.push({ name: queryName, message: res.error.message }); + logEntries.push({ + queryName, + status: "MISS", + kind: "syntax", + error: res.error, + }); + } else { + // status === "empty": described, no columns. Soft unknown, not cached. + logEntries.push({ queryName, status: "MISS", kind: "empty" }); + } } else { - const { sql, sqlHash, index } = uncachedQueries[batchOffset + i]; + // executeStatement rejected → the warehouse was unreachable (down, + // network, timeout, auth). This is NOT a query error: reuse the + // last-known-good cached type if we have one (the cache only ever + // holds good types now), otherwise emit `unknown`. Never cached, + // never fatal — so a transient outage can't fail the build. + const { sql, index } = uncachedQueries[batchOffset + i]; const reason = entry.reason instanceof Error ? entry.reason.message : String(entry.reason); - logger.warn("DESCRIBE rejected for %s: %s", queryName, reason); - const type = generateUnknownResultQuery(sql, queryName); + const prior = cache.queries[queryName]; + const type = + prior?.type ?? generateUnknownResultQuery(sql, queryName); + logger.warn( + "DESCRIBE unreachable for %s: %s — %s", + queryName, + reason, + prior ? "reusing last cached type" : "emitting unknown (no cache)", + ); freshResults.push({ index, schema: { name: queryName, type } }); - cache.queries[queryName] = { hash: sqlHash, type, retry: true }; logEntries.push({ queryName, status: "MISS", - failed: true, + kind: "connectivity", error: parseError(reason), }); } @@ -507,36 +552,59 @@ export async function generateQueriesFromDescribe( ); console.log(` ${separator}`); for (const entry of logEntries) { - const tag = entry.failed - ? pc.bold(pc.red("ERROR")) - : entry.status === "HIT" - ? `cache ${pc.bold(pc.green("HIT "))}` - : `cache ${pc.bold(pc.yellow("MISS "))}`; + let tag: string; + switch (entry.kind) { + case "syntax": + tag = pc.bold(pc.red("SQL ERR")); + break; + case "connectivity": + tag = pc.bold(pc.yellow("OFFLINE")); + break; + case "empty": + tag = pc.dim("EMPTY "); + break; + default: + tag = + entry.status === "HIT" + ? `cache ${pc.bold(pc.green("HIT "))}` + : `cache ${pc.bold(pc.yellow("MISS "))}`; + } const rawName = entry.queryName.padEnd(maxNameLen); - const name = entry.failed ? pc.dim(pc.strikethrough(rawName)) : rawName; + // Only genuine SQL errors are struck through. Connectivity/empty kept a + // usable type (reused or unknown), so they read as degraded, not broken. + const name = + entry.kind === "syntax" ? pc.dim(pc.strikethrough(rawName)) : rawName; const errorCode = entry.error?.message.match(/\[([^\]]+)\]/)?.[1]; const reason = errorCode ? ` ${pc.dim(errorCode)}` : ""; console.log(` ${tag} ${name}${reason}`); } const newCount = logEntries.filter( - (e) => e.status === "MISS" && !e.failed, + (e) => e.status === "MISS" && !e.kind, ).length; - const cacheCount = logEntries.filter( - (e) => e.status === "HIT" && !e.failed, + const cacheCount = logEntries.filter((e) => e.status === "HIT").length; + const syntaxCount = logEntries.filter((e) => e.kind === "syntax").length; + const offlineCount = logEntries.filter( + (e) => e.kind === "connectivity", ).length; - const errorCount = logEntries.filter((e) => e.failed).length; + const emptyCount = logEntries.filter((e) => e.kind === "empty").length; console.log(` ${separator}`); const parts = [`${newCount} new`, `${cacheCount} from cache`]; - if (errorCount > 0) - parts.push(`${errorCount} ${errorCount === 1 ? "error" : "errors"}`); + if (syntaxCount > 0) + parts.push( + `${syntaxCount} SQL ${syntaxCount === 1 ? "error" : "errors"}`, + ); + if (offlineCount > 0) parts.push(`${offlineCount} unreachable`); + if (emptyCount > 0) parts.push(`${emptyCount} empty`); console.log(` ${parts.join(", ")}. ${pc.dim(`${elapsed}s`)}`); console.log(""); } // Merge and sort by original file index for deterministic output - return [...cachedResults, ...freshResults] + const schemas = [...cachedResults, ...freshResults] .sort((a, b) => a.index - b.index) .map((r) => r.schema); + + return { schemas, syntaxErrors }; } /** diff --git a/packages/appkit/src/type-generator/tests/generate-queries.test.ts b/packages/appkit/src/type-generator/tests/generate-queries.test.ts index ac43ef9e2..0a707bbc9 100644 --- a/packages/appkit/src/type-generator/tests/generate-queries.test.ts +++ b/packages/appkit/src/type-generator/tests/generate-queries.test.ts @@ -64,7 +64,7 @@ describe("generateQueriesFromDescribe", () => { ]), ); - const schemas = await generateQueriesFromDescribe("/queries", "wh-123"); + const { schemas } = await generateQueriesFromDescribe("/queries", "wh-123"); expect(schemas).toHaveLength(1); expect(schemas[0].name).toBe("users"); @@ -85,7 +85,7 @@ describe("generateQueriesFromDescribe", () => { }, }); - const schemas = await generateQueriesFromDescribe("/queries", "wh-123"); + const { schemas } = await generateQueriesFromDescribe("/queries", "wh-123"); expect(schemas).toHaveLength(1); expect(schemas[0].name).toBe("bad_table"); @@ -102,7 +102,7 @@ describe("generateQueriesFromDescribe", () => { status: { state: "FAILED" }, }); - const schemas = await generateQueriesFromDescribe("/queries", "wh-123"); + const { schemas } = await generateQueriesFromDescribe("/queries", "wh-123"); expect(schemas).toHaveLength(1); expect(schemas[0].name).toBe("query"); @@ -127,7 +127,7 @@ describe("generateQueriesFromDescribe", () => { }, }); - const schemas = await generateQueriesFromDescribe("/queries", "wh-123"); + const { schemas } = await generateQueriesFromDescribe("/queries", "wh-123"); expect(schemas).toHaveLength(2); @@ -143,7 +143,7 @@ describe("generateQueriesFromDescribe", () => { expect(mocks.saveCache).toHaveBeenCalledTimes(1); }); - test("all queries fail — caches with retry flag, all unknown result types", async () => { + test("all queries fail (connectivity + syntax) — all produce unknown result types", async () => { mocks.readdir.mockResolvedValue(["a.sql", "b.sql"]); mocks.readFile .mockResolvedValueOnce("SELECT * FROM table_a") @@ -156,7 +156,7 @@ describe("generateQueriesFromDescribe", () => { status: { state: "FAILED", error: { message: "Table not found" } }, }); - const schemas = await generateQueriesFromDescribe("/queries", "wh-123"); + const { schemas } = await generateQueriesFromDescribe("/queries", "wh-123"); expect(schemas).toHaveLength(2); expect(schemas[0].name).toBe("a"); @@ -181,9 +181,13 @@ describe("generateQueriesFromDescribe", () => { .mockResolvedValueOnce(succeededResult([["id", "INT", null]])) .mockResolvedValueOnce(succeededResult([["id", "INT", null]])); - const schemas = await generateQueriesFromDescribe("/queries", "wh-123", { - concurrency: 2, - }); + const { schemas } = await generateQueriesFromDescribe( + "/queries", + "wh-123", + { + concurrency: 2, + }, + ); expect(schemas).toHaveLength(3); expect(schemas[0].name).toBe("q1"); @@ -201,7 +205,7 @@ describe("generateQueriesFromDescribe", () => { ); mocks.executeStatement.mockRejectedValueOnce(new Error("timeout")); - const schemas = await generateQueriesFromDescribe("/queries", "wh-123"); + const { schemas } = await generateQueriesFromDescribe("/queries", "wh-123"); expect(schemas).toHaveLength(1); expect(schemas[0].type).toContain("status: SQLStringMarker"); diff --git a/packages/appkit/src/type-generator/types.ts b/packages/appkit/src/type-generator/types.ts index f54176a8c..aefa42823 100644 --- a/packages/appkit/src/type-generator/types.ts +++ b/packages/appkit/src/type-generator/types.ts @@ -76,3 +76,30 @@ export interface QuerySchema { name: string; type: string; } + +/** + * A genuine SQL error: `DESCRIBE QUERY` ran against a *reachable* warehouse and + * the warehouse reported the statement as FAILED (bad table, syntax error, + * incompatible type, …). Distinct from a connectivity failure (warehouse + * unreachable), which is non-fatal and never recorded here. + * @property name - the query name + * @property message - the SQL error message reported by the warehouse + */ +export interface QuerySyntaxError { + name: string; + message: string; +} + +/** + * Result of describing a folder of queries. + * @property schemas - one schema per query, in original file order. Queries that + * could not be described carry `result: unknown` so output stays valid. + * @property syntaxErrors - queries whose DESCRIBE failed against a reachable + * warehouse (genuine SQL errors). Connectivity failures are deliberately NOT + * included: they degrade silently (reuse last-known-good type or emit + * `unknown`) so a transient outage never fails a build. + */ +export interface QueryGenerationResult { + schemas: QuerySchema[]; + syntaxErrors: QuerySyntaxError[]; +} From bea1bb17c795365fac83ccd7a3d7f636dc6f19d7 Mon Sep 17 00:00:00 2001 From: Atila Fassina Date: Tue, 2 Jun 2026 09:25:17 +0200 Subject: [PATCH 2/3] test(appkit): cover typegen warehouse-down classification and throw seam Add the regression coverage the warehouse-down crash slipped through: the prior suite tested rejection->unknown as graceful but never connected it to the aggregate throw in generateFromEntryPoint. query-registry (generate-queries.test.ts): - connectivity reuses the last-known-good cached type - empty result (described, no columns) -> unknown, not syntax, not cached - syntax (FAILED) -> recorded in syntaxErrors, not cached - cache HIT serves the stored type without a warehouse call - legacy retry-flagged entry is re-described, not reused - mixed run records only the syntax failure; failures are not persisted generateFromEntryPoint (index.test.ts): - syntax errors throw TypegenSyntaxError - connectivity-only failures do NOT throw (the warehouse-down regression) - the .d.ts is written before the throw Layers 1+2 of the test plan; Layer 3 (analytics vite-plugin) and Layer 4 (CLI exit codes) land in PR2 with their await/parseAsync refactors. Co-authored-by: Isaac Signed-off-by: Atila Fassina --- .../tests/generate-queries.test.ts | 145 +++++++++++++++++- .../src/type-generator/tests/index.test.ts | 105 ++++++++++++- 2 files changed, 246 insertions(+), 4 deletions(-) diff --git a/packages/appkit/src/type-generator/tests/generate-queries.test.ts b/packages/appkit/src/type-generator/tests/generate-queries.test.ts index 0a707bbc9..5d032fb33 100644 --- a/packages/appkit/src/type-generator/tests/generate-queries.test.ts +++ b/packages/appkit/src/type-generator/tests/generate-queries.test.ts @@ -38,6 +38,20 @@ vi.mock("../cache", async (importOriginal) => { }); const { generateQueriesFromDescribe } = await import("../query-registry"); +const { CACHE_VERSION, hashSQL } = await import("../cache"); + +// Sentinel for a previously-generated good type. The code passes cached types +// through verbatim, so equality proves reuse rather than regeneration. +const CACHED_GOOD_TYPE = "RESULT_REUSED_FROM_CACHE"; + +// The `queries` map of the cache object last handed to saveCache — i.e. what +// actually got persisted this run. +const lastSavedQueries = () => + ( + mocks.saveCache.mock.calls.at(-1)?.[0] as + | { queries: Record } + | undefined + )?.queries; function succeededResult(columns: [string, string, string | null][]) { return { @@ -64,7 +78,10 @@ describe("generateQueriesFromDescribe", () => { ]), ); - const { schemas } = await generateQueriesFromDescribe("/queries", "wh-123"); + const { schemas, syntaxErrors } = await generateQueriesFromDescribe( + "/queries", + "wh-123", + ); expect(schemas).toHaveLength(1); expect(schemas[0].name).toBe("users"); @@ -72,6 +89,9 @@ describe("generateQueriesFromDescribe", () => { expect(schemas[0].type).toContain("name: string"); expect(mocks.spinnerStop).toHaveBeenCalledWith(""); expect(mocks.saveCache).toHaveBeenCalledTimes(1); + // clean success: cached, and not flagged as a syntax error + expect(syntaxErrors).toEqual([]); + expect(lastSavedQueries()?.users.type).toContain("id: number"); }); test("FAILED status with error message — reports SQL error and produces unknown result type", async () => { @@ -156,7 +176,10 @@ describe("generateQueriesFromDescribe", () => { status: { state: "FAILED", error: { message: "Table not found" } }, }); - const { schemas } = await generateQueriesFromDescribe("/queries", "wh-123"); + const { schemas, syntaxErrors } = await generateQueriesFromDescribe( + "/queries", + "wh-123", + ); expect(schemas).toHaveLength(2); expect(schemas[0].name).toBe("a"); @@ -166,6 +189,11 @@ describe("generateQueriesFromDescribe", () => { // saveCache called once after all parallel queries complete expect(mocks.saveCache).toHaveBeenCalledTimes(1); + // a = connectivity (rejected) → NOT a syntax error; b = FAILED → syntax error + expect(syntaxErrors).toEqual([{ name: "b", message: "Table not found" }]); + // neither failure is persisted to the cache + expect(lastSavedQueries()).not.toHaveProperty("a"); + expect(lastSavedQueries()).not.toHaveProperty("b"); }); test("concurrency batching — saves cache after each batch", async () => { @@ -212,4 +240,117 @@ describe("generateQueriesFromDescribe", () => { expect(schemas[0].type).toContain("org: SQLTypeMarker"); expect(schemas[0].type).toContain("result: unknown"); }); + + test("connectivity failure reuses the last-known-good cached type", async () => { + const sql = "SELECT id FROM users"; + mocks.readdir.mockResolvedValue(["users.sql"]); + mocks.readFile.mockResolvedValue(sql); + // A prior good type cached under a STALE hash: the query is a cache MISS + // (so DESCRIBE is attempted) but a known-good type still exists to reuse. + mocks.loadCache.mockReturnValueOnce({ + version: CACHE_VERSION, + queries: { + users: { hash: "stale-hash", type: CACHED_GOOD_TYPE, retry: false }, + }, + }); + mocks.executeStatement.mockRejectedValueOnce(new Error("ECONNREFUSED")); + + const { schemas, syntaxErrors } = await generateQueriesFromDescribe( + "/queries", + "wh-123", + ); + + // reused the cached type instead of clobbering it with `result: unknown` + expect(schemas[0].type).toBe(CACHED_GOOD_TYPE); + expect(schemas[0].type).not.toContain("result: unknown"); + // connectivity is never recorded as a syntax error + expect(syntaxErrors).toEqual([]); + // the existing good entry is left intact (not overwritten) + expect(lastSavedQueries()?.users).toEqual({ + hash: "stale-hash", + type: CACHED_GOOD_TYPE, + retry: false, + }); + }); + + test("empty result (described, no columns) is unknown, not a syntax error, not cached", async () => { + mocks.readdir.mockResolvedValue(["empty.sql"]); + mocks.readFile.mockResolvedValue("SELECT 1"); + mocks.executeStatement.mockResolvedValue(succeededResult([])); + + const { schemas, syntaxErrors } = await generateQueriesFromDescribe( + "/queries", + "wh-123", + ); + + expect(schemas[0].type).toContain("result: unknown"); + expect(syntaxErrors).toEqual([]); + expect(lastSavedQueries()).not.toHaveProperty("empty"); + }); + + test("syntax error (FAILED) is recorded in syntaxErrors and not cached", async () => { + mocks.readdir.mockResolvedValue(["broken.sql"]); + mocks.readFile.mockResolvedValue("SELECT * FROM missing"); + mocks.executeStatement.mockResolvedValue({ + statement_id: "stmt", + status: { + state: "FAILED", + error: { message: "Table or view not found: missing" }, + }, + }); + + const { schemas, syntaxErrors } = await generateQueriesFromDescribe( + "/queries", + "wh-123", + ); + + expect(schemas[0].type).toContain("result: unknown"); + expect(syntaxErrors).toEqual([ + { name: "broken", message: "Table or view not found: missing" }, + ]); + expect(lastSavedQueries()).not.toHaveProperty("broken"); + }); + + test("cache HIT serves the stored type without calling the warehouse", async () => { + const sql = "SELECT id FROM t"; + mocks.readdir.mockResolvedValue(["t.sql"]); + mocks.readFile.mockResolvedValue(sql); + mocks.loadCache.mockReturnValueOnce({ + version: CACHE_VERSION, + queries: { + t: { hash: hashSQL(sql), type: CACHED_GOOD_TYPE, retry: false }, + }, + }); + + const { schemas, syntaxErrors } = await generateQueriesFromDescribe( + "/queries", + "wh-123", + ); + + expect(mocks.executeStatement).not.toHaveBeenCalled(); + expect(schemas[0].type).toBe(CACHED_GOOD_TYPE); + expect(syntaxErrors).toEqual([]); + }); + + test("stale retry-flagged cache entry is re-described, not reused", async () => { + const sql = "SELECT id FROM t"; + mocks.readdir.mockResolvedValue(["t.sql"]); + mocks.readFile.mockResolvedValue(sql); + // Matching hash but retry:true (legacy poisoned entry) → must NOT be a HIT. + mocks.loadCache.mockReturnValueOnce({ + version: CACHE_VERSION, + queries: { + t: { hash: hashSQL(sql), type: "STALE_UNKNOWN", retry: true }, + }, + }); + mocks.executeStatement.mockResolvedValue( + succeededResult([["id", "INT", null]]), + ); + + const { schemas } = await generateQueriesFromDescribe("/queries", "wh-123"); + + expect(mocks.executeStatement).toHaveBeenCalledTimes(1); + expect(schemas[0].type).toContain("id: number"); + expect(schemas[0].type).not.toBe("STALE_UNKNOWN"); + }); }); diff --git a/packages/appkit/src/type-generator/tests/index.test.ts b/packages/appkit/src/type-generator/tests/index.test.ts index bd2052273..02e149df2 100644 --- a/packages/appkit/src/type-generator/tests/index.test.ts +++ b/packages/appkit/src/type-generator/tests/index.test.ts @@ -1,7 +1,30 @@ import fs from "node:fs"; import path from "node:path"; -import { afterAll, beforeAll, describe, expect, test } from "vitest"; -import { generateFromEntryPoint } from "../index"; +import { + afterAll, + beforeAll, + beforeEach, + describe, + expect, + test, + vi, +} from "vitest"; + +const mocks = vi.hoisted(() => ({ + generateQueriesFromDescribe: vi.fn(), +})); + +// Mock only the warehouse-describe step; index.ts owns the throw decision we +// want to exercise (syntax errors fatal, connectivity failures non-fatal). +vi.mock("../query-registry", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + generateQueriesFromDescribe: mocks.generateQueriesFromDescribe, + }; +}); + +const { generateFromEntryPoint, TypegenSyntaxError } = await import("../index"); const outputDir = path.join(__dirname, "__output__"); @@ -52,3 +75,81 @@ describe("generateFromEntryPoint", () => { expect(content).toContain("interface QueryRegistry {}"); }); }); + +describe("generateFromEntryPoint — query failure handling", () => { + const failuresDir = path.join(__dirname, "__output_failures__"); + const outFile = path.join(failuresDir, "analytics.d.ts"); + + const unknownSchema = (name: string) => ({ + name, + type: `{ name: "${name}"; parameters: Record; result: unknown; }`, + }); + + beforeAll(() => { + if (!fs.existsSync(failuresDir)) { + fs.mkdirSync(failuresDir, { recursive: true }); + } + }); + + afterAll(() => { + if (fs.existsSync(failuresDir)) { + fs.rmSync(failuresDir, { recursive: true }); + } + }); + + beforeEach(() => { + vi.clearAllMocks(); + }); + + test("throws TypegenSyntaxError when a query has a genuine SQL error", async () => { + mocks.generateQueriesFromDescribe.mockResolvedValue({ + schemas: [unknownSchema("bad")], + syntaxErrors: [{ name: "bad", message: "Table not found: bad" }], + }); + + await expect( + generateFromEntryPoint({ + outFile, + queryFolder: "/queries", + warehouseId: "wh-1", + }), + ).rejects.toThrow(TypegenSyntaxError); + }); + + test("does not throw when only connectivity failures occurred (warehouse down)", async () => { + mocks.generateQueriesFromDescribe.mockResolvedValue({ + schemas: [unknownSchema("a"), unknownSchema("b")], + syntaxErrors: [], + }); + + // The reported bug: a down warehouse must NOT crash type generation. + await expect( + generateFromEntryPoint({ + outFile, + queryFolder: "/queries", + warehouseId: "wh-1", + }), + ).resolves.toBeUndefined(); + }); + + test("writes the .d.ts before throwing on a syntax error", async () => { + mocks.generateQueriesFromDescribe.mockResolvedValue({ + schemas: [unknownSchema("bad")], + syntaxErrors: [{ name: "bad", message: "Table not found: bad" }], + }); + + await expect( + generateFromEntryPoint({ + outFile, + queryFolder: "/queries", + warehouseId: "wh-1", + }), + ).rejects.toThrow(TypegenSyntaxError); + + // Types are emitted even on failure so the build/dev still has a valid file. + expect(fs.existsSync(outFile)).toBe(true); + expect(fs.readFileSync(outFile, "utf-8")).toContain( + "interface QueryRegistry", + ); + }); +}); From 1e6121695e038dfd1a86546f6ea5d95ece329bb4 Mon Sep 17 00:00:00 2001 From: Atila Fassina Date: Wed, 3 Jun 2026 14:17:42 +0200 Subject: [PATCH 3/3] fix: classify typegen describe rejections Signed-off-by: Atila Fassina --- .../src/type-generator/query-registry.ts | 41 ++++++++++++++----- .../tests/generate-queries.test.ts | 28 ++++++++++--- 2 files changed, 53 insertions(+), 16 deletions(-) diff --git a/packages/appkit/src/type-generator/query-registry.ts b/packages/appkit/src/type-generator/query-registry.ts index 3ab4c678f..a413ee192 100644 --- a/packages/appkit/src/type-generator/query-registry.ts +++ b/packages/appkit/src/type-generator/query-registry.ts @@ -84,6 +84,17 @@ function parseError(raw: string): { code?: string; message: string } { return { message: raw }; } +function isConnectivityError(raw: string): boolean { + return ( + /\b(ECONNREFUSED|ECONNRESET|ENOTFOUND|ETIMEDOUT|EAI_AGAIN|EHOSTUNREACH|ENETUNREACH)\b/i.test( + raw, + ) || + /\b(connection refused|connection reset|fetch failed|network error|socket hang up|timed? ?out|timeout)\b/i.test( + raw, + ) + ); +} + /** * Extract parameters from a SQL query * @param sql - the SQL query to extract parameters from @@ -492,31 +503,41 @@ export async function generateQueriesFromDescribe( logEntries.push({ queryName, status: "MISS", kind: "empty" }); } } else { - // executeStatement rejected → the warehouse was unreachable (down, - // network, timeout, auth). This is NOT a query error: reuse the - // last-known-good cached type if we have one (the cache only ever - // holds good types now), otherwise emit `unknown`. Never cached, - // never fatal — so a transient outage can't fail the build. - const { sql, index } = uncachedQueries[batchOffset + i]; + // executeStatement rejected before the warehouse returned a statement + // result. Only clear transport failures are treated as offline; auth, + // bad warehouse IDs, malformed requests, and SDK/config failures stay + // fatal so users fix the underlying setup issue. + const { sql, sqlHash, index } = uncachedQueries[batchOffset + i]; const reason = entry.reason instanceof Error ? entry.reason.message : String(entry.reason); + const error = parseError(reason); + if (!isConnectivityError(reason)) { + spinner.stop(""); + throw new Error( + `DESCRIBE request failed for ${queryName}: ${error.message}`, + ); + } const prior = cache.queries[queryName]; - const type = - prior?.type ?? generateUnknownResultQuery(sql, queryName); + const canReusePrior = prior?.hash === sqlHash && !prior.retry; + const type = canReusePrior + ? prior.type + : generateUnknownResultQuery(sql, queryName); logger.warn( "DESCRIBE unreachable for %s: %s — %s", queryName, reason, - prior ? "reusing last cached type" : "emitting unknown (no cache)", + canReusePrior + ? "reusing last cached type" + : "emitting unknown (no matching cache)", ); freshResults.push({ index, schema: { name: queryName, type } }); logEntries.push({ queryName, status: "MISS", kind: "connectivity", - error: parseError(reason), + error, }); } } diff --git a/packages/appkit/src/type-generator/tests/generate-queries.test.ts b/packages/appkit/src/type-generator/tests/generate-queries.test.ts index 5d032fb33..e6b2cd081 100644 --- a/packages/appkit/src/type-generator/tests/generate-queries.test.ts +++ b/packages/appkit/src/type-generator/tests/generate-queries.test.ts @@ -241,12 +241,13 @@ describe("generateQueriesFromDescribe", () => { expect(schemas[0].type).toContain("result: unknown"); }); - test("connectivity failure reuses the last-known-good cached type", async () => { + test("connectivity failure with stale cache emits unknown for the current SQL", async () => { const sql = "SELECT id FROM users"; mocks.readdir.mockResolvedValue(["users.sql"]); mocks.readFile.mockResolvedValue(sql); // A prior good type cached under a STALE hash: the query is a cache MISS - // (so DESCRIBE is attempted) but a known-good type still exists to reuse. + // (so DESCRIBE is attempted). If the warehouse is unreachable, do not + // publish the stale result columns for different SQL text. mocks.loadCache.mockReturnValueOnce({ version: CACHE_VERSION, queries: { @@ -260,12 +261,11 @@ describe("generateQueriesFromDescribe", () => { "wh-123", ); - // reused the cached type instead of clobbering it with `result: unknown` - expect(schemas[0].type).toBe(CACHED_GOOD_TYPE); - expect(schemas[0].type).not.toContain("result: unknown"); + expect(schemas[0].type).not.toBe(CACHED_GOOD_TYPE); + expect(schemas[0].type).toContain("result: unknown"); // connectivity is never recorded as a syntax error expect(syntaxErrors).toEqual([]); - // the existing good entry is left intact (not overwritten) + // the existing good entry is left intact (not overwritten with unknown) expect(lastSavedQueries()?.users).toEqual({ hash: "stale-hash", type: CACHED_GOOD_TYPE, @@ -273,6 +273,22 @@ describe("generateQueriesFromDescribe", () => { }); }); + test("fatal rejected DESCRIBE request is not downgraded to offline", async () => { + mocks.readdir.mockResolvedValue(["users.sql"]); + mocks.readFile.mockResolvedValue("SELECT id FROM users"); + mocks.executeStatement.mockRejectedValueOnce( + new Error("PERMISSION_DENIED: missing warehouse permission"), + ); + + await expect( + generateQueriesFromDescribe("/queries", "wh-123"), + ).rejects.toThrow( + "DESCRIBE request failed for users: PERMISSION_DENIED: missing warehouse permission", + ); + + expect(mocks.saveCache).not.toHaveBeenCalled(); + }); + test("empty result (described, no columns) is unknown, not a syntax error, not cached", async () => { mocks.readdir.mockResolvedValue(["empty.sql"]); mocks.readFile.mockResolvedValue("SELECT 1");