Add envio serve — Hasura-compatible GraphQL server#1386
Open
DZakh wants to merge 31 commits into
Open
Conversation
Fixture DDL + seed derived from scenarios/test_codegen, a replica of the indexer's Hasura tracking calls with response-limit/aggregate knobs, a 253-case corpus (selects, filters, ordering, relationships, aggregates, scalars serialization, internal tables, errors, introspection, limits), and recorded ground-truth snapshots from hasura/graphql-engine v2.43.0. The differential.test.ts suite runs every case against both Hasura and envio serve and requires identical responses. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WM4FUnFYvEvWsBykmGg6Qf
Adds the serve module skeleton: minimal version-tolerant config.yaml reader (only the schema field), lenient schema.graphql relationship extraction, live pg_catalog introspection, the server model merging both, per-role GraphQL type registry mirroring Hasura's generated schema, the execution IR, and the axum HTTP entry. Validation, SQL planning, introspection resolution and websockets are stubs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WM4FUnFYvEvWsBykmGg6Qf
Adds per-type where-operator, aggregate, ordering/distinct, variable coercion, scalar serialization, relationship, and error-shape matrices, each verified deterministic across two live Hasura runs. Also adds the per-case benchmark runner (bench.ts + bench-seed.sql) and the diffServe.ts iteration helper that diffs a running envio serve against the stored oracle snapshots. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WM4FUnFYvEvWsBykmGg6Qf
Hand-rolled clients for graphql-transport-ws and legacy graphql-ws, plus live differential scenarios: live-query updates on data change, by_pk with variables, queries over ws, error frames (whose shape differs between the protocols), role gating, and streaming cursors. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WM4FUnFYvEvWsBykmGg6Qf
The fixtures mirror the actual v2.21.5-tag config shape (networks/ rpc_config keys) that current strict HumanConfig parsing rejects; the serve config reader must keep accepting them since it only reads the top-level schema field. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WM4FUnFYvEvWsBykmGg6Qf
…trospection The engine reproduces Hasura v2.43 semantics pinned by the oracle corpus: a lexical pre-scan for cases graphql-parser can't represent, Hasura-exact validation errors (paths, aeson-style coercion messages, Haskell Scientific bounds displays), field merging, fragment/directive/ variable rules, response-limit clamping, and SQL that splices Postgres-built JSON verbatim (row_to_json rows, json_agg lists, json_build_object aggregates, LATERAL relationship joins, EXISTS filters) so serialization is byte-identical - including the stringified-numerics asymmetries and PG error shapes. 586 of 591 default-phase corpus cases match Hasura exactly; the remainder need the admin mutation surface, http invalid-json shape, introspection root key order, and aggregate-nodes response capping. Also adds the differential CI job running the suite against service containers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WM4FUnFYvEvWsBykmGg6Qf
ws.rs: both Hasura subprotocols (graphql-transport-ws and legacy graphql-ws) with connection lifecycle, per-protocol error frame shapes, ka keepalives, 1s live-query polling that pushes only on change, and streaming cursors advanced via a parallel text-cast probe query. Also: comparison-operator descriptions in introspection, Hasura's exact malformed-body error taxonomy (invalid-json/parse-failed incl. the lone-surrogate decodeUtf8 case), and aggregate-nodes capping at the public response limit while aggregates stay uncapped. 587/591 default-phase and 19/19 limited-phase corpus cases match. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WM4FUnFYvEvWsBykmGg6Qf
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WM4FUnFYvEvWsBykmGg6Qf
Scopes envio serve to the read operations HyperIndex exposes (admin mutations are deliberately out): the full-admin introspection case is replaced with query_root/subscription_root comparisons, and the two mutation-execution cases are dropped. Fixes the lone-surrogate body error mapping (serde reports it as 'unexpected end of hex escape'). The suite now covers 590 default-phase + 19 limited-phase HTTP cases and 12 WebSocket subscription scenarios across both Hasura protocols, all byte-identical between engines. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WM4FUnFYvEvWsBykmGg6Qf
- Wrong admin secret was returning HTTP 401 with an unquoted message;
live Hasura actually returns HTTP 200 with header names quoted in the
message ('invalid "x-hasura-admin-secret"/"x-hasura-access-key"').
Extended the corpus role type to admin-wrong to pin this.
- Investigated adding WITH TIES to the stream cursor SQL (suggested by
Hasura's own subscription tests) but reverted it: real Hasura v2.43.0
loses rows tied with the batch boundary on a single-column cursor
instead of protecting them, and our original plain-LIMIT
implementation already reproduced that exactly. Added a regression
test pinning the real (buggy) behavior so it isn't re-introduced.
- Split bench.ts into --record-baseline (Hasura-only, run once, results
cached to fixtures/differential/hasura-baseline.json and merged rather
than overwritten on a --filter re-record) and the default engine-only
mode, so Rust iteration never needs Hasura/Docker running. Removed the
embedded oracle-diff from the engine mode - it produced false
mismatches once bench-seed.sql changed row counts; diffServe.ts is the
single source of truth for correctness, run separately against the
small dataset.
- diffServe.ts now runs cases concurrently (~3s for 590 cases instead of
sequential).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WM4FUnFYvEvWsBykmGg6Qf
Surfaced by cross-referencing Hasura's own server/tests-py test suite (Apache-2.0) for coverage gaps; this was the one genuinely missing operator/edge-case pattern found after sampling boolexp, order_by, limits, offset, enums, aggregations, and permissions test dirs - the rest substantially overlaps our existing hand-authored corpus. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WM4FUnFYvEvWsBykmGg6Qf
…rginal Confirms the concurrency=6 sweep's apparent regressions were a benchmark- methodology artifact (mixed heavy/light queries contending for one shared Postgres instance across concurrent workers), not real engine issues - verified by isolated single-request timing and a concurrency=1 re-measurement, both showing every flagged case is actually 1.4-3.8x faster. Defaulted bench.ts to --concurrency 1 and documented the tradeoff. Only rm-order-object-rel-dangling-nulls-first is measurably (31%) slower, both engines sub-second - a correlated-subquery ordering pattern worth a follow-up look, not a correctness issue. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WM4FUnFYvEvWsBykmGg6Qf
A coverage audit (cross-referencing every CompareOp/aggregate-op/error- code variant against the corpus) found _cast (CompareOp::CastText, casting a jsonb column to text for String_comparison_exp matching) was implemented but never exercised by any test. constraint-violation and postgres-error error codes were also found uncovered but are correctly unreachable from envio serve's read-only surface (class-23 constraint violations only occur on writes; postgres-error requires a connection failure, not a query condition) - not real gaps. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WM4FUnFYvEvWsBykmGg6Qf
…-by perf fix - ENVIO_PG_SSL_MODE now connects over TLS with the server certificate verified against the platform's trusted root CA store (no toggle to skip verification); adds a live-handshake test proving both accept and reject paths work. - Add a 3-job CI split: a required fast job diffing against committed oracle snapshots (Postgres only, no Hasura), an optional live-Hasura confidence job, and an optional benchmark job that records a fresh same-machine baseline per run. - Fix table/column description (GraphQL docstring -> Hasura `comment`) propagation: envio serve was bleeding a relationship field's own description onto its underlying fk column, which real Hasura never does. Added differential coverage (table, column, bool_exp/order_by, and stream_cursor_value_input description behavior) recorded against live Hasura, and extended the test harness to actually send `comment` metadata matching packages/envio/src/Hasura.res. - Fix slow ORDER BY on nested object-relationship columns by emitting a LEFT JOIN instead of a correlated subselect per row (safe: object relationships match at most one remote row). - Dedupe identifier-quoting in sql.rs and drop dead relationship- description plumbing that Hasura's own introspection never surfaces. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WM4FUnFYvEvWsBykmGg6Qf
Splits the 3470-line request validator/planner into exec/validate/
{args,bool_exp,coerce,fragments,prescan,selection,variables}.rs along
its existing section boundaries (mechanical move, no behavior change).
Pulls the self-contained tokenizer/prescan out of mod.rs, and replaces
every `use super::*` in the child modules with explicit imports so
cross-file dependencies are visible at a glance.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WM4FUnFYvEvWsBykmGg6Qf
Production hardening (each behavior pinned by a docker-gated test in serve/robustness_tests.rs): - Bound every Postgres interaction: server-side statement_timeout plus a client-side whole-operation timeout (ENVIO_PG_QUERY_TIMEOUT_MS, default 120s) so a frozen Postgres cannot hang requests; pool wait/connect timeouts (ENVIO_PG_POOL_WAIT_TIMEOUT_MS, ENVIO_PG_CONNECT_TIMEOUT_MS) so an exhausted pool sheds load instead of queuing forever. - /healthz now probes the database (bounded SELECT 1) instead of unconditionally returning 200, so orchestrators can see outages. - SIGTERM/Ctrl-C drains in-flight requests (25s bound) instead of hard-dropping connections. - Subscriptions retry transient connection-level failures for a few polls and self-heal across Postgres blips instead of dying on the first error frame; deterministic query errors still terminate immediately, matching Hasura. Memory (from a dedicated investigation; statement-cache growth measured and ruled out): - Cap the default pool at min(2*cores, 10), tunable via ENVIO_PG_POOL_MAX_SIZE — peak RSS was dominated by N concurrent fully-buffered large responses. - Write Postgres' response text straight into the output buffer instead of an intermediate owned String (-21% peak on mixed load, -35% on concurrent large responses). Review fixes: - parse_decimal used unchecked exponent arithmetic: a literal like 1e9223372036854775807 overflowed i64 and panicked under debug assertions (user-triggerable). Checked math now falls back to echoing the raw literal; regression cases added. - Transient-vs-deterministic error classification is a structured GraphQLError::is_transient_infra() instead of a cross-file string match; pool failures carry the postgres-error code. - The query timeout bounds the whole operation, not each root field, so a multi-root query cannot stack N timeout budgets. - Drop dead GraphQLError::parse_failed and a stale allow(dead_code); collapse per-field test asserts into single whole-value asserts. Verified: 40 serve unit tests (incl. 4 robustness chaos tests), clippy -D warnings, fmt, and the full 629-case live differential suite (HTTP + both WS protocols) against real Hasura — byte parity preserved. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WM4FUnFYvEvWsBykmGg6Qf
…iddleware - Fix a real clippy drift (collapsible_match) blocking CI on the base commit. - Rename serve-only pool/timeout tuning env vars from ENVIO_PG_* to ENVIO_SERVE_* (ENVIO_PG_HOST/PORT/USER/PASSWORD/DATABASE/SCHEMA/SSL_MODE stay ENVIO_PG_* since they're shared with the JS indexer's Env.res). - Bounded exponential-backoff startup retry (ENVIO_SERVE_STARTUP_RETRY_BUDGET_MS, default 60s) when Postgres isn't reachable yet at boot; only retries connectivity failures, not real backend errors (bad credentials/database). - WebSocket dead-client detection: server-driven protocol-level ping with a 2x-interval dead-peer close (ENVIO_SERVE_WS_PING_INTERVAL_MS), so a black-holed client's subscription poll loop stops instead of running against Postgres forever in the background. SO_KEEPALIVE (+ tuned idle/interval/retries) on the listening socket as a network-level backstop. - HTTP middleware on the query endpoint: tower ConcurrencyLimitLayer + request-timeout layer for load shedding above pool capacity, plus an optional per-IP rate limit (ENVIO_SERVE_RATE_LIMIT_PER_SEC). - healthz polish: configurable probe timeout (ENVIO_SERVE_HEALTHZ_TIMEOUT_MS) and a separate /livez (process-only liveness, no DB probe). - New docker-gated robustness tests for the black-holed-client close and startup-retry-then-healthy behaviors; extracted a shared test_support module (docker helpers, TestPg) out of robustness_tests/env_config to stop duplicating them. Verified against the local dev Postgres: healthz/livez/query all correct, WS ping frame observed. Full docker-gated suite deferred to CI (no docker in this sandbox). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ERWncww5ZmCZ661f4sjexS
…tness tests - exec/sql.rs: emit_stream_select also aggregates each cursor column's last-row value (in final sort order) alongside the batch JSON, so execute_stream_root reads both from one query result. Halves DB load per _stream subscription poll — previously advance_stream_cursor fired a second, near-identical query just to read the cursor columns back out. - exec/mod.rs: add execute_stream_operation (execute_operation's stream counterpart), sharing the query-timeout wrapper via a new with_query_timeout helper. - ws.rs: run_subscription now calls execute_stream_operation directly for stream roots instead of execute_operation + a second advance_stream_cursor query; drop the now-dead probe-building code (advance_stream_cursor, clone_bool_exp, clone_compare_op). - Fix two bugs CI's first real run of the new robustness tests caught: the black-holed-client test was calling ws.next() in a loop immediately (auto-ponging every server ping, so the connection never went idle) instead of actually going quiet first; the startup-retry test never applied the differential fixture schema to the freshly-started Postgres, so model building failed with "No tables to serve". Verified against the local dev Postgres: a 3-row-batch stream subscription over 10 seeded rows returns all rows across 4 polls in correct cursor order with no duplicates, and stays silent (as designed) once exhausted. Full docker-gated robustness suite deferred to CI (no docker in this sandbox). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ERWncww5ZmCZ661f4sjexS
execute_operation_inner now runs every root field through join_all and splices the resulting fragments back in the request's original order (each fragment is computed independently, so completion order doesn't matter). A query with N table root fields can hold up to N pooled connections at once instead of serially, bounded by the existing ENVIO_SERVE_POOL_MAX_SIZE / ENVIO_SERVE_POOL_WAIT_TIMEOUT_MS safety valves. Verified against the local dev Postgres: a 3-root-field query (two table roots plus __typename) returns all three fragments correctly ordered. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ERWncww5ZmCZ661f4sjexS
…solution
- prescan.rs: f64-overflowing float literals are now rewritten to a unique
finite per-occurrence sentinel (like the existing oversized-int handling)
instead of just being recorded in a flat list. coerce.rs's Num::display
looked up the original text by sign only ("first entry matching +/-"),
so two distinct out-of-range float literals in the same query with the
same sign would collapse to whichever text happened to come first --
wrong error text for the second one. Keying by the sentinel's exact bit
pattern fixes that; verified with a round-trip test walking the real
graphql-parser AST to confirm the rewritten literal reparses to the exact
same f64 bits. Also fixes a latent correctness gap this rewrite exposed:
the Float/Float8 bounds check only looked at `f.is_infinite()`, which a
rewritten (now-finite) sentinel would no longer trip -- it now also
checks the sentinel map so `1e400` still errors instead of silently
coercing to a huge finite value.
- model.rs: the three call sites resolving a GraphQL field name to its
(possibly snake_cased) db column -- object-relationship fk columns,
described scalar fields, array-relationship remote columns -- each
hand-rolled the same "try original name, then snake_case" lookup.
Consolidated into one resolve_db_column helper.
Verified against the local dev Postgres: object + array relationship
queries (User -> tokens -> owner/collection) resolve correctly through the
consolidated resolver.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ERWncww5ZmCZ661f4sjexS
… blocks - Extract the near-identical `arg`/`flag` process.argv parsing repeated in bench.ts, diffServe.ts, and record.ts into a shared packages/e2e-tests/src/differential/cliArgs.ts. (runSql was already a single shared helper in hasuraSetup.ts, so no dedup was needed there.) - Add a `start-envio-serve` composite action (.github/actions/start-envio-serve) encapsulating the background-spawn + /healthz-poll sequence duplicated across differential-test's two serve starts and differential-benchmark's serve start, parameterized by port, working-directory, the two Hasura-compat env vars, and whether to kill a prior serve process first. - Collapse the identical postgres/hasura `services:` blocks repeated across scenarios-test, differential-test, differential-test-live, differential-benchmark, and e2e-test into YAML anchors defined once on scenarios-test (first in file order) and referenced via aliases elsewhere — GitHub Actions' YAML parser accepts standard anchors/aliases even though composite actions can't encapsulate job-level `services:`. Verified by parsing before/after with PyYAML and diffing the resolved job objects: services blocks and all other job-level keys are identical. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ERWncww5ZmCZ661f4sjexS
bench.ts only ever ran at concurrency=1; production traffic is concurrent. soakLoad.ts fires a worker pool against a mixed sample of the bench-flagged corpus for a configurable duration and asserts the server stays healthy: zero 5xx, no RSS growth trend (leak), no fd growth trend (leak), and no p99 latency drift over time. RSS/fd are sampled from /proc directly; latency stats use reservoir sampling so memory stays bounded on multi-hour runs. Defaults to a 60s smoke run; --duration 2h --spawn is the real acceptance soak. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ERWncww5ZmCZ661f4sjexS
…+ TS harness - bench-report.md: rerun the full differential bench sweep against cargo build --release --lib instead of the debug build the prior numbers were from. Geomean speedup x4.81 (was x3.85 debug), peak envio RSS 533MB (was 931MB debug). Full differential parity (596/596 default phase, 19/19 limited) reconfirmed on the release binary before benchmarking. Also ran a 45s RSS-load smoke test against the release binary via the new soak harness; documented the result and that the real 2h acceptance run hasn't been executed yet (impractical in an interactive session). - soakLoad.ts: concurrent load-test harness (16-64 concurrency, configurable duration) asserting no RSS growth trend, no fd leaks, stable p99, 0 5xx over a run -- `pnpm soak:differential -- --duration 2h --concurrency 48 --spawn` is the real acceptance test. - cliArgs.ts: shared CLI arg-parsing helper, replacing near-identical process.argv parsing duplicated across bench.ts/diffServe.ts/record.ts. - CI: new start-envio-serve composite action replacing the duplicated background-spawn + /healthz-poll step blocks in differential-test and differential-benchmark; collapsed the byte-identical postgres/hasura services: blocks duplicated across 5 jobs into YAML anchors/aliases. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ERWncww5ZmCZ661f4sjexS
.claude/worktrees/ is scratch space for isolated background-agent checkouts, never meant to be repo content -- was showing up as untracked files. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ERWncww5ZmCZ661f4sjexS
Re-records hasura-baseline.json (and its report) against the exact machine window the pending release-build engine sweep will run in — the committed baseline dates from a different (debug-build) session, and machine load drifts enough between sessions that comparing across windows isn't fair (see the module doc comment in bench.ts). The engine-side release numbers land in a follow-up commit once that sweep finishes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WM4FUnFYvEvWsBykmGg6Qf
Same-window recording against a release build (cargo build --release), with an explicit contention check: verified via ps aux that exactly one envio serve process existed throughout both the baseline recording and the engine sweep. That check exists because an earlier same-session attempt was discarded after discovering a stray duplicate envio serve process (orphaned from an earlier restart, 37+ minutes of accumulated background CPU) that had been silently competing for CPU and Postgres connections the whole time — it inflated the reported envio-side CPU time roughly 140x (1038s -> 7.4s once killed) and skewed the recorded geomean. That run's numbers were never committed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WM4FUnFYvEvWsBykmGg6Qf
`arguments: null` on a non-count aggregate predicate (bool_and/bool_or) skipped column population but still passed the required-field check, so sql.rs emitted invalid syntax like `bool_and(*)` instead of a clean validation error. Only count's `arguments` is a genuinely nullable list (null means count(*)); bool_and/bool_or's is a single non-null column enum and should reject null the same way coerce_enum already does for every other enum-typed field. Adds a docker-gated regression test in robustness_tests.rs covering both the count+null (still valid) and bool_and+null (now rejected) cases against a real server.
…d) (#1383) * Correct aggregate bool_exp null-arguments fix to match Hasura exactly The previous fix only rejected a null `arguments` for bool_and/bool_or, assuming count's `arguments: null` was a valid count(*) alias. Verified live against Hasura 2.43.0 (fresh Postgres + Hasura containers, the committed differential fixture, and a throwaway boolean column added to Token) and that assumption was wrong: Hasura rejects an explicit `arguments: null` for every op, including count — "expected a list, but found null". Omitting the `arguments` key entirely is the actual count(*) equivalent, and that path was already correct. - bool_exp.rs: count's arguments now goes through the existing `expect_list` helper (already used elsewhere in this file for the same null-rejection message), instead of a bespoke null-skip. - robustness_tests.rs: the regression test now asserts full response bodies against the exact strings/paths captured live from Hasura for all three cases (arguments omitted, arguments: null on count, arguments: null on bool_and), not just an error code. * Fix 3 more null-literal validation gaps found by sweeping the same pattern Systematically searched every is_null() shortcut in the validator for the same shape as the aggregate bool_exp arguments bug (a null check skips a coercion call that would otherwise reject the value), then verified each candidate live against Hasura 2.43.0 rather than assuming. Confirmed and fixed three more real gaps; ruled out ~9 other candidates that looked similar but are genuinely accepted by Hasura (distinct_on: null, offset: null, order_by: null and its nested/aggregate forms, includeDeprecated: null) so those were left unchanged. Fixed, all using the coercion helper each site already had available: - bool_exp.rs: aggregate bool_exp `distinct: null` now goes through coerce_bool_strict instead of silently defaulting to false. - mod.rs: aggregate selection `count(columns: null)` now goes through expect_list instead of silently acting as count(*); `count(distinct: null)` now goes through coerce_bool_strict instead of defaulting to false. - args.rs: a json/jsonb field's `path: null` now goes through coerce_string_strict instead of silently returning the whole value. Each fix's regression test asserts the exact response body captured live from Hasura for that query, and each was confirmed to fail without the fix before being confirmed to pass with it. * Fix _stream cursor validation to reject empty/null cursor lists coerce_stream_args required the cursor argument to be present, but never checked that the resulting cursor list was non-empty after filtering out null elements. cursor: [], cursor: [null], and cursor: null all silently produced an empty cursor, which sql.rs then compiled with no ORDER BY — an unbounded, non-deterministic _stream poll instead of an error. Verified live against Hasura 2.43.0 over both WS subprotocols: - cursor: null -> "expected a list, but found null" (same expect_list helper already used for every other "explicit null on a list argument" case in this codebase) - cursor: [] and cursor: [null] -> "one streaming column field is expected" once nulls are filtered out and nothing valid remains Added to packages/e2e-tests/src/differential/subscriptions.test.ts, which already runs this exact category of live WS comparison test (subscriptions can't be exercised over plain HTTP, so this couldn't be a Rust-only regression test like the earlier fixes in this PR). Confirmed each new case fails without the fix (envio serve just polls the same unordered batch forever instead of erroring) and passes with it. * Eliminate the _stream cursor double-query race advance_stream_cursor fired a second, near-identical query after the batch query to read back the cursor columns' values for the last row, purely because the client-facing selection might not include them. A row inserted or deleted between the two queries could desync the cursor from what was actually sent to the client (skipped or duplicated rows on the next poll) — a real, if narrow, race window. Fixes it by construction rather than narrowing the window: emit_stream_ select now aggregates each cursor column's last-row value in the same query as the batch JSON, via array_agg(col::text ORDER BY <same order as the batch>)[count(*)] — the same ORDER BY drives both aggregates, so "last element of the array" is guaranteed to be the same row the batch JSON's last element came from, read atomically in one round trip. This also halves DB load per _stream poll (one query instead of two) and deletes ~150 lines of now-dead probe-building code (advance_stream_ cursor, clone_bool_exp, clone_compare_op). This exact fix already exists, independently authored and smoke-tested, on a sibling branch (claude/envio-production-ready-vvfw6h, commit 65b60b8) that forked from the same base commit as this branch while doing unrelated hardening work. Diffed that commit in isolation from its parent (excluding an unrelated WS-dead-client-detection commit that sits between the fork point and it in that branch's history) and applied it here after confirming these three files were byte-identical to the shared ancestor. Reviewed the SQL-aggregation logic by hand before applying rather than taking it on faith. Verified in this session: - cargo clippy -D warnings and the full serve:: unit suite (33 tests) pass, including the existing stream_cursor_bound test (already updated by that commit to assert the new one-query SQL shape). - Re-running the live differential WS suite (cursor advancement + tie-breaking behavior against real Hasura) to confirm no regression from the SQL rewrite; results to follow. * Fix clippy::collapsible_match flagged by a newer stable toolchain CI's cargo-test job failed: dtolnay/rust-toolchain@stable resolved to 1.96.1, which flags this pattern (a single-arm null-check inside a match arm's body) as collapsible into a match guard. My sandbox was on a stale 1.94.1 toolchain that doesn't have this lint refinement, so it passed locally right up until I checked CI. Updated the sandbox toolchain to match (rustup update stable -> 1.96.1) and reproduced the failure before fixing it, rather than guessing from the CI log alone. Pre-existing code (not touched by any commit in this PR before now) - just happened to sit in the same function I edited for the cursor validation fix. Verified: cargo clippy -D warnings clean on 1.96.1, and the full `cargo test --no-default-features` (whole crate, matching CI's exact command) passes: 400 passed, 0 failed. --------- Co-authored-by: Claude <noreply@anthropic.com>
Contributor
|
Important Review skippedToo many files! This PR contains 694 files, which is 394 over the limit of 300. To get a review, narrow the scope: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (694)
You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
… dead-client detection, concurrent execution Pulls in the useful changes from claude/envio-production-ready-vvfw6h (closed PR #1375) that we hadn't independently written ourselves: - HTTP middleware: per-IP rate limiting (ENVIO_SERVE_RATE_LIMIT_PER_SEC) and a request-level timeout/concurrency-limit layer with clean 503s on overload, both previously untested -- added rate_limiter unit tests and two new robustness_tests.rs integration tests covering them end-to-end. - WebSocket dead-client detection: a silent (black-holed) subscriber is closed within 30s instead of leaking its Postgres poll loop forever. - Startup retry: `envio serve` started before Postgres is reachable retries within ENVIO_SERVE_STARTUP_RETRY_BUDGET_MS and becomes healthy once Postgres comes up, instead of crashing on a cold-start race. - Concurrent root-field execution: independent root fields in one operation now run via `join_all` instead of sequentially. - env var rename: serve-only tuning vars moved from ENVIO_PG_* to ENVIO_SERVE_* (shared vars with the JS indexer's Env.res stay ENVIO_PG_*). - test_support.rs: shared docker-container test helpers, deduping what env_config.rs's TLS test and robustness_tests.rs each rolled themselves. - inf_float_originals: per-occurrence sentinel keying so two distinct oversized float literals sharing a sign no longer collapse to one reported original (coexists with this branch's separate i64-overflow fix in the same coercion path). - model.rs: resolve_db_column() consolidates the three previously triplicated original-name/snake_case column-resolution call sites. - CI: build_and_verify.yml dedups repeated postgres/hasura service blocks via YAML anchors and a new start-envio-serve composite action. - e2e-tests: cliArgs.ts (shared CLI parsing) and soakLoad.ts (concurrent soak/load harness: RSS growth, fd leaks, p99 drift, 5xx rate). Both merge conflicts in exec/mod.rs and robustness_tests.rs were between convergent/independent implementations of the same behavior (stream-cursor fold, new test functions landing at the same point in the file) -- resolved by keeping one canonical copy of each plus whatever each side added on top. bench-report.md's conflict was left as our committed version; it's generated data, not hand-merged. Verified: cargo check/test/fmt clean; all 50 serve unit+robustness tests pass against live Postgres/docker containers; full differential suite (596 default-phase + 19 limited-phase cases) matches the Hasura oracle snapshots one-to-one with zero regressions. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WM4FUnFYvEvWsBykmGg6Qf
…dant with Envoy) Envoy already sits in front of envio serve and handles per-client rate limiting, request timeouts, and concurrency shedding at the proxy layer, so duplicating that logic in-process was redundant surface area. Removes: - the per-IP fixed-window RateLimiter and its axum middleware (ENVIO_SERVE_RATE_LIMIT_PER_SEC) - the tower ConcurrencyLimitLayer + TimeoutLayer stack on the query POST route and its overload-to-HTTP-status error mapping (ENVIO_SERVE_MAX_CONCURRENT_REQUESTS, ENVIO_SERVE_REQUEST_TIMEOUT_MS) - the now-unused `tower` direct dependency (axum still pulls its own compatible copy transitively) - the corresponding ServeState/ServeEnv fields, unit tests, and the two robustness_tests.rs integration tests added to cover this middleware The client-side GraphQL query timeout (ENVIO_SERVE_QUERY_TIMEOUT_MS) is unaffected -- that bounds the Postgres round-trip itself and has nothing to do with the HTTP-layer concerns Envoy now owns exclusively. Verified: cargo check/test/fmt/clippy clean (serve-scoped); all 46 remaining serve tests pass against live Postgres/docker containers; full differential suite (596 default + 19 limited-phase cases) still matches the Hasura oracle one-to-one. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WM4FUnFYvEvWsBykmGg6Qf
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
2 participants
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.
Implements a production-ready GraphQL server (
envio serve) that provides Hasura API compatibility over Postgres, enabling local development and testing without Hasura Cloud.Summary
This PR adds a complete GraphQL execution engine in Rust that:
graphql-transport-wsandgraphql-wsprotocols)Key Changes
Core execution pipeline (
packages/cli/src/serve/):mod.rs: Server initialization and state managementhttp.rs: HTTP handler for/v1/graphqland health endpointsws.rs: WebSocket subscription support (both modern and legacy protocols)env_config.rs: Environment configuration with TLS supportmodel.rs: Server model derived from Postgres catalog + indexer metadataproject_schema.rs: Version-tolerant project config readingGraphQL schema & introspection (
packages/cli/src/serve/gql/):schema_build.rs: Per-role schema generation matching Hasura's exact shapetypes.rs: In-memory GraphQL type systemintrospection.rs:__schema/__typeresolution with byte-identical outputRequest execution (
packages/cli/src/serve/exec/):mod.rs: Request pipeline orchestrationerror.rs: Hasura-shaped error responsesir.rs: Intermediate representation of validated operationsvalidate/mod.rs: GraphQL parsing, validation, and IR generationargs.rs: Argument coercion (select, by_pk, stream)bool_exp.rs: Boolean expression parsingcoerce.rs: Scalar type coercion (strict GraphQL semantics)selection.rs: Selection set flatteningvariables.rs: Variable binding and coercionfragments.rs: Fragment definition trackingprescan.rs: Lexical pre-scan for duplicate detectionsql.rs: SQL compilation and executionrow_to_json,json_agg,LEFT OUTER JOIN LATERALSTRINGIFY_NUMERIC_TYPESTesting & fixtures (
packages/e2e-tests/fixtures/differential/):schema.sql: Differential test fixture schemaseed.sql: Deterministic test databench-seed.sql: Benchmark volume datahasura-baseline.json: Recorded Hasura response baselinesnapshots/default/: 200+ test case snapshots covering:CLI integration:
packages/cli/src/cli_args/clap_definitions.rs: Addedservecommandpackages/cli/src/executor/mod.rs: Wired serve command executionpackages/cli/src/lib.rs: Exposed serve modulepackages/cli/Cargo.toml: Added dependencies (tokio, tokio-postgres, graphql-parser, serde_json)packages/cli/test/configs/: Legacy config format test fixturesCI/CD:
.github/workflows/build_and_verify.yml: Added differential benchmark jobNotable Implementation Details
https://claude.ai/code/session_01WM4FUnFYvEvWsBykmGg6Qf