fix(appkit): don't crash typegen when the warehouse is unreachable#406
Draft
atilafassina wants to merge 2 commits into
Draft
fix(appkit): don't crash typegen when the warehouse is unreachable#406atilafassina wants to merge 2 commits into
atilafassina wants to merge 2 commits into
Conversation
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 <atila@fassina.eu>
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 <atila@fassina.eu>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Type generation crashed with an uncaught error whenever the SQL warehouse was unavailable. When the warehouse is down, every
DESCRIBE QUERYfails; those per-query failures are caught (Promise.allSettled→result: unknown), butgenerateFromEntryPointthen unconditionally threw an aggregate"Type generation failed"error. That throw escaped uncaught at both call sites — the Vite plugin (un-awaitedgenerate()) and the CLI (synccmd.parse()) — taking down the dev server / build / CLI (on Node 20 an unhandled rejection is fatal).The aggregate throw was introduced in
f2d44ae7(#254) to fail builds on genuinely-broken SQL, but it could not tell "one query has a syntax error against a reachable warehouse" from "the whole warehouse is down, so every query failed."Fix (PR 1 of 2 — user-visible behavior)
Distinguish the two failure classes; only a genuine SQL error is treated as fatal.
unknown; warn; exit 0DESCRIBE … FAILED)unknown; warn; exit 0Changes
query-registry.ts— classify each describe outcome:ok(cached),syntax(state === "FAILED"→ collected, not cached),connectivity(executeStatementrejects → reuse last-known-good cached type orunknown; never cached, never fatal),empty(described, no columns → softunknown, not cached). The console table reports the classes distinctly.index.ts— replaced the unconditional throw with a typedTypegenSyntaxErrorthrown only for syntax errors, and after the.d.tsis always written. Connectivity failures degrade silently.types.ts— addedQuerySyntaxError+QueryGenerationResult({ schemas, syntaxErrors }).result: unknown— 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.Verification
pnpm -r typecheck— clean across all 7 projectsvite-plugin.test.tsmirroring the serving one.Deferred to PR 2 (hardening + tests)
awaitthe VitebuildStart/ watcher and useparseAsync().catch()in the CLI, so a prod syntax error fails the build cleanly rather than via an unhandled rejection. (The warehouse-down crash is fully fixed in this PR — dev and prod; this is the remaining cosmetic path for genuine SQL errors.)This pull request and its description were written by Isaac.