Skip to content

Commit cd74c80

Browse files
jnsdlsclaude
andauthored
fix: match upstream on surrogates, wire keys, raw bodies and query results (#30)
## Summary Fixes divergences B4, B6, B7 and B8 from the 1.10.0 accuracy investigation, plus three smaller ones found along the way: handler concurrency, `details: null`, and response key order and number spelling. | Finding | Before | After (matches `@rocicorp/zero` 1.10.0) | | --- | --- | --- | | B4: lone surrogate in an app error message | PushFailed `database`, nothing recorded | app error, stored as `JSON.stringify` writes it (`\udc00` escaped) | | B4: lone surrogate in a bound string (`item.put`, where literal) | driver refuses, app error or query error | bound as U+FFFD, as postgres.js sends it | | B4: lone surrogate in a Starlette response | `UnicodeEncodeError`, 500 | `JSON.stringify` text, UTF-8 | | B6: snake_case keys (`client_group_id`, `order_by`, `parent_field`, `app_id`, ...) | accepted | refused, as valita refuses them | | B6: extra `"schema_version": null` in a push body | refused | accepted (passthrough) | | B7: `/query` body with NaN or Infinity | accepted | TransformFailed `parse`, `SyntaxError` | | B7: body with bytes that are not UTF-8, or one or two BOMs | `UnicodeDecodeError` or parse failure | decoded as Node's fetch decodes it (U+FFFD, up to two BOMs dropped) | | B8: query handler returns a primitive | whole batch fails, TransformFailed `internal` | that query fails alone: `Cannot use 'in' operator to search for 'query' in X`, `{name: "TypeError"}` | | B8: `defineQuery` function returns a non-query | passed through | `asQueryInternals` error, as upstream | | Query handlers | run one after another | run together, as `Promise.all` runs them | | Error with `details: null` | `{name}` reported | no details, as upstream | | Response key order | Python declaration order | upstream's order, including its second order for response mutation IDs, negated simple conditions, a condition's subquery, the unsupported-push-version failure and start rows (`Object.entries`) | | A number past a double (`1e400`) in a response | crash in `js_number` | `null`, as `JSON.stringify` writes Infinity | The fast paths from #29 keep their speed and follow these rules. The fast `js_stringify` escapes surrogates, the cached optional keys are aliases only, the hand-built mutate response writes upstream's key order, and every transaction a session opens goes through the surrogate-cleaning wrapper. Not mirrored: - The deprecated `{query: q}` unwrap. A dict with a `"query"` key fails the batch instead. - A query name with a `~` part, or a JS `Object.prototype` name. Upstream's registry lookup finds the registry's type tag, and what fails after that is the caller's own code. - Promise tick ordering between sync and async handlers when two of them fail in one batch. Open divergences the explore runs found that this PR leaves alone: the builder accepts a direction like `"�asc"` and a non-`$param` object as a literal upstream, and Python's builder refuses both. ## Evidence Each fix began as a failing differential test against the published package: - `tests/wire/test_bodies.py` (new) sends raw bytes through upstream's `Request` path and through Python's bytes path and Starlette, then compares the response bytes. Cases: NaN, Infinity, `1e400`, `-0`, huge ints, BOMs, UTF-16, truncated bodies and bytes that are not UTF-8. - `tests/wire/test_js.py`: `js_stringify` against `JSON.stringify` over JSON values that include lone surrogates. - `tests/wire/test_requests.py` and `test_ast.py`: `corrupted()` now renames keys to their Python names and adds a null under that name. - `tests/wire/test_query.py`: handlers that return each kind of primitive and object, a gate test that deadlocks unless handlers run together, `ApplicationError` details, and response comparison as JSON text. - `tests/wire/test_mutate.py`: lone surrogates in values and messages, `details` null or absent, and response comparison as JSON text. - `tests/strategies.py`: `strings()` replaces `st.text` and gives about one character in eight as a lone surrogate. Gate: ruff, format and ty clean. pytest 460 passed, 100% branch coverage. CodSpeed against main 8994dca: 5 benchmarks faster (query_json.complex.b100 by 52%), 33 unchanged, none slower. One explore run of `test_mutate.py` and `test_requests.py`: 17 passed. Earlier full explore runs of `tests/wire` before the rebase onto #29: 309 passed. ## Merge Danger Door: two-way. One commit, nothing persisted changes shape, and a revert restores the old behaviour. Blast radius: - Wire parsing is stricter: a snake_case key is now an unknown key. zero-cache never sends one. - Response key order changes. zero-cache hashes the ASTs it receives, and its normalization keeps a simple condition's key order. A Python server's query hashes should now match a TypeScript server's. A running Python deployment may rehydrate queries once after the upgrade. - Query handlers now run concurrently. A handler that relied on running after the previous one finished has to do its own locking. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <[email protected]>
1 parent 8994dca commit cd74c80

33 files changed

Lines changed: 815 additions & 162 deletions

‎harness/mutate.ts‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,11 @@ const mutators: Record<string, (tx: Tx, args: Args) => Promise<void>> = {
8080
await put(tx, args);
8181
throw new TypeError(String(args.message ?? 'bad type'));
8282
},
83+
// A `details` property only when the args have one, which may be null.
84+
'item.detailsError': async (tx, args) => {
85+
await put(tx, args);
86+
throw Object.assign(new TypeError('with details'), 'details' in args ? {details: args.details} : {});
87+
},
8388
'crud.insert': (tx, args) => tx.mutate[args.table!].insert(args.row),
8489
'crud.upsert': (tx, args) => tx.mutate[args.table!].upsert(args.row),
8590
'crud.update': (tx, args) => tx.mutate[args.table!].update(args.row),
@@ -147,6 +152,24 @@ export function mutate({
147152
return handleMutateRequest({dbProvider: provider(url), handler, query, body: body as never, userID, logLevel: 'error'});
148153
}
149154

155+
/** handleMutateRequest over a raw request, its body given as base64, answered as the text a server would send. */
156+
export async function mutateRequest({
157+
url,
158+
search,
159+
body,
160+
userID,
161+
}: {
162+
url: string;
163+
search: string;
164+
body: string;
165+
userID: string | null;
166+
}) {
167+
const request = new Request(`http://localhost/mutate?${search}`, {method: 'POST', body: Buffer.from(body, 'base64')});
168+
return JSON.stringify(
169+
await handleMutateRequest({dbProvider: provider(url), handler, request, userID, logLevel: 'error'}),
170+
);
171+
}
172+
150173
export type {TransactionProviderHooks};
151174

152175
// PushProcessor dispatches by name to a registry from defineMutators, or to plain legacy functions.

‎harness/ops.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ import {
99
transformRequestMessageSchema,
1010
v,
1111
} from './zero.ts';
12-
import {mutate, mutateWithSchema, pushProcessor, wireSchema} from './mutate.ts';
13-
import {query, runQuery, schemaJSON} from './query.ts';
12+
import {mutate, mutateRequest, mutateWithSchema, pushProcessor, wireSchema} from './mutate.ts';
13+
import {query, queryRequest, runQuery, schemaJSON} from './query.ts';
1414

1515
// The schemas upstream parses requests with, and the mode it parses each in.
1616
const schemas = {
@@ -23,9 +23,11 @@ const schemas = {
2323

2424
export const ops = {
2525
mutate,
26+
mutateRequest,
2627
pushProcessor,
2728
mutateWithSchema,
2829
query,
30+
queryRequest,
2931
runQuery,
3032
schema: schemaJSON,
3133
wireSchema: () => JSON.parse(JSON.stringify(wireSchema)),
@@ -36,6 +38,11 @@ export const ops = {
3638
return v.test(value, type as v.Type<unknown>, mode).ok;
3739
},
3840

41+
/** `JSON.stringify(value)`. */
42+
jsonStringify({value}: {value: unknown}) {
43+
return JSON.stringify(value);
44+
},
45+
3946
/** `String(value)`, for a JSON scalar or array. */
4047
jsString({value}: {value: unknown}) {
4148
return String(value);

‎harness/query.ts‎

Lines changed: 38 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// The query half of the wire tier: the published handleQueryRequest over queries that interpret a
22
// builder program, which tests/wire/programs.py interprets the same way in Python.
33
import {createBuilder, defineQueries, defineQuery, mustGetQuery} from '@rocicorp/zero';
4-
import {handleQueryRequest} from '@rocicorp/zero/server';
4+
import {ApplicationError, handleQueryRequest} from '@rocicorp/zero/server';
55
import {zeroPostgresJS} from '@rocicorp/zero/server/adapters/postgresjs';
66
import postgres from 'postgres';
77
import {QueryParseError} from './node_modules/@rocicorp/zero/out/zql/src/query/error.js';
@@ -106,17 +106,46 @@ const queries = defineQueries({
106106
appError: defineQuery(() => {
107107
throw new Error('nope');
108108
}),
109+
appDetails: defineQuery((({args}: {args: unknown}) => {
110+
throw new ApplicationError('app', {details: args as never});
111+
}) as never),
112+
returns: defineQuery((({args}: {args: string}) => RETURNS[args]) as never),
109113
});
110114

115+
/** What a query function may return in place of a query, by name. */
116+
const RETURNS: Record<string, unknown> = {number: 5, fraction: 1.5, string: 'x', boolean: true, object: {}, array: []};
117+
118+
/**
119+
* Queries by name through the registry, except `raw`, which returns `RETURNS[args]` itself, and `gate.wait`,
120+
* which finishes only once `gate.open` has run in the same request.
121+
*/
122+
function makeHandler() {
123+
let open!: () => void;
124+
const gate = new Promise<void>(resolve => (open = resolve));
125+
return async (name: string, args: unknown) => {
126+
if (name === 'raw') return RETURNS[args as string] as never;
127+
if (name === 'gate.wait') {
128+
await gate;
129+
return zql.issue;
130+
}
131+
if (name === 'gate.open') {
132+
open();
133+
return zql.user;
134+
}
135+
return mustGetQuery(queries, name).fn({args, ctx: undefined} as never);
136+
};
137+
}
138+
111139
export function query({body, userID}: {body: unknown; userID: string | null}) {
112-
return handleQueryRequest({
113-
handler: (name, args) => mustGetQuery(queries, name).fn({args, ctx: undefined} as never),
114-
schema,
115-
body: body as never,
116-
query: {},
117-
userID,
118-
logLevel: 'error',
119-
});
140+
const handler = makeHandler();
141+
return handleQueryRequest({handler, schema, body: body as never, query: {}, userID, logLevel: 'error'});
142+
}
143+
144+
/** handleQueryRequest over a raw request body, given as base64, answered as the text a server would send. */
145+
export async function queryRequest({body, userID}: {body: string; userID: string | null}) {
146+
const request = new Request('http://localhost/query', {method: 'POST', body: Buffer.from(body, 'base64')});
147+
const handler = makeHandler();
148+
return JSON.stringify(await handleQueryRequest({handler, schema, request, userID, logLevel: 'error'}));
120149
}
121150

122151
export function schemaJSON() {

‎src/zero_server/integrations/starlette.py‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
from zero_server.schema import Schema
2222
from zero_server.server.process_mutations import Database, TransactFn, handle_mutate_request
2323
from zero_server.server.process_queries import handle_query_request
24+
from zero_server.shared.js import js_stringify
2425
from zero_server.shared.json import JSONValue
2526
from zero_server.shared.registry import Registry
2627

@@ -30,6 +31,13 @@
3031
"""The `ctx` queries and mutators receive. May be async."""
3132

3233

34+
class _JSONResponse(JSONResponse):
35+
"""The bytes a TypeScript server sends: `JSON.stringify`'s text, which escapes any lone surrogate."""
36+
37+
def render(self, content: Any) -> bytes:
38+
return js_stringify(content).encode()
39+
40+
3341
class Unauthorized(Exception):
3442
"""Raised by `authenticate` for a token it rejects. The endpoint answers 401 instead of running."""
3543

@@ -64,7 +72,7 @@ async def caller(request: Request) -> tuple[str | None, Any] | Response:
6472
try:
6573
user_id = await _maybe_await(authenticate(request))
6674
except Unauthorized as error:
67-
return JSONResponse({"error": str(error) or "Unauthorized"}, status_code=401)
75+
return _JSONResponse({"error": str(error) or "Unauthorized"}, status_code=401)
6876
return user_id, await _maybe_await(context(request, user_id))
6977

7078
if queries is not None:
@@ -80,7 +88,7 @@ async def query(request: Request) -> Response:
8088
body=await request.body(),
8189
user_id=user_id,
8290
)
83-
return JSONResponse(response.to_json())
91+
return _JSONResponse(response.to_json())
8492

8593
routes.append(Route(query_path, query, methods=["POST"]))
8694

@@ -108,7 +116,7 @@ async def run(tx: Any, name: str, args: JSONValue | None) -> None:
108116
body=await request.body(),
109117
user_id=user_id,
110118
)
111-
return JSONResponse(response.to_json())
119+
return _JSONResponse(response.to_json())
112120

113121
routes.append(Route(mutate_path, mutate, methods=["POST"]))
114122

‎src/zero_server/protocol/ast.py‎

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,13 @@
1111

1212
from collections.abc import Callable, Sequence
1313
from dataclasses import dataclass
14-
from typing import Annotated, Literal, Protocol
14+
from typing import Annotated, Any, Literal, Protocol
1515

1616
from pydantic import Field, StrictBool, StrictStr
1717

1818
from zero_server.protocol.wire import Number, StrictWireModel, WireChecked
1919
from zero_server.shared.asserts import AssertionFailedError, check
20-
from zero_server.shared.js import js_sort, js_string
20+
from zero_server.shared.js import js_keys, js_sort, js_string
2121
from zero_server.shared.json import JSONValue
2222

2323
SUBQ_PREFIX = "zsubq_"
@@ -73,9 +73,19 @@ class Parameter(StrictWireModel):
7373

7474
class SimpleCondition(StrictWireModel):
7575
type: Literal["simple"] = "simple"
76-
op: WireChecked[SimpleOperator]
7776
left: ValuePosition
7877
right: Annotated[Parameter | LiteralReference, Field(discriminator="type")]
78+
op: WireChecked[SimpleOperator]
79+
80+
def negated(self, op: str) -> SimpleCondition:
81+
return _NegatedSimpleCondition(op=op, left=self.left, right=self.right) # ty: ignore[invalid-argument-type]
82+
83+
84+
class _NegatedSimpleCondition(SimpleCondition):
85+
"""Upstream's `not` writes the negated condition with `op` second."""
86+
87+
def _ordered(self, dumped: dict[str, Any]) -> dict[str, Any]:
88+
return {"type": dumped["type"], "op": dumped["op"], "left": dumped["left"], "right": dumped["right"]}
7989

8090

8191
class Conjunction(StrictWireModel):
@@ -95,6 +105,12 @@ class CorrelatedSubqueryCondition(StrictWireModel):
95105
flip: WireChecked[StrictBool | None] = None
96106
scalar: WireChecked[StrictBool | None] = None
97107

108+
def _ordered(self, dumped: dict[str, Any]) -> dict[str, Any]:
109+
# The builder writes a condition's subquery `system` first, where `related` on the AST has it last.
110+
related = dumped["related"]
111+
keys = [k for k in ("system", "correlation", "hidden", "subquery") if k in related]
112+
return {**dumped, "related": {k: related[k] for k in keys}}
113+
98114

99115
type Condition = Annotated[
100116
SimpleCondition | Conjunction | Disjunction | CorrelatedSubqueryCondition, Field(discriminator="type")
@@ -108,10 +124,10 @@ class Correlation(StrictWireModel):
108124

109125
class CorrelatedSubquery(StrictWireModel):
110126
correlation: Correlation
111-
subquery: AST
112-
system: System | None = None
113127
# A hop the client never sees, such as the junction table of a many-to-many relationship.
114128
hidden: StrictBool | None = None
129+
subquery: AST
130+
system: System | None = None
115131

116132

117133
class Bound(StrictWireModel):
@@ -126,9 +142,9 @@ class AST(StrictWireModel):
126142
alias: StrictStr | None = None
127143
where: Condition | None = None
128144
related: tuple[CorrelatedSubquery, ...] | None = None
145+
start: Bound | None = None
129146
limit: Number | None = None
130147
order_by: Ordering | None = Field(default=None, alias="orderBy")
131-
start: Bound | None = None
132148

133149

134150
for _model in (Conjunction, Disjunction, CorrelatedSubqueryCondition, CorrelatedSubquery, AST):
@@ -201,10 +217,10 @@ def col(column: str) -> str:
201217

202218

203219
def _entries(row: Row) -> list[tuple[str, Value]]:
204-
"""`Object.entries(row)`. The builder takes any row, and an array's keys are its indexes."""
220+
"""`Object.entries(row)`, index-like keys first. The builder takes any row, and an array's keys are its indexes."""
205221
if isinstance(row, list):
206222
return [(str(i), value) for i, value in enumerate(row)]
207-
return list(row.items())
223+
return [(key, row[key]) for key in js_keys(row)]
208224

209225

210226
def _transform_where(where: Condition, table: str, transform: _Transform) -> Condition:

‎src/zero_server/protocol/error.py‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
"""
77

88
from enum import StrEnum
9-
from typing import Literal
9+
from typing import Any, Literal
1010

1111
from pydantic import Field, StrictStr
1212

@@ -65,17 +65,27 @@ class ErrorReason(StrEnum):
6565

6666

6767
class _PushFailedBase(WireModel):
68+
"""Fields in the order upstream writes them; each subclass narrows `origin` and `reason` in place."""
69+
6870
kind: Literal[ErrorKind.PUSH_FAILED] = ErrorKind.PUSH_FAILED
69-
details: JSONValue | None = None
71+
origin: ErrorOrigin
72+
reason: ErrorReason
73+
message: StrictStr
7074
# The mutations that went unprocessed, which can be a tail of the request's.
7175
mutation_ids: list[MutationID] = Field(alias="mutationIDs")
72-
message: StrictStr
76+
details: JSONValue | None = None
7377

7478

7579
class ServerPushFailed(_PushFailedBase):
7680
origin: Literal[ErrorOrigin.SERVER] = ErrorOrigin.SERVER
7781
reason: ServerPushFailedReason
7882

83+
def _ordered(self, dumped: dict[str, Any]) -> dict[str, Any]:
84+
if self.reason == ErrorReason.UNSUPPORTED_PUSH_VERSION:
85+
# Upstream writes this one failure with `message` last.
86+
dumped["message"] = dumped.pop("message")
87+
return dumped
88+
7989

8090
class ZeroCachePushHTTPFailed(_PushFailedBase):
8191
origin: Literal[ErrorOrigin.ZERO_CACHE]
@@ -93,10 +103,14 @@ class ZeroCachePushFailed(_PushFailedBase):
93103

94104

95105
class _TransformFailedBase(WireModel):
106+
"""Fields in the order upstream writes them; each subclass narrows `origin` and `reason` in place."""
107+
96108
kind: Literal[ErrorKind.TRANSFORM_FAILED] = ErrorKind.TRANSFORM_FAILED
97-
details: JSONValue | None = None
98-
query_ids: list[StrictStr] = Field(alias="queryIDs")
109+
origin: ErrorOrigin
110+
reason: ErrorReason
99111
message: StrictStr
112+
query_ids: list[StrictStr] = Field(alias="queryIDs")
113+
details: JSONValue | None = None
100114

101115

102116
class ServerTransformFailed(_TransformFailedBase):

‎src/zero_server/protocol/mutate_server.py‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,13 @@ class MutateSuccess(WireModel):
2020
nullable = frozenset({"user_id"})
2121

2222
kind: Literal["MutateResponse"] = "MutateResponse"
23-
user_id: StrictStr | None = Field(default=None, alias="userID")
2423
mutations: list[MutationResponse]
24+
user_id: StrictStr | None = Field(default=None, alias="userID")
2525

2626
def to_json(self) -> dict[str, JSONValue]:
27-
out: dict[str, JSONValue] = {"kind": self.kind}
27+
out: dict[str, JSONValue] = {"kind": self.kind, "mutations": [m.to_json() for m in self.mutations]}
2828
if self.user_id is not None or "user_id" in self.model_fields_set:
2929
out["userID"] = self.user_id
30-
out["mutations"] = [m.to_json() for m in self.mutations]
3130
return out
3231

3332

‎src/zero_server/protocol/mutation.py‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
zero/v1.10.0 (905d08516ba0caaf314cd28e93a5a47e8acdd482). Apache-2.0, Copyright Rocicorp.
55
"""
66

7-
from typing import Annotated, Literal
7+
from typing import Annotated, Any, Literal
88

99
from pydantic import Field, StrictBool, StrictStr
1010

@@ -130,4 +130,11 @@ class MutationResponse(WireModel):
130130
result: MutationResult
131131

132132
def to_json(self) -> dict[str, JSONValue]:
133-
return {"id": self.id.to_json(), "result": self.result.to_json()}
133+
# Upstream writes a response's ID `clientID` first, unlike the IDs a failed push lists.
134+
mutation_id = self.id.to_json()
135+
return {"id": {"clientID": mutation_id["clientID"], "id": mutation_id["id"]}, "result": self.result.to_json()}
136+
137+
def _ordered(self, dumped: dict[str, Any]) -> dict[str, Any]:
138+
# Upstream writes a response's ID `clientID` first, unlike the IDs a failed push lists.
139+
mutation_id = dumped["id"]
140+
return {**dumped, "id": {"clientID": mutation_id["clientID"], "id": mutation_id["id"]}}

‎src/zero_server/protocol/query_server.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@ class QuerySuccess(WireModel):
2222
nullable = frozenset({"user_id"})
2323

2424
kind: Literal["QueryResponse"] = "QueryResponse"
25-
user_id: StrictStr | None = Field(default=None, alias="userID")
2625
queries: TransformResponseBody
26+
user_id: StrictStr | None = Field(default=None, alias="userID")
2727

2828

2929
type QueryResponse = QuerySuccess | TransformFailedBody

0 commit comments

Comments
 (0)