Skip to content

Commit f403d88

Browse files
authored
fix(foundation): always include a result key in json-rpc responses (#24840)
## Problem The safe JSON-RPC server built success responses as `{ jsonrpc, id, result }`. When a handler returns `undefined`, `JSON.stringify` drops the `result` key, producing `{"jsonrpc":"2.0","id":N}` — a response with **neither `result` nor `error`**, which violates JSON-RPC 2.0 and leaves raw/external callers unable to tell "not found" from a malformed reply. Surfaced via `node_getContract` on an undeployed (but valid) address: the response had no `result` field at all. ## Fix One line in the shared server (`safe_json_rpc_server.ts`): coerce an `undefined` return to `null` (`result: result ?? null`, so `0`/`false`/`''` are preserved). The `result` key is now always present. ## Blast radius Framework-level, so it fixes every method that can return `undefined` — ~20 on the node interface (`getContract`, `getContractClass`, `getTxEffect`, `getTxByHash`, `getBlock`, the witness getters, synced slot/epoch, validator stats, …) plus every other server on this framework (PXE, prover-node, archiver). Void methods now also return `result: null`. ## Why it's safe for existing clients The TS client short-circuits `null`/`undefined`/`'null'`/`'undefined'` **before** schema validation, so it never runs `null` through an `.optional()` schema — omitted vs `result: null` both resolve to `undefined` client-side. Only raw/external callers change, and they now get a spec-compliant response. ## Testing - Added a test: a handler returning `undefined` now yields `{ jsonrpc, result: null }` (red before, green after). - Updated the existing void-method (`clear`) assertions, single and batch, to the new shape. - Added integration tests (`test/integration.test.ts`) pinning that optional result schemas tolerate `null` responses: - end-to-end: the wire response for an undefined return carries an explicit `result: null`, and the safe client resolves it to `undefined`; - client-isolated: a stubbed fetch returning `result: null` verbatim resolves to `undefined` against an `.optional()` schema instead of throwing a `ZodError`. Both go red if the client's null short-circuit is removed (verified: `Invalid input: expected object, received null`). - Full `json-rpc` suite: 102 passed; `foundation` lint clean.
1 parent f245aed commit f403d88

3 files changed

Lines changed: 48 additions & 5 deletions

File tree

yarn-project/foundation/src/json-rpc/server/safe_json_rpc_server.test.ts

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,10 +76,17 @@ describe('SafeJsonRpcServer', () => {
7676
it('calls an RPC function with no inputs nor outputs', async () => {
7777
const response = await send({ method: 'clear', params: [] });
7878
expect(response.status).toBe(200);
79-
expect(response.text).toEqual(JSON.stringify({ jsonrpc }));
79+
expect(response.text).toEqual(JSON.stringify({ jsonrpc, result: null }));
8080
expect(testState.notes).toEqual([]);
8181
});
8282

83+
it('returns an explicit null result (not an omitted field) when a handler returns undefined', async () => {
84+
const response = await send({ method: 'getNote', params: [99] });
85+
expect(response.status).toBe(200);
86+
expect(response.text).toEqual(JSON.stringify({ jsonrpc, result: null }));
87+
expect(JSON.parse(response.text)).toHaveProperty('result', null);
88+
});
89+
8390
it('calls an RPC function that returns a primitive object and a bigint', async () => {
8491
const response = await send({ method: 'getStatus', params: [] });
8592
expect(response.status).toBe(200);
@@ -185,7 +192,7 @@ describe('SafeJsonRpcServer', () => {
185192
expect(resp.text).toEqual(
186193
JSON.stringify([
187194
{ jsonrpc: '2.0', id: 42, result: { status: 'ok', count: '2' } },
188-
{ jsonrpc: '2.0', id: 43 },
195+
{ jsonrpc: '2.0', id: 43, result: null },
189196
]),
190197
);
191198
});
@@ -210,7 +217,7 @@ describe('SafeJsonRpcServer', () => {
210217
expect(resp.text).toEqual(
211218
JSON.stringify([
212219
{ jsonrpc: '2.0', id: 42, error: { code: -32601, message: 'Method not found: toString' } },
213-
{ jsonrpc: '2.0', id: 43 },
220+
{ jsonrpc: '2.0', id: 43, result: null },
214221
]),
215222
);
216223
});
@@ -221,7 +228,7 @@ describe('SafeJsonRpcServer', () => {
221228
expect(resp.status).toEqual(200);
222229
expect(resp.text).toEqual(
223230
JSON.stringify([
224-
{ jsonrpc: '2.0', id: 43 },
231+
{ jsonrpc: '2.0', id: 43, result: null },
225232
{ jsonrpc: '2.0', error: { code: -32600, message: 'Invalid Request' }, id: null },
226233
]),
227234
);

yarn-project/foundation/src/json-rpc/server/safe_json_rpc_server.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,11 @@ export class SafeJsonRpcServer {
224224
result = await this.proxy.call(method, params);
225225
}
226226

227-
return { jsonrpc, id, result };
227+
// Coerce an undefined return value to null so the response always carries a `result` key.
228+
// JSON.stringify drops undefined-valued keys, which would otherwise produce a JSON-RPC
229+
// response with neither `result` nor `error` — a spec violation that leaves callers unable
230+
// to distinguish "not found" from a malformed response.
231+
return { jsonrpc, id, result: result ?? null };
228232
} catch (err: any) {
229233
if (err && err instanceof ZodError) {
230234
const message = err.issues.map(e => `${e.message} (${e.path.join('.')})`).join('. ') || 'Validation error';

yarn-project/foundation/src/json-rpc/test/integration.test.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import type { JsonRpcFetch } from '../client/fetch.js';
12
import { createSafeJsonRpcClient } from '../client/safe_json_rpc_client.js';
23
import { TestNote, TestState, type TestStateApi, TestStateSchema } from '../fixtures/test_state.js';
34
import { startHttpRpcServer } from '../server/safe_json_rpc_server.js';
@@ -125,6 +126,37 @@ describe('JsonRpc integration', () => {
125126
});
126127
});
127128

129+
describe('null results against optional schemas', () => {
130+
let client: TestStateApi;
131+
let url: string;
132+
133+
beforeEach(async () => {
134+
({ server, httpServer, client, url } = await createJsonRpcTestSetup(testState, TestStateSchema));
135+
});
136+
137+
it('client accepts the explicit null result the server sends for an undefined return', async () => {
138+
const response = await fetch(url, {
139+
method: 'POST',
140+
headers: { 'content-type': 'application/json' },
141+
body: JSON.stringify({ jsonrpc: '2.0', id: 0, method: 'getNote', params: [10] }),
142+
});
143+
expect(await response.json()).toEqual({ jsonrpc: '2.0', id: 0, result: null });
144+
145+
await expect(client.getNote(10)).resolves.toBeUndefined();
146+
});
147+
148+
it('client resolves undefined when any server returns null for an optional result schema', async () => {
149+
const nullResultFetch: JsonRpcFetch = (_host, body) =>
150+
Promise.resolve({
151+
response: body.map((req: { id: number }) => ({ jsonrpc: '2.0', id: req.id, result: null })),
152+
headers: { get: () => undefined },
153+
});
154+
const nullClient = createSafeJsonRpcClient<TestStateApi>(url, TestStateSchema, { fetch: nullResultFetch });
155+
156+
await expect(nullClient.getNote(0)).resolves.toBeUndefined();
157+
});
158+
});
159+
128160
describe('namespaced', () => {
129161
let lettersState: TestState;
130162
let numbersState: TestState;

0 commit comments

Comments
 (0)