Skip to content

Add gRPC-to-HTTP query fallback, fix test spicepod version, strengthen browser test assertions - #316

Open
lukekim wants to merge 6 commits into
trunkfrom
test/browser-client-request-assertions
Open

Add gRPC-to-HTTP query fallback, fix test spicepod version, strengthen browser test assertions#316
lukekim wants to merge 6 commits into
trunkfrom
test/browser-client-request-assertions

Conversation

@lukekim

@lukekim lukekim commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Four changes, from most to least significant:

1. SDK: fall back to HTTP when an Arrow Flight query fails mid-query

The Flight channel can initialize successfully but then fail on the query itself — observed as empty grpc-js status errors (undefined undefined: undefined) from the SDK running inside Vercel serverless functions, which made all Cloud Tests fail with 500s from the test app. Previously doQueryRequest only fell back to HTTP when channel initialization failed.

Now sql() falls back to HTTP on a failed Flight query, unless:

  • flightOnly is set (no HTTP transport to fall back to), or
  • partial results were already streamed to the caller's callback (fallback would deliver duplicate data).

sqlJson() in gRPC mode routes through sql() and inherits the same behavior. Falling back logs a warning through the configurable logger.

2. SDK: send raw SQL over HTTP unless parameters require the JSON envelope

Deploying fix 1 exposed a second bug: Spice Cloud's /v1/sql parses every request body as raw SQL and rejects the {sql, parameters} JSON envelope (introduced in #268) with 400 ParserError. The OSS runtime accepts both formats (verified against the runtime source and a local spiced).

  • Queries without parameters are sent as text/plain raw SQL — the format every endpoint accepts, and the pre-Add Parameterized Queries #268 behavior.
  • Parameterized queries keep the JSON envelope against the OSS runtime.
  • Parameterized queries over HTTP against Spice Cloud fail fast with a clear error (parameter binding there requires Arrow Flight) instead of a cryptic server 400.

New unit tests in test/grpc-http-fallback.test.ts cover all paths (6 tests), wired into the Local Runtime CI job.

3. CI: fix test spicepod version (v1beta1v1)

CI installs the latest spiced release, which dropped support for v1beta1 spicepods — the runtime failed to start and all six Local Runtime jobs failed with ECONNREFUSED. Validated against a local spiced before pushing.

4. Tests: strengthen browser client assertions for HTTP request formats

  • Plain sql() asserts the text/plain raw-SQL body; a new test asserts the JSON envelope for parameterized queries.
  • sqlJson() test asserts Content-Type: text/plain with the raw SQL body and the application/vnd.spiceai.sql.v1+json Accept header.
  • The config-logging test now sets SPICE_DEBUG=true (restored after), making it deterministic regardless of NODE_ENV.

Testing

  • test/grpc-http-fallback.test.ts — 6 tests pass
  • npm run test:browser — 62 tests pass
  • Both body formats verified 200 OK against a local OSS spiced; the envelope-vs-raw behavior of Spice Cloud confirmed from CI logs (ParserError on {)
  • Prettier clean; eslint delta on changed files is zero (repo-wide lint issues are pre-existing)

@vercel

vercel Bot commented Jul 30, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
spice-js Ready Ready Preview Aug 1, 2026 6:42pm

Request Review

…eamed

The Flight channel can initialize successfully but then fail mid-query
(observed as empty grpc-js status errors in serverless environments,
e.g. Vercel functions). Previously doQueryRequest only fell back to HTTP
when channel initialization failed, so these errors surfaced to callers.

Now sql() falls back to HTTP on a failed Flight query unless flightOnly
is set or partial results were already streamed to the caller's callback
(which would deliver duplicate data). sqlJson() in gRPC mode routes
through sql() and inherits the same behavior.
@lukekim lukekim changed the title Strengthen browser client test assertions for HTTP request formats Add gRPC-to-HTTP query fallback, fix test spicepod version, strengthen browser test assertions Jul 30, 2026
Spice Cloud's /v1/sql parses every request body as raw SQL and rejects
the {sql, parameters} JSON envelope with a 400 ParserError. The OSS
runtime accepts both formats (envelope only when Content-Type is
application/json).

Queries without parameters are now sent as text/plain raw SQL — the
format every endpoint accepts, and the pre-parameterized-queries
behavior. Parameterized queries keep the JSON envelope against the OSS
runtime, and fail fast with a clear error against Spice Cloud over
HTTP, where parameter binding requires Arrow Flight.
v2 is the current Spicepod manifest version (and the runtime default);
validated against the released spiced v2.1.2 binary, which CI installs.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR improves SDK robustness by falling back from Arrow Flight (gRPC) to HTTP when a query fails after the channel has initialized, and adjusts HTTP request formatting to match what Spice Cloud and the OSS runtime actually accept. It also updates CI/runtime fixtures and strengthens test assertions around request bodies/headers.

Changes:

  • Add query-time gRPC → HTTP fallback in sql() with safeguards (no fallback in flightOnly or after partial streaming).
  • Send raw SQL (text/plain) over HTTP when there are no parameters; keep JSON envelope for parameterized queries on OSS, and fail fast for parameterized HTTP on Spice Cloud.
  • Update spicepod version for CI and add/strengthen unit + browser tests (and wire the new unit test into CI).

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/client-common.ts Adds query-time gRPC→HTTP fallback logic and conditional HTTP body/content-type selection (raw SQL vs JSON envelope).
test/grpc-http-fallback.test.ts New unit tests covering fallback behavior and HTTP body formats (including Spice Cloud rejection for parameterized HTTP).
test/browser/client.test.ts Tightens browser-side assertions for HTTP request formats and makes config logging test deterministic via SPICE_DEBUG.
test/scripts/spicepod.yaml Updates spicepod schema version to v1 for compatibility with latest spiced.
.github/workflows/test.yml Runs the new gRPC→HTTP fallback unit tests in the local runtime CI job.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread test/browser/client.test.ts
Comment thread src/client-common.ts
…e wins over custom headers

- The SPICE_DEBUG test now restores the env var in a finally block (and
  deletes it when originally unset, since assigning undefined stores the
  string "undefined").
- doHttpQueryRequest merges custom headers before the computed
  Content-Type/Accept, so a caller override can no longer desync the
  headers from the conditionally-chosen body format. Added a unit test
  locking in the precedence.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants