From 7579098170a78c80b86ca0b34c6d95e52bcbeb23 Mon Sep 17 00:00:00 2001 From: Atila Fassina Date: Tue, 1 Sep 2026 16:45:31 +0200 Subject: [PATCH 1/2] fix(appkit): degrade typegen on build-time auth errors instead of failing the build MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A deploy build runs `appkit generate-types --wait` (blocking mode). When a per-query or per-metric-view DESCRIBE rejected with anything the connectivity allowlist didn't recognize, it was recorded as a fatal error and thrown before the committed-types gate — so a build-time PERMISSION_DENIED failed the deploy even though generated types were committed. Build-time identity is not the app's runtime on-behalf-of identity, so a build-time permission gap should degrade to committed types, not block a deploy. Add `isAuthError` (HTTP 401/403 and Databricks error_code PERMISSION_DENIED / UNAUTHENTICATED, including a statusless error_code carried as a field or JSON body in the message) and route auth failures through the same degrade path as connectivity in both the query and metric-view DESCRIBE paths. Bad SQL, bad warehouse id (404), and malformed requests (400) stay fatal. A fresh checkout with no committed types still crashes via the has-types gate. Co-authored-by: Isaac Signed-off-by: Atila Fassina --- packages/appkit/src/type-generator/errors.ts | 76 +++++++++++++++---- .../src/type-generator/mv-registry/sync.ts | 24 ++++-- .../src/type-generator/mv-registry/types.ts | 6 +- .../src/type-generator/query-registry.ts | 21 +++-- .../src/type-generator/tests/errors.test.ts | 67 +++++++++++++++- .../type-generator/tests/mv-registry.test.ts | 34 +++++++-- 6 files changed, 191 insertions(+), 37 deletions(-) diff --git a/packages/appkit/src/type-generator/errors.ts b/packages/appkit/src/type-generator/errors.ts index a6053f576..92798530c 100644 --- a/packages/appkit/src/type-generator/errors.ts +++ b/packages/appkit/src/type-generator/errors.ts @@ -180,23 +180,46 @@ export function classifyBlockingFailure( return "environmental"; } +// Databricks REST/SDK auth error codes. These arrive as an `error_code` string +// (a top-level field, or a JSON body embedded in the message) rather than a +// numeric HTTP status, so status-only detection misses them. +const AUTH_ERROR_CODES = new Set(["PERMISSION_DENIED", "UNAUTHENTICATED"]); + /** - * Coarse cause label for an environmental failure, used by the `--wait` - * committed-types warning so the log says *why* generation fell back. - * - * Returns: - * - "unreachable": transport/connectivity failure (see {@link isConnectivityError}). - * - "auth": HTTP 401/403, including a status carried on `response.status` or - * wrapped in a `cause`/`AggregateError` chain. - * - "unavailable": everything else (DELETED/DELETING, wait timeouts, degraded - * DESCRIBEs). + * Extract a Databricks `error_code` (e.g. "PERMISSION_DENIED") from a thrown + * error. The SDK surfaces it either as a top-level `error_code` field or as a + * JSON body embedded in the message string, e.g. + * `Response from server (Forbidden) {"error_code":"PERMISSION_DENIED",...}`. */ -export function classifyEnvironmentalCause( - error: unknown, -): "auth" | "unreachable" | "unavailable" { - if (isConnectivityError(error)) return "unreachable"; +function getDatabricksErrorCode(error: unknown): string | undefined { + if (!isObject(error)) return undefined; + if (typeof error.error_code === "string") return error.error_code; + + const message = typeof error.message === "string" ? error.message : undefined; + const jsonMatch = message?.match(/\{[\s\S]*\}/); + if (jsonMatch) { + try { + const parsed = JSON.parse(jsonMatch[0]) as { error_code?: unknown }; + if (typeof parsed.error_code === "string") return parsed.error_code; + } catch { + // not valid JSON — fall through + } + } + return undefined; +} - // Walk the error chain so a wrapped 401/403 is still labeled as auth. +/** + * True when a thrown failure is an authentication/authorization problem: an + * HTTP 401/403, or a Databricks `error_code` of PERMISSION_DENIED / + * UNAUTHENTICATED (which can arrive with no numeric status). Walks + * `cause`/`AggregateError` chains so a wrapped auth error is still recognized. + * + * Callers degrade rather than fail the build on `true`: a build-time identity + * gap — the build runs as a different principal than the app's runtime + * on-behalf-of user — must not block a deploy when committed types exist. The + * has-types gate still crashes a fresh checkout with nothing to fall back to. + */ +export function isAuthError(error: unknown): boolean { const seen = new Set(); const stack = [error]; @@ -206,10 +229,33 @@ export function classifyEnvironmentalCause( seen.add(current); const status = getErrorStatus(current); - if (status !== undefined && AUTH_ERROR_STATUSES.has(status)) return "auth"; + if (status !== undefined && AUTH_ERROR_STATUSES.has(status)) return true; + + const code = getDatabricksErrorCode(current); + if (code && AUTH_ERROR_CODES.has(code)) return true; stack.push(...getErrorChildren(current)); } + return false; +} + +/** + * Coarse cause label for an environmental failure, used by the `--wait` + * committed-types warning so the log says *why* generation fell back. + * + * Returns: + * - "unreachable": transport/connectivity failure (see {@link isConnectivityError}). + * - "auth": HTTP 401/403 or a PERMISSION_DENIED / UNAUTHENTICATED `error_code`, + * including one carried on `response.status` or wrapped in a + * `cause`/`AggregateError` chain (see {@link isAuthError}). + * - "unavailable": everything else (DELETED/DELETING, wait timeouts, degraded + * DESCRIBEs). + */ +export function classifyEnvironmentalCause( + error: unknown, +): "auth" | "unreachable" | "unavailable" { + if (isConnectivityError(error)) return "unreachable"; + if (isAuthError(error)) return "auth"; return "unavailable"; } diff --git a/packages/appkit/src/type-generator/mv-registry/sync.ts b/packages/appkit/src/type-generator/mv-registry/sync.ts index bc9fd1c29..43a4242cf 100644 --- a/packages/appkit/src/type-generator/mv-registry/sync.ts +++ b/packages/appkit/src/type-generator/mv-registry/sync.ts @@ -1,4 +1,8 @@ -import { getErrorDiagnostic, isConnectivityError } from "../errors"; +import { + getErrorDiagnostic, + isAuthError, + isConnectivityError, +} from "../errors"; import type { DatabricksStatementExecutionResponse } from "../types"; import { extractMetricColumns, @@ -73,10 +77,18 @@ export async function syncMetrics( response = await fetcher(entry.source); } catch (err) { const reason = `DESCRIBE TABLE EXTENDED failed: ${getErrorDiagnostic(err)}`; - // Connectivity blips self-converge (retry next pass); auth, a bad - // warehouse id, a truncated / multi-chunk result, or a malformed request - // are deterministic and must surface — the same split the query path makes. - return failedOutcome(index, entry, reason, isConnectivityError(err)); + // Connectivity blips self-converge (retry next pass), and a build-time + // auth/permission gap (the build runs as a different principal than the + // app's runtime on-behalf-of user) must not fail a deploy when committed + // types exist — both degrade. A bad warehouse id, a truncated / + // multi-chunk result, or a malformed request are deterministic and must + // surface — the same split the query path makes. + return failedOutcome( + index, + entry, + reason, + isConnectivityError(err) || isAuthError(err), + ); } const state = response.status?.state; @@ -139,7 +151,7 @@ export async function syncMetrics( index, entry, `DESCRIBE TABLE EXTENDED failed: ${getErrorDiagnostic(result.reason)}`, - isConnectivityError(result.reason), + isConnectivityError(result.reason) || isAuthError(result.reason), ); schemas[index] = schema; failureSlots[index] = failure; diff --git a/packages/appkit/src/type-generator/mv-registry/types.ts b/packages/appkit/src/type-generator/mv-registry/types.ts index fc973c6ff..ffd07be57 100644 --- a/packages/appkit/src/type-generator/mv-registry/types.ts +++ b/packages/appkit/src/type-generator/mv-registry/types.ts @@ -128,8 +128,10 @@ export interface MetricSyncFailure { /** Single human-readable reason (DESCRIBE failed, parse failed, zero columns). */ reason: string; /** - * Whether the failure is expected to self-converge on a later pass without - * a config change. + * Whether the failure should degrade rather than fail the build: a + * connectivity blip that self-converges on a later pass, or a build-time + * auth/permission gap that the committed-types gate handles. Deterministic + * failures (bad id, malformed request, zero columns) are not transient. */ transient: boolean; } diff --git a/packages/appkit/src/type-generator/query-registry.ts b/packages/appkit/src/type-generator/query-registry.ts index 5ba8df9cf..df8f95217 100644 --- a/packages/appkit/src/type-generator/query-registry.ts +++ b/packages/appkit/src/type-generator/query-registry.ts @@ -11,6 +11,7 @@ import { classifyBlockingFailure, classifyEnvironmentalCause, getErrorDiagnostic, + isAuthError, isConnectivityError, } from "./errors"; import { decidePreflight, type PreflightMode } from "./preflight"; @@ -974,7 +975,12 @@ export async function generateQueriesFromDescribe( schema: { name: queryName, ...degraded }, }); - if (!isConnectivityError(entry.reason)) { + if ( + !isConnectivityError(entry.reason) && + !isAuthError(entry.reason) + ) { + // Bad SQL, a bad/missing warehouse id (404), and malformed + // requests (400) stay fatal so users fix the underlying setup. fatalErrors.push({ name: queryName, message: error.message }); logEntries.push({ queryName, @@ -985,16 +991,21 @@ export async function generateQueriesFromDescribe( continue; } - // Environmental for the same reason as the preflight connectivity - // branch above, so the has-types gate still sees it. + // Environmental for the same reason as the preflight branch above, + // so the has-types gate still sees it: a connectivity blip + // self-converges, and a build-time auth/permission gap (the build + // runs as a different principal than the app's runtime on-behalf-of + // user) must not fail a deploy when committed types exist. if (mode === "blocking") { hadEnvironmentalFailure = true; - environmentalCause = environmentalCause ?? "unreachable"; + environmentalCause = + environmentalCause ?? classifyEnvironmentalCause(entry.reason); } logger.warn( - "DESCRIBE unreachable for %s: %s — %s", + "DESCRIBE degraded for %s (%s): %s — %s", queryName, + classifyEnvironmentalCause(entry.reason), reason, canReusePrior ? "reusing last cached type" diff --git a/packages/appkit/src/type-generator/tests/errors.test.ts b/packages/appkit/src/type-generator/tests/errors.test.ts index d3d5fd874..18973644b 100644 --- a/packages/appkit/src/type-generator/tests/errors.test.ts +++ b/packages/appkit/src/type-generator/tests/errors.test.ts @@ -1,6 +1,10 @@ import { describe, expect, it } from "vitest"; -import { classifyBlockingFailure, classifyEnvironmentalCause } from "../errors"; +import { + classifyBlockingFailure, + classifyEnvironmentalCause, + isAuthError, +} from "../errors"; describe("classifyBlockingFailure", () => { describe("deterministic failures", () => { @@ -291,4 +295,65 @@ describe("classifyEnvironmentalCause", () => { ])("labels %s as unavailable", (_name, error) => { expect(classifyEnvironmentalCause(error)).toBe("unavailable"); }); + + it("labels a PERMISSION_DENIED error_code (no status) as auth", () => { + const error = Object.assign(new Error("2f1a9c…"), { + error_code: "PERMISSION_DENIED", + }); + expect(classifyEnvironmentalCause(error)).toBe("auth"); + }); +}); + +describe("isAuthError", () => { + it.each([401, 403])("detects HTTP %i", (status) => { + expect(isAuthError(Object.assign(new Error("Denied"), { status }))).toBe( + true, + ); + }); + + it("detects a PERMISSION_DENIED error_code carried with no numeric status", () => { + // The shape observed on deploy: an error_code string, no HTTP status. + const error = Object.assign(new Error("2f1a9c…"), { + error_code: "PERMISSION_DENIED", + }); + expect(isAuthError(error)).toBe(true); + }); + + it("detects UNAUTHENTICATED via error_code", () => { + const error = Object.assign(new Error("no token"), { + error_code: "UNAUTHENTICATED", + }); + expect(isAuthError(error)).toBe(true); + }); + + it("detects error_code embedded as a JSON body in the message", () => { + const error = new Error( + 'Response from server (Forbidden) {"error_code":"PERMISSION_DENIED","message":"nope"}', + ); + expect(isAuthError(error)).toBe(true); + }); + + it("detects an auth status wrapped in a cause chain", () => { + const error = new Error("Request failed", { + cause: Object.assign(new Error("Denied"), { status: 403 }), + }); + expect(isAuthError(error)).toBe(true); + }); + + it.each([ + ["a bad-id 404", Object.assign(new Error("Not found"), { status: 404 })], + ["a 400", Object.assign(new Error("Bad request"), { status: 400 })], + [ + "a connectivity code", + Object.assign(new Error("x"), { code: "ECONNREFUSED" }), + ], + [ + "a non-auth error_code", + Object.assign(new Error("x"), { error_code: "TABLE_OR_VIEW_NOT_FOUND" }), + ], + ["a plain error", new Error("boom")], + ["a non-object", "just a string"], + ])("returns false for %s", (_name, error) => { + expect(isAuthError(error)).toBe(false); + }); }); diff --git a/packages/appkit/src/type-generator/tests/mv-registry.test.ts b/packages/appkit/src/type-generator/tests/mv-registry.test.ts index 49d28fee0..78adefd5a 100644 --- a/packages/appkit/src/type-generator/tests/mv-registry.test.ts +++ b/packages/appkit/src/type-generator/tests/mv-registry.test.ts @@ -1045,12 +1045,13 @@ describe("syncMetrics", () => { }); }); -// ── D′ transience classification: every failure says whether retrying the -// unchanged entry can succeed. ONLY recognized connectivity errors are -// transient (self-converge, retry next pass); everything else — deterministic +// ── D′ transience classification: every failure says whether it should degrade +// rather than fail the build. Recognized connectivity errors (self-converge, +// retry next pass) AND auth/permission errors (a build-time identity gap the +// committed-types gate handles) are transient; everything else — deterministic // warehouse answers (FAILED, zero rows, unparseable payload, zero columns), the // truncation guard, AND unrecognized throws — is non-transient and surfaces as -// a build failure, matching the query path's pessimistic default. +// a build failure, matching the query path's split. describe("syncMetrics — failure transience (D′)", () => { const singleEntryResolution = () => resolveMetricConfig({ @@ -1082,9 +1083,12 @@ describe("syncMetrics — failure transience (D′)", () => { expect(failures[0].transient).toBe(true); }); - test("an auth failure is non-transient (deterministic — must surface)", async () => { - // A 403 / permission error is a real misconfiguration, not a blip: it must - // surface (and fail the build via the caller), never retry forever. + test("an auth failure is transient (build-time identity gap — degrade to committed types)", async () => { + // A 403 / permission error at build time is a build-time identity gap (the + // build runs as a different principal than the app's runtime OBO user), not + // a config error that fails a deploy: it degrades so the committed-types + // gate can reuse types. A fresh checkout with nothing committed still + // crashes via the caller's gate. const fetcher = async (): Promise => { const err = new Error( "PERMISSION_DENIED: cannot access metric view", @@ -1094,7 +1098,21 @@ describe("syncMetrics — failure transience (D′)", () => { }; const { failures } = await syncMetrics(singleEntryResolution(), fetcher); expect(failures).toHaveLength(1); - expect(failures[0].transient).toBe(false); + expect(failures[0].transient).toBe(true); + }); + + test("a PERMISSION_DENIED error_code with no HTTP status is transient", async () => { + // The shape seen on deploy: `{ error_code: "PERMISSION_DENIED", message }` + // with NO numeric status. Status-only detection would miss it and surface a + // fatal; isAuthError reads the error_code so it degrades instead. + const fetcher = async (): Promise => { + throw Object.assign(new Error("2f1a9c…"), { + error_code: "PERMISSION_DENIED", + }); + }; + const { failures } = await syncMetrics(singleEntryResolution(), fetcher); + expect(failures).toHaveLength(1); + expect(failures[0].transient).toBe(true); }); test.each<[string, DatabricksStatementExecutionResponse]>([ From e07eb373f48057b776fd76c866c4e3db300a0866 Mon Sep 17 00:00:00 2001 From: Atila Fassina Date: Tue, 1 Sep 2026 17:53:48 +0200 Subject: [PATCH 2/2] refactor(appkit): typegen degrades DESCRIBE failures by default, deny-list stays fatal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the per-class allowlist at the DESCRIBE reject path with a degrade-by-default policy plus an explicit deny-list. A statement that never ran (connectivity, auth/permission, SDK/config, or any unrecognized throw) now degrades so the has-types gate can reuse committed types; only the deny-list of deterministic client errors — HTTP 404 (bad warehouse id) and 400 (malformed request), via classifyBlockingFailure — stays fatal. This makes the reject path symmetric with preflight and stops the whack-a-mole of enumerating every error shape that lacks a status (the deploy PERMISSION_DENIED had no numeric status). Bad SQL remains a ran-and-failed statement and stays fatal via the syntax-error branch. Applies to both the query and metric-view DESCRIBE paths. Tests that asserted 'unrecognized/auth reject -> fatal' are updated to the new intent; fatal-path coverage switches to deny-list (404/400) errors. isAuthError is retained only to label the degrade cause (accurate 'auth blocked' warning for a statusless PERMISSION_DENIED). Co-authored-by: Isaac Signed-off-by: Atila Fassina --- .../src/type-generator/mv-registry/sync.ts | 22 +++--- .../src/type-generator/mv-registry/types.ts | 9 +-- .../src/type-generator/query-registry.ts | 30 ++++---- .../tests/generate-queries.test.ts | 61 +++++++++-------- .../src/type-generator/tests/index.test.ts | 12 ++-- .../type-generator/tests/mv-registry.test.ts | 68 ++++++++++++------- 6 files changed, 110 insertions(+), 92 deletions(-) diff --git a/packages/appkit/src/type-generator/mv-registry/sync.ts b/packages/appkit/src/type-generator/mv-registry/sync.ts index 43a4242cf..ee520c115 100644 --- a/packages/appkit/src/type-generator/mv-registry/sync.ts +++ b/packages/appkit/src/type-generator/mv-registry/sync.ts @@ -1,8 +1,4 @@ -import { - getErrorDiagnostic, - isAuthError, - isConnectivityError, -} from "../errors"; +import { classifyBlockingFailure, getErrorDiagnostic } from "../errors"; import type { DatabricksStatementExecutionResponse } from "../types"; import { extractMetricColumns, @@ -77,17 +73,17 @@ export async function syncMetrics( response = await fetcher(entry.source); } catch (err) { const reason = `DESCRIBE TABLE EXTENDED failed: ${getErrorDiagnostic(err)}`; - // Connectivity blips self-converge (retry next pass), and a build-time - // auth/permission gap (the build runs as a different principal than the - // app's runtime on-behalf-of user) must not fail a deploy when committed - // types exist — both degrade. A bad warehouse id, a truncated / - // multi-chunk result, or a malformed request are deterministic and must - // surface — the same split the query path makes. + // The DESCRIBE never ran (fetcher threw). Degrade by default (connectivity, + // auth/permission, SDK/config) so the has-types gate can reuse committed + // types; only the deny-list of deterministic client errors (bad warehouse + // id 404, malformed request 400) surfaces as fatal — the same split the + // query path and preflight make. (Truncated/multi-chunk and zero-column + // responses are ran-and-failed, handled below and kept non-transient.) return failedOutcome( index, entry, reason, - isConnectivityError(err) || isAuthError(err), + classifyBlockingFailure(err) !== "deterministic", ); } @@ -151,7 +147,7 @@ export async function syncMetrics( index, entry, `DESCRIBE TABLE EXTENDED failed: ${getErrorDiagnostic(result.reason)}`, - isConnectivityError(result.reason) || isAuthError(result.reason), + classifyBlockingFailure(result.reason) !== "deterministic", ); schemas[index] = schema; failureSlots[index] = failure; diff --git a/packages/appkit/src/type-generator/mv-registry/types.ts b/packages/appkit/src/type-generator/mv-registry/types.ts index ffd07be57..d12e004bd 100644 --- a/packages/appkit/src/type-generator/mv-registry/types.ts +++ b/packages/appkit/src/type-generator/mv-registry/types.ts @@ -128,10 +128,11 @@ export interface MetricSyncFailure { /** Single human-readable reason (DESCRIBE failed, parse failed, zero columns). */ reason: string; /** - * Whether the failure should degrade rather than fail the build: a - * connectivity blip that self-converges on a later pass, or a build-time - * auth/permission gap that the committed-types gate handles. Deterministic - * failures (bad id, malformed request, zero columns) are not transient. + * Whether the failure should degrade rather than fail the build. True for any + * DESCRIBE that never ran (connectivity, auth/permission, SDK/config) — the + * has-types gate reuses committed types. False for the deny-list of + * deterministic client errors (bad warehouse id 404, malformed request 400) + * and ran-and-failed responses (unparseable payload, zero columns). */ transient: boolean; } diff --git a/packages/appkit/src/type-generator/query-registry.ts b/packages/appkit/src/type-generator/query-registry.ts index df8f95217..db9cf57b9 100644 --- a/packages/appkit/src/type-generator/query-registry.ts +++ b/packages/appkit/src/type-generator/query-registry.ts @@ -11,7 +11,6 @@ import { classifyBlockingFailure, classifyEnvironmentalCause, getErrorDiagnostic, - isAuthError, isConnectivityError, } from "./errors"; import { decidePreflight, type PreflightMode } from "./preflight"; @@ -955,9 +954,12 @@ export async function generateQueriesFromDescribe( } } else { // executeStatement rejected without a normal StatementExecution result. - // Only structured transport/connectivity failures are treated as - // offline; auth, bad warehouse IDs, malformed requests, and SDK/config - // failures stay fatal so users fix the underlying setup issue. + // executeStatement rejected — the statement never ran. Degrade by + // default (connectivity, auth/permission, SDK/config); the has-types + // gate reuses committed types, or crashes a fresh checkout with + // nothing to fall back to. Only the deny-list below — deterministic + // client errors — stays fatal. (Bad SQL is a *ran-and-failed* + // statement, handled by the syntax-error branch above, not here.) completed++; spinner.update( `Describing ${total} ${total === 1 ? "query" : "queries"} (${completed}/${total})`, @@ -975,12 +977,9 @@ export async function generateQueriesFromDescribe( schema: { name: queryName, ...degraded }, }); - if ( - !isConnectivityError(entry.reason) && - !isAuthError(entry.reason) - ) { - // Bad SQL, a bad/missing warehouse id (404), and malformed - // requests (400) stay fatal so users fix the underlying setup. + if (classifyBlockingFailure(entry.reason) === "deterministic") { + // Deny-list: a bad/typo'd warehouse id (404) or a malformed + // request (400) is a config error — surface it so users fix it. fatalErrors.push({ name: queryName, message: error.message }); logEntries.push({ queryName, @@ -991,11 +990,12 @@ export async function generateQueriesFromDescribe( continue; } - // Environmental for the same reason as the preflight branch above, - // so the has-types gate still sees it: a connectivity blip - // self-converges, and a build-time auth/permission gap (the build - // runs as a different principal than the app's runtime on-behalf-of - // user) must not fail a deploy when committed types exist. + // Not on the deny-list: degrade so the has-types gate can reuse + // committed types, exactly as the preflight branch above does. This + // covers connectivity blips and build-time auth/permission gaps (the + // build runs as a different principal than the app's runtime + // on-behalf-of user), which must not fail a deploy when committed + // types exist. if (mode === "blocking") { hadEnvironmentalFailure = true; environmentalCause = 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 3a5d3e9a4..9117cbcb9 100644 --- a/packages/appkit/src/type-generator/tests/generate-queries.test.ts +++ b/packages/appkit/src/type-generator/tests/generate-queries.test.ts @@ -394,7 +394,7 @@ describe("generateQueriesFromDescribe", () => { }); }); - test("fatal rejected DESCRIBE request is not downgraded to offline", async () => { + test("an auth-flavoured rejected DESCRIBE degrades to unknown (not on the deny-list)", async () => { mocks.readdir.mockResolvedValue(["users.sql"]); mocks.readFile.mockResolvedValue("SELECT id FROM users"); mocks.executeStatement.mockRejectedValueOnce( @@ -406,13 +406,11 @@ describe("generateQueriesFromDescribe", () => { "wh-123", ); + // A build-time permission gap is not on the deny-list (404/400): the DESCRIBE + // degrades to `unknown` and the has-types gate decides — it does not fail the + // build here. Never cached. expect(schemas[0].type).toContain("result: unknown"); - expect(fatalErrors).toEqual([ - { - name: "users", - message: "PERMISSION_DENIED: missing warehouse permission", - }, - ]); + expect(fatalErrors).toEqual([]); expect(mocks.saveCache).toHaveBeenCalledTimes(1); expect(lastSavedQueries()).not.toHaveProperty("users"); }); @@ -463,11 +461,11 @@ describe("generateQueriesFromDescribe", () => { expect(fatalErrors).toEqual([]); }); - test("mixed syntax and fatal failures are both returned", async () => { - mocks.readdir.mockResolvedValue(["syntax.sql", "fatal.sql"]); + test("mixed syntax and deny-list failures are both returned", async () => { + mocks.readdir.mockResolvedValue(["syntax.sql", "bad_id.sql"]); mocks.readFile .mockResolvedValueOnce("SELECT * FROM missing") - .mockResolvedValueOnce("SELECT * FROM auth_blocked"); + .mockResolvedValueOnce("SELECT * FROM whatever"); mocks.executeStatement .mockResolvedValueOnce({ statement_id: "stmt-syntax", @@ -476,7 +474,11 @@ describe("generateQueriesFromDescribe", () => { error: { message: "Table not found" }, }, }) - .mockRejectedValueOnce(new Error("PERMISSION_DENIED")); + // A 404 is on the deny-list — it stays fatal (bad SQL is a ran-and-failed + // FAILED statement, above; this is a deterministic client error). + .mockRejectedValueOnce( + Object.assign(new Error("warehouse wh-123 not found"), { status: 404 }), + ); const { schemas, syntaxErrors, fatalErrors } = await describeQueries( "/queries", @@ -488,7 +490,7 @@ describe("generateQueriesFromDescribe", () => { { name: "syntax", message: "Table not found" }, ]); expect(fatalErrors).toEqual([ - { name: "fatal", message: "PERMISSION_DENIED" }, + { name: "bad_id", message: "warehouse wh-123 not found" }, ]); }); @@ -543,7 +545,13 @@ describe("generateQueriesFromDescribe", () => { expect(fatalErrors).toEqual([]); }); - test("bare timeout and fetch failed messages are not overmatched as connectivity", async () => { + test("non-deny-list rejections (param error, expired token) degrade, not fatal", async () => { + // Neither carries an HTTP 404/400, so under degrade-by-default both fall off + // the deny-list and degrade to `unknown` rather than failing the build. (A + // malformed request that surfaces a real 400 status is still fatal — see the + // deny-list tests. Catching message-only deterministic errors like + // INVALID_PARAMETER_VALUE would require expanding the deny-list beyond + // status; deliberately out of scope.) mocks.readdir.mockResolvedValue(["timeout.sql", "oauth.sql"]); mocks.readFile .mockResolvedValueOnce("SELECT id FROM timeout") @@ -564,26 +572,21 @@ describe("generateQueriesFromDescribe", () => { ); expect(schemas).toHaveLength(2); - expect(fatalErrors).toEqual([ - { - name: "timeout", - message: "INVALID_PARAMETER_VALUE: timeout must be > 0", - }, - { - name: "oauth", - message: "fetch failed: token expired: EXPIRED_OAUTH_TOKEN", - }, - ]); + expect(schemas[0].type).toContain("result: unknown"); + expect(schemas[1].type).toContain("result: unknown"); + expect(fatalErrors).toEqual([]); }); - test("successful describes in a fatal batch are saved", async () => { - mocks.readdir.mockResolvedValue(["good.sql", "bad_auth.sql"]); + test("successful describes in a deny-list-fatal batch are saved", async () => { + mocks.readdir.mockResolvedValue(["good.sql", "bad_id.sql"]); mocks.readFile .mockResolvedValueOnce("SELECT id FROM good") - .mockResolvedValueOnce("SELECT id FROM bad_auth"); + .mockResolvedValueOnce("SELECT id FROM bad_id"); mocks.executeStatement .mockResolvedValueOnce(succeededResult([["id", "INT", null]])) - .mockRejectedValueOnce(new Error("PERMISSION_DENIED")); + .mockRejectedValueOnce( + Object.assign(new Error("malformed request"), { status: 400 }), + ); const { schemas, fatalErrors } = await describeQueries( "/queries", @@ -593,10 +596,10 @@ describe("generateQueriesFromDescribe", () => { expect(schemas[0].type).toContain("id: number"); expect(schemas[1].type).toContain("result: unknown"); expect(fatalErrors).toEqual([ - { name: "bad_auth", message: "PERMISSION_DENIED" }, + { name: "bad_id", message: "malformed request" }, ]); expect(lastSavedQueries()?.good.type).toContain("id: number"); - expect(lastSavedQueries()).not.toHaveProperty("bad_auth"); + expect(lastSavedQueries()).not.toHaveProperty("bad_id"); }); test("empty result (described, no columns) is unknown, not a syntax error, not cached", async () => { diff --git a/packages/appkit/src/type-generator/tests/index.test.ts b/packages/appkit/src/type-generator/tests/index.test.ts index 64edcf358..21b5a7e0c 100644 --- a/packages/appkit/src/type-generator/tests/index.test.ts +++ b/packages/appkit/src/type-generator/tests/index.test.ts @@ -602,19 +602,21 @@ describe("generateFromEntryPoint — metric-view emission", () => { ); }); - test("blocking + a per-key DESCRIBE failure: escalates to a build failure (TypegenFatalError)", async () => { + test("blocking + a per-key deny-list DESCRIBE failure: escalates to a build failure (TypegenFatalError)", async () => { writeMetricConfig(); - // An injected fetcher always runs and bypasses preflight; throwing makes the - // key a deterministic DESCRIBE failure. Non-blocking only warns (covered - // above) — but `--wait` promised correct types, so it must fail the build. + // An injected fetcher always runs and bypasses preflight; throwing a + // deny-list error (400 malformed request) makes the key a deterministic + // DESCRIBE failure. Non-blocking only warns (covered above) — but `--wait` + // promised correct types, so a deny-list failure must fail the build. + // (A non-deny-list throw would instead degrade — see the transient case.) const error = await generateFromEntryPoint({ outFile, queryFolder, warehouseId: "wh-1", mode: "blocking", metricFetcher: async () => { - throw new Error("DESCRIBE exploded"); + throw Object.assign(new Error("DESCRIBE exploded"), { status: 400 }); }, }).then( () => { diff --git a/packages/appkit/src/type-generator/tests/mv-registry.test.ts b/packages/appkit/src/type-generator/tests/mv-registry.test.ts index 78adefd5a..90b07185c 100644 --- a/packages/appkit/src/type-generator/tests/mv-registry.test.ts +++ b/packages/appkit/src/type-generator/tests/mv-registry.test.ts @@ -995,10 +995,10 @@ describe("syncMetrics", () => { expect(failures[0]).toMatchObject({ key: "revenue", source: "demo.public.revenue", - // A throw with no recognizable connectivity signal is treated as - // deterministic (transient: false) — surfaced, not silently retried. The - // point of this test is that it is RECORDED, never an uncaught crash. - transient: false, + // A throw that carries no 404/400 is not on the deny-list, so it degrades + // (transient: true) and the has-types gate decides. The point of this test + // is that it is RECORDED, never an uncaught crash. + transient: true, }); expect(failures[0].reason).toMatch(/warehouse unreachable/); }); @@ -1036,22 +1036,25 @@ describe("syncMetrics", () => { expect(failures[0]).toMatchObject({ key: "revenue", source: "demo.public.revenue", - // Truncation is deterministic — re-describing the unchanged entry yields - // the same multi-chunk result — so it is non-transient (sticky/fatal), - // never a retryable blip. The point of the test: RECORDED, not a crash. - transient: false, + // The truncation guard throws inside the fetcher, so it lands on the + // "never ran" reject path and degrades by default (transient: true) — it + // carries no 404/400, so it is off the deny-list. Types stay stable via + // the has-types gate; nothing partial is ever emitted. The point of the + // test: RECORDED, not a crash. + transient: true, }); expect(failures[0].reason).toMatch(/multi-chunk/i); }); }); // ── D′ transience classification: every failure says whether it should degrade -// rather than fail the build. Recognized connectivity errors (self-converge, -// retry next pass) AND auth/permission errors (a build-time identity gap the -// committed-types gate handles) are transient; everything else — deterministic -// warehouse answers (FAILED, zero rows, unparseable payload, zero columns), the -// truncation guard, AND unrecognized throws — is non-transient and surfaces as -// a build failure, matching the query path's split. +// rather than fail the build. A DESCRIBE that never ran degrades by default +// (connectivity, auth/permission, SDK/config, and unrecognized throws) — the +// has-types gate reuses committed types. Only the deny-list of deterministic +// client errors (bad warehouse id 404, malformed request 400) and ran-and-failed +// responses (FAILED, zero rows, unparseable payload, zero columns, the +// truncation guard) are non-transient and surface as a build failure — the same +// split the query path and preflight make. describe("syncMetrics — failure transience (D′)", () => { const singleEntryResolution = () => resolveMetricConfig({ @@ -1083,12 +1086,12 @@ describe("syncMetrics — failure transience (D′)", () => { expect(failures[0].transient).toBe(true); }); - test("an auth failure is transient (build-time identity gap — degrade to committed types)", async () => { - // A 403 / permission error at build time is a build-time identity gap (the - // build runs as a different principal than the app's runtime OBO user), not - // a config error that fails a deploy: it degrades so the committed-types - // gate can reuse types. A fresh checkout with nothing committed still - // crashes via the caller's gate. + test("an auth failure (403) is transient — not on the deny-list, so it degrades", async () => { + // A 403 at build time is a build-time identity gap (the build runs as a + // different principal than the app's runtime OBO user), not a config error + // that fails a deploy. It isn't a deterministic 404/400, so it degrades and + // the committed-types gate reuses types. A fresh checkout with nothing + // committed still crashes via the caller's gate. const fetcher = async (): Promise => { const err = new Error( "PERMISSION_DENIED: cannot access metric view", @@ -1103,8 +1106,9 @@ describe("syncMetrics — failure transience (D′)", () => { test("a PERMISSION_DENIED error_code with no HTTP status is transient", async () => { // The shape seen on deploy: `{ error_code: "PERMISSION_DENIED", message }` - // with NO numeric status. Status-only detection would miss it and surface a - // fatal; isAuthError reads the error_code so it degrades instead. + // with NO numeric status. It carries no 404/400, so it is not on the + // deny-list and degrades (the build no longer has to recognize the error + // class — only the deny-list stays fatal). const fetcher = async (): Promise => { throw Object.assign(new Error("2f1a9c…"), { error_code: "PERMISSION_DENIED", @@ -1115,6 +1119,17 @@ describe("syncMetrics — failure transience (D′)", () => { expect(failures[0].transient).toBe(true); }); + test("a deny-list error (404 bad warehouse id) is non-transient — surfaces", async () => { + // The deny-list: a bad/typo'd warehouse id (404) or malformed request (400) + // is a config error the build should surface, not mask with committed types. + const fetcher = async (): Promise => { + throw Object.assign(new Error("warehouse not found"), { status: 404 }); + }; + const { failures } = await syncMetrics(singleEntryResolution(), fetcher); + expect(failures).toHaveLength(1); + expect(failures[0].transient).toBe(false); + }); + test.each<[string, DatabricksStatementExecutionResponse]>([ [ "a FAILED statement", @@ -1147,11 +1162,12 @@ describe("syncMetrics — failure transience (D′)", () => { expect(failures[0].transient).toBe(false); }); - test("a defensive rejected settlement is non-transient (unknown cause — surface, don't loop)", async () => { + test("a defensive rejected settlement is transient (unknown cause — degrade, has-types gate decides)", async () => { // Same poisoned-response trick as the scheduling suite: blow up after // the fetch try/catch so the settlement itself rejects. An unknown internal - // failure carries no connectivity signal, so it is surfaced (deterministic) - // rather than retried forever — the pessimistic default matching the query path. + // failure carries no 404/400, so it is not on the deny-list: it degrades and + // the has-types gate arbitrates (reuse committed types, or crash a fresh + // checkout) rather than unconditionally failing the build. const poisoned = new Proxy({} as DatabricksStatementExecutionResponse, { get(_target, prop) { if (prop === "then") { @@ -1163,7 +1179,7 @@ describe("syncMetrics — failure transience (D′)", () => { const fetcher = async () => poisoned; const { failures } = await syncMetrics(singleEntryResolution(), fetcher); expect(failures).toHaveLength(1); - expect(failures[0].transient).toBe(false); + expect(failures[0].transient).toBe(true); }); });