Repository navigation
fix(appkit): fall back to inline results when a Cloud Fetch download is refused - #723
Open
pffigueiredo wants to merge 9 commits into
Open
pffigueiredo wants to merge 9 commits into
pffigueiredo wants to merge 9 commits into
Conversation
Charts on the Arrow path fail with HTTP 500 when workspace storage refuses the app server's Cloud Fetch download, and hang until the route's 120 s first-byte timer fires when storage never answers (LA-2169). Add tests for the behavior the fix needs, all failing against v0.91.0: - Processor: link headers are sent, a 400 is not retried, a 403 re-mints the link once, a wait for headers is bounded and counts as a denial, every download failure carries EXTERNAL_LINK_DOWNLOAD_FAILED. - Delivery: a failed first chunk is replaced by an inline JSON_ARRAY run rebuilt as Arrow, and a result too large for inline is never stranded. - Route: over real HTTP against a loopback storage host, charts render, a definite refusal is remembered for ten minutes, and errors say why. - Rebuild: Arrow rebuilt from positional rows matches the native schema and values for every scalar type, including nulls. Signed-off-by: Pedro Figueiredo <pedro.figueiredo@databricks.com>
…is refused A chart on the Arrow path asks for ARROW_STREAM, which a standard SQL warehouse serves only as EXTERNAL_LINKS. When workspace storage refused the app server's download, the route made three attempts and returned HTTP 500 with "Query execution failed". When storage never answered, the route's 120 s first-byte timer returned a 503 that blamed the warehouse (LA-2169). Processor - Send the link's httpHeaders with every GET, and the new link's headers after a re-mint. - Wait at most responseTimeoutMs (10 s) for response headers only. A timeout is not retried. Body reads keep the per-read idle timeout, so a healthy chunk may stream for longer. - A 403 re-mints the link once and retries at once. Any other 4xx except 408 and 429 stops. Network errors and 5xx keep the 1 s and 2 s backoff. - Every download failure, before or after the headers, carries errorCode EXTERNAL_LINK_DOWNLOAD_FAILED and the last HTTP status. Delivery - deliverArrowBytesWithFallback pulls the first chunk before the route sends headers. On a failure before the first byte it runs the query as INLINE + JSON_ARRAY and rebuilds the Arrow IPC stream from the positional rows and manifest columns, typed by parseDatabricksType. - Only a 400 or 403 after the re-mint is remembered as "links-denied", for ten minutes. A timeout, network error, or body failure falls back once and is forgotten. - A result the inline run cannot hold (manifest byte count over 25 MiB, or a multi-chunk inline response) is never served truncated. After an inconclusive failure Cloud Fetch gets one patient pass. After a refusal the error is EXTERNAL_LINKS_DENIED and says why. - Any other inline failure is rethrown unchanged. A rebuild failure is ARROW_REBUILD_FAILED. Connector - executeStatement accepts rawRows to keep dataArray and carry the manifest columns on the result. EXTERNAL_LINKS results now carry totalByteCount. The inline fallback run is cached like the inline Arrow attempt, under its own key. @databricks/appkit-ui is unchanged. Signed-off-by: Pedro Figueiredo <pedro.figueiredo@databricks.com>
Builder pods run with internet off and no SEG, so a download straight to cloud storage hangs instead of refusing. Remember that as links-denied once the inline fallback has served the result, so later charts skip the wait. Signed-off-by: Pedro Figueiredo <pedro.figueiredo@databricks.com>
Contributor
📦 Bundle size reportCompared against
|
| dist | raw | gzip |
|---|---|---|
| JS (runtime) | 1.3 MB (+14 KB) | 467 KB (+4.7 KB) |
| Type declarations | 465 KB (+252 B) | 169 KB (+103 B) |
| Source maps | 2.6 MB (+26 KB) | 871 KB (+7.9 KB) |
| Other | 11 KB | 3.7 KB |
| Total | 4.3 MB (+40 KB) | 1.5 MB (+13 KB) |
Per-entry composition (own code — deps external (as shipped))
| Entry | Initial (gz) | Lazy (gz) | Total (gz) | node_modules (min) | Own code (min) |
|---|---|---|---|---|---|
. |
110 KB (+2.4 KB) | 2.5 KB | 112 KB (+2.4 KB) | external | 353 KB (+6.1 KB) |
./beta |
105 KB (+1.2 KB) | 484 B | 105 KB (+1.2 KB) | external | 314 KB (+2.9 KB) |
./testing |
46 KB | 33 KB | 79 KB | external | 228 KB |
./tsdown |
520 B | 0 B | 520 B | external | 813 B |
./type-generator |
26 KB | 0 B | 26 KB | external | 74 KB |
Chunks:
| Entry | Chunk | Load | Size (gz) |
|---|---|---|---|
. |
index.js |
initial | 105 KB |
. |
utils.js |
initial | 4.6 KB |
. |
remote-tunnel-manager.js |
lazy | 2.5 KB |
./beta |
beta.js |
initial | 84 KB |
./beta |
modular.js |
initial | 6.0 KB |
./beta |
stream-manager.js |
initial | 5.9 KB |
./beta |
service-context.js |
initial | 4.3 KB |
./beta |
databricks.js |
initial | 3.3 KB |
./beta |
client.js |
initial | 646 B |
./beta |
index.js |
initial | 20 B |
./beta |
supervisor-api.js |
lazy | 192 B |
./beta |
databricks.js |
lazy | 177 B |
./beta |
index.js |
lazy | 115 B |
./testing |
manifest.js |
initial | 33 KB |
./testing |
index.js |
initial | 10 KB |
./testing |
wide-event-emitter.js |
initial | 2.9 KB |
./testing |
index.js |
lazy | 29 KB |
./testing |
remote-tunnel-manager.js |
lazy | 2.5 KB |
./testing |
utils.js |
lazy | 1.8 KB |
./tsdown |
index.js |
initial | 520 B |
./type-generator |
index.js |
initial | 26 KB |
@databricks/appkit-ui
npm tarball (packed): 397 KB (-4 B) — gzipped download (dist + bin; excludes release-only docs/NOTICE).
| dist | raw | gzip |
|---|---|---|
| JS (runtime) | 429 KB | 147 KB |
| Type declarations | 267 KB | 98 KB (-1 B) |
| Source maps | 848 KB | 285 KB |
| CSS | 16 KB | 3.2 KB |
| Total | 1.5 MB | 533 KB (-1 B) |
Per-entry composition (consumer bundle — deps bundled, peerDeps external)
| Entry | Initial (gz) | Lazy (gz) | Total (gz) | node_modules (min) | Own code (min) |
|---|---|---|---|---|---|
./js |
5.3 KB | 49 KB | 55 KB | 208 KB | 14 KB |
./js/beta |
2.1 KB | 0 B | 2.1 KB | 0 B | 5.0 KB |
./react |
434 KB | 49 KB | 483 KB | 1.3 MB | 177 KB |
./react/beta |
4.9 KB | 0 B | 4.9 KB | 0 B | 12 KB |
Chunks:
| Entry | Chunk | Load | Size (gz) |
|---|---|---|---|
./js |
index.js |
initial | 5.2 KB |
./js |
chunk |
initial | 120 B |
./js |
apache-arrow |
lazy | 49 KB |
./js/beta |
beta.js |
initial | 2.1 KB |
./react |
index.js |
initial | 432 KB |
./react |
tslib |
initial | 2.1 KB |
./react |
apache-arrow |
lazy | 49 KB |
./react/beta |
beta.js |
initial | 4.9 KB |
Contributor
🤖 AppKit PR bot🔬 Run evalsStart an eval for this PR from the evals-monitor app: Go to Evals Monitor → 📦 Try this PR's app templateScaffolds a new app from this PR's SDK build. Run it in any folder (requires the GitHub CLI — gh run download 42809505078 -R databricks/appkit -n appkit-template-0.91.2-pr.3312f29-fix-la-2169-cloud-fetch-fallback-723 -D appkit-pr-723 \
&& unzip -o "appkit-pr-723/appkit-template-0.91.2-pr.3312f29-fix-la-2169-cloud-fetch-fallback-723.zip" -d "appkit-pr-723" \
&& databricks apps init --template "appkit-pr-723"The template pins |
…odec table Each scalar type's JSON_ARRAY parser now sits next to its formatter in json-array-cells.ts. The formatters move there unchanged from result-delivery.ts. Cells that don't fit their type now throw instead of passing through altered: a DECIMAL with more fraction digits than its scale (was silently cut), a TIMESTAMP without an offset, and a TIMESTAMP_NTZ with one. The rebuilt stream names its columns like the native Cloud Fetch stream, duplicates included. The browser skips relabeling when names repeat, so col_N names reached the user. buildArrowIPCFromRows takes the SDK's ColumnInfo. tests/fixtures/typed-values.json is a captured warehouse response, with the JSON_ARRAY text and the native Arrow stream, that pins the formats. Signed-off-by: Pedro Figueiredo <pedro.figueiredo@databricks.com>
…169-cloud-fetch-fallback Signed-off-by: Pedro Figueiredo <pedro.figueiredo@databricks.com>
The template lockfiles from the v0.91.2 sync pin versions that Databricks' npm proxies refuse with 403: import-in-the-middle 3.6.0, semantic-conventions 1.45.0, ansi-regex 6.6.0, and the OpenTelemetry 0.223.1 / 2.12.1 patch release. Every Template Deploy-Shape and PR Template Artifact run failed on install. Pin import-in-the-middle to 3.5.3 and semantic-conventions to 1.44.0 under the bunyan and pino instrumentations, which need ^1.41.1. Re-resolve the other blocked npm lock entries, which lands on the allowed 0.223.0 / 2.12.0 and ansi-regex 6.5.0. Signed-off-by: Pedro Figueiredo <pedro.figueiredo@databricks.com>
…e.yaml tools/check-template-deps.ts requires identical override keys in both files, and tools/check-template-artifact.ts expects string specifiers. Replace the scoped semantic-conventions pins with one flat 1.44.0 pin, and pin import-in-the-middle 3.5.3 for pnpm too. Both lockfiles stay clear of the versions the npm security policy blocks. Signed-off-by: Pedro Figueiredo <pedro.figueiredo@databricks.com>
…dant Null branch Shorten docblocks to their contracts, drop a JSDoc that repeated the TTL note, and fix the TTL comment that still said only refusals are remembered. vectorFromArray takes the rebuilt type without a cast, and an all-null VOID column needs no special case. Signed-off-by: Pedro Figueiredo <pedro.figueiredo@databricks.com>
…parsing Review follow-ups for LA-2169: - With the links-denied hint, an inline failure that isn't about size is rethrown at once instead of running the query again over Cloud Fetch. - EXTERNAL_LINKS_DENIED only when storage refused or never answered; any other failure of the full-header-wait retry keeps its own error. A download that never answered is remembered as links-denied even when the retry then fails another way. - The client message is cloud-neutral, and each switch to inline is logged. - Cells parse strictly: decimal or scientific floats, base64 binary, real calendar dates, and decimals within their precision. - Unnamed rebuilt columns are column_N, matching the X-Appkit-Arrow-Columns header. - downloadFailureKind() sits next to the error it reads, one first-chunk helper replaces two copies, the capability memo keeps its expiry in the same map, and `patient` is now `fullHeaderWait`. - The typed-values fixture is re-captured with nulls in every column. Signed-off-by: Pedro Figueiredo <pedro.figueiredo@databricks.com>
This branch has not been deployed
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
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.
Why
A chart on the Arrow path fails with HTTP 500 "Query execution failed" when workspace storage refuses the app server's Cloud Fetch download. This happens, for example, when the storage account's firewall doesn't allow the serverless network the app runs on (Jira LA-2169). AppKit makes three attempts and has no fallback. On pods with internet off, a blocked host can also hang until the route's 120 s timer returns a 503 that blames the warehouse. This PR makes the chart render in both cases. Healthy workspaces get the same bytes and the same executions as before.
What changed
The chunk downloader sends each link's
httpHeaders. It waits at most 10 s for response headers and keeps the per-read idle timeout for the body. On a 403 it re-mints the link once with no wait. On any other 4xx except 408 and 429 it stops. Every failure carriesEXTERNAL_LINK_DOWNLOAD_FAILED.deliverArrowBytesWithFallbackwrapsdeliverArrowBytes. If the download fails before the first byte, the wrapper runs the query asINLINE+JSON_ARRAYand rebuilds the same Arrow IPC stream from the raw rows, with the native column names andparseDatabricksTypetypes. Each cell is read through one table injson-array-cells.ts, which pairs every type's parser with the formatter the Reyden fallback already used. A cell that doesn't fit its type throws, including a date that isn't on the calendar or a decimal over its precision. The route and@databricks/appkit-uidon't change.A refusal (400 or 403 after the re-mint), or a download that never answers, records
"links-denied"for the warehouse for 10 minutes, so later queries skip the doomed download. A missing answer is recorded only after the inline result has been served, or after the retry described under Tradeoffs also fails. With that record in place, the inline run goes first, and an inline failure that isn't about size is returned at once instead of running the query again. Every switch to inline is logged as a warning.The template lockfiles move off npm versions that Databricks' npm proxies refuse:
import-in-the-middle3.6.0,@opentelemetry/semantic-conventions1.45.0,ansi-regex6.6.0, and the OpenTelemetry 0.223.1 / 2.12.1 patch release. Since the v0.91.2 template sync, every template check failed on install for any PR. The template now pinsimport-in-the-middle3.5.3 and@opentelemetry/semantic-conventions1.44.0 in bothpackage.jsonandpnpm-workspace.yaml. That includes AppKit's own 1.38.0 dependency, and the package holds only constants. Both template smoke tests pass locally through the proxy.Scope
EXTERNAL_LINKS_DENIEDand a message that names the storage download. That code is used only when storage refused or never answered; any other failure keeps its own error.UTCinstead of the warehouse session time zone. The instants are identical, and@databricks/appkit-uidoesn't read the label.Tradeoffs
Blast radius
The change is limited to the
ARROW_STREAMroute of the analytics plugin and the chunk downloader. The JSON path, metric views, and the browser package don't change. Every new branch runs only after a download has failed. Before this PR, that path returned a 500.Verification
tests/fixtures/typed-values.jsonis a captured warehouse response with 17 columns and nulls in every column: theJSON_ARRAYtext next to the native Arrow stream. The rebuild matches the native stream in 14 columns, names and nulls included. Tests assert the other 3 as the documented limits. Every codec also has a round-trip test and rejection cases.vitest run packages/appkit: 4369 passed, 1 skipped.pnpm --filter @databricks/appkit typecheckandpnpm checkare clean.template/and viewed in headless Chrome. A preload answered the storage downloads with 403, 400, or no response. With 0.89.0, the twoformat="arrow_stream"charts showed "Error loading chart: Query execution failed", and a hung download returned 503 after 120 s. With this branch, every chart rendered: 2.8 s on a 403 and 12.9 s on a hang, with 7 ms on the next load in both cases. Healthy storage still used Cloud Fetch. Across 17 columns of real data, the rebuild matched native Arrow in 14. The other 3 are the documented limits above.