Skip to content

Commit

Permalink
Make RpcResponse provide the resopnse payload directly (#3185)
Browse files Browse the repository at this point in the history
This PR reverts some of the changes in the previous stack by making the following key change:

```ts
// Before.
type RpcResponse<TResponse = unknown> = {
    readonly json: () => Promise<TResponse>;
    readonly text: () => Promise<string>;
};

// After.
type RpcResponse<TResponse = unknown> = TResponse;
```

This is because we no longer believe that it is the responsibility of the RPC API to affect the response as text or JSON. This aspect should be encapsulated by the RPC Transport layer, meaning any custom JSON parsing/stringifying will need to be done at the RPC Transport layer.

Note that, we could simply delete the `RpcResponse` type since it returns its type parameter directly but keeping it makes the code clearer as we will use `RpcResponse` instead of `unknown` everywhere in the codebase.
  • Loading branch information
lorisleiva authored Sep 10, 2024
1 parent 28ca5d1 commit e1dfb2c
Show file tree
Hide file tree
Showing 14 changed files with 41 additions and 184 deletions.
2 changes: 1 addition & 1 deletion .changeset/light-bugs-compare.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
'@solana/rpc-spec': patch
---

Add new `RpcRequest` and `RpcResponse` types with `RpcRequestTransformer`, `RpcResponseTransformer` and `createJsonRpcResponseTransformer` functions
Add new `RpcRequest` and `RpcResponse` types with `RpcRequestTransformer` and `RpcResponseTransformer` functions
11 changes: 2 additions & 9 deletions packages/rpc-api/src/__tests__/get-health-test.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,9 @@
import { SOLANA_ERROR__JSON_RPC__SERVER_ERROR_NODE_UNHEALTHY, SolanaError } from '@solana/errors';
import { createRpc, type Rpc, type RpcResponse } from '@solana/rpc-spec';
import { createRpc, type Rpc } from '@solana/rpc-spec';

import { createSolanaRpcApi, GetHealthApi } from '../index';
import { createLocalhostSolanaRpc } from './__setup__';

function createMockResponse<T>(jsonResponse: T): RpcResponse<T> {
return {
json: () => Promise.resolve(jsonResponse),
text: () => Promise.resolve(JSON.stringify(jsonResponse)),
};
}

describe('getHealth', () => {
describe('when the node is healthy', () => {
let rpc: Rpc<GetHealthApi>;
Expand All @@ -36,7 +29,7 @@ describe('getHealth', () => {
beforeEach(() => {
rpc = createRpc({
api: createSolanaRpcApi(),
transport: jest.fn().mockResolvedValue(createMockResponse({ error: errorObject })),
transport: jest.fn().mockResolvedValue({ error: errorObject }),
});
});
it('returns an error message', async () => {
Expand Down
21 changes: 1 addition & 20 deletions packages/rpc-spec/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,7 @@ A function that accepts an `RpcRequest` and returns another `RpcRequest`. This a

### `RpcResponse`

An object that represents the response from a JSON RPC server. It contains two asynchronous methods that can be used to access the response data:

- `await response.json()`: Returns the data as a JSON object.
- `await response.text()`: Returns the data, unparsed, as a JSON string.

This allows the `RpcApi` to decide whether they want the parsed JSON object or the raw JSON string. Ultimately, the `json` method will be used by the `Rpc` to provide the final response to the caller.
A type that represents the response from a JSON RPC server. This could be any sort of data which is why `RpcResponse` defaults to `unknown`. You may use a type parameter to specify the shape of the response — e.g. `RpcResponse<{ result: number }>`.

### `RpcResponseTransformer`

Expand Down Expand Up @@ -140,17 +135,3 @@ A config object with the following properties:

- `requestTransformer<T>(request: RpcRequest<T>): RpcRequest`: An optional function that transforms the `RpcRequest` before it is sent to the JSON RPC server.
- `responseTransformer<T>(response: RpcResponse, request: RpcRequest): RpcResponse<T>`: An optional function that transforms the `RpcResponse` before it is returned to the caller.

### `createJsonRpcResponseTransformer(jsonTransformer)`

Creates an `RpcResponseTransformer<T>` function from a function that transforms any JSON value to a value of type `T` by wrapping it in a `json` async function.

```ts
const getResultTransformer = createJsonRpcResponseTransformer((json: unknown): unknown => {
return (json as { result: unknown }).result;
});
```

#### Arguments

- `jsonTransformer: (json: unknown, request: RpcRequest) => T`: A function that transforms an unknown JSON value to a value of type `T`.
56 changes: 0 additions & 56 deletions packages/rpc-spec/src/__tests__/rpc-shared-test.ts

This file was deleted.

16 changes: 4 additions & 12 deletions packages/rpc-spec/src/__tests__/rpc-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,12 @@ import { createRpcMessage } from '@solana/rpc-spec-types';

import { createRpc, Rpc } from '../rpc';
import { RpcApi, RpcApiRequestPlan } from '../rpc-api';
import { createJsonRpcResponseTransformer, RpcResponse } from '../rpc-shared';
import { RpcTransport } from '../rpc-transport';

interface TestRpcMethods {
someMethod(...args: unknown[]): unknown;
}

function createMockResponse<T>(jsonResponse: T): RpcResponse<T> {
return {
json: () => Promise.resolve(jsonResponse),
text: () => Promise.resolve(JSON.stringify(jsonResponse)),
};
}

describe('JSON-RPC 2.0', () => {
let makeHttpRequest: RpcTransport;
let rpc: Rpc<TestRpcMethods>;
Expand All @@ -41,7 +33,7 @@ describe('JSON-RPC 2.0', () => {
});
it('returns results from the transport', async () => {
expect.assertions(1);
(makeHttpRequest as jest.Mock).mockResolvedValueOnce(createMockResponse(123));
(makeHttpRequest as jest.Mock).mockResolvedValueOnce(123);
const result = await rpc.someMethod().send();
expect(result).toBe(123);
});
Expand Down Expand Up @@ -81,7 +73,7 @@ describe('JSON-RPC 2.0', () => {
let responseTransformer: jest.Mock;
let rpc: Rpc<TestRpcMethods>;
beforeEach(() => {
responseTransformer = jest.fn(createJsonRpcResponseTransformer(json => `${json} processed response`));
responseTransformer = jest.fn(json => `${json} processed response`);
rpc = createRpc({
api: {
someMethod(...params: unknown[]): RpcApiRequestPlan<unknown> {
Expand All @@ -97,14 +89,14 @@ describe('JSON-RPC 2.0', () => {
});
it('calls the response transformer with the response from the JSON-RPC 2.0 endpoint', async () => {
expect.assertions(1);
const rawResponse = createMockResponse(123);
const rawResponse = 123;
(makeHttpRequest as jest.Mock).mockResolvedValueOnce(rawResponse);
await rpc.someMethod().send();
expect(responseTransformer).toHaveBeenCalledWith(rawResponse, { methodName: 'someMethod', params: [] });
});
it('returns the processed response', async () => {
expect.assertions(1);
(makeHttpRequest as jest.Mock).mockResolvedValueOnce(createMockResponse(123));
(makeHttpRequest as jest.Mock).mockResolvedValueOnce(123);
const result = await rpc.someMethod().send();
expect(result).toBe('123 processed response');
});
Expand Down
19 changes: 1 addition & 18 deletions packages/rpc-spec/src/rpc-shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,7 @@ export type RpcRequest<TParams = unknown> = {
readonly params: TParams;
};

export type RpcResponse<TResponse = unknown> = {
readonly json: () => Promise<TResponse>;
readonly text: () => Promise<string>;
};
export type RpcResponse<TResponse = unknown> = TResponse;

export type RpcRequestTransformer = {
<TParams>(request: RpcRequest<TParams>): RpcRequest;
Expand All @@ -15,17 +12,3 @@ export type RpcRequestTransformer = {
export type RpcResponseTransformer<TResponse = unknown> = {
(response: RpcResponse, request: RpcRequest): RpcResponse<TResponse>;
};

export function createJsonRpcResponseTransformer<TResponse = unknown>(
jsonTransformer: (json: unknown, request: RpcRequest) => TResponse,
): RpcResponseTransformer<TResponse> {
return function (response: RpcResponse, request: RpcRequest): RpcResponse<TResponse> {
return Object.freeze({
...response,
json: async () => {
const json = await response.json();
return jsonTransformer(json, request);
},
});
};
}
3 changes: 1 addition & 2 deletions packages/rpc-spec/src/rpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,7 @@ function createPendingRpcRequest<TRpcMethods, TRpcTransport extends RpcTransport
payload: createRpcMessage(methodName, params),
signal: options?.abortSignal,
});
const response = responseTransformer ? responseTransformer(rawResponse, request) : rawResponse;
return await response.json();
return responseTransformer ? responseTransformer(rawResponse, request) : rawResponse;
},
};
}
Original file line number Diff line number Diff line change
@@ -1,26 +1,17 @@
import { SOLANA_ERROR__JSON_RPC__PARSE_ERROR, SolanaError } from '@solana/errors';
import { RpcRequest, RpcResponse } from '@solana/rpc-spec';
import { RpcRequest } from '@solana/rpc-spec';

import { getDefaultResponseTransformerForSolanaRpc } from '../response-transformer';
import { KEYPATH_WILDCARD } from '../tree-traversal';

function createMockResponse<T>(jsonResponse: T): RpcResponse<T> {
return {
json: () => Promise.resolve(jsonResponse),
text: () => Promise.resolve(JSON.stringify(jsonResponse)),
};
}

describe('getDefaultResponseTransformerForSolanaRpc', () => {
describe('given an array as input', () => {
const input = [10, 10n, '10', ['10', [10n, 10], 10]] as const;
const request = {} as RpcRequest;
const response = createMockResponse({ result: input });
it('casts the numbers in the array to a `bigint`, recursively', async () => {
expect.assertions(1);
const response = { result: input };
it('casts the numbers in the array to a `bigint`, recursively', () => {
const transformer = getDefaultResponseTransformerForSolanaRpc();
const transformedResponse = transformer(response, request);
await expect(transformedResponse.json()).resolves.toStrictEqual([
expect(transformer(response, request)).toStrictEqual([
BigInt(input[0]),
input[1],
input[2],
Expand All @@ -31,13 +22,11 @@ describe('getDefaultResponseTransformerForSolanaRpc', () => {
describe('given an object as input', () => {
const input = { a: 10, b: 10n, c: { c1: '10', c2: 10 } } as const;
const request = {} as RpcRequest;
const response = createMockResponse({ result: input });
const response = { result: input };

it('casts the numbers in the object to `bigints`, recursively', async () => {
expect.assertions(1);
it('casts the numbers in the object to `bigints`, recursively', () => {
const transformer = getDefaultResponseTransformerForSolanaRpc();
const transformedResponse = transformer(response, request);
await expect(transformedResponse.json()).resolves.toStrictEqual({
expect(transformer(response, request)).toStrictEqual({
a: BigInt(input.a),
b: input.b,
c: { c1: input.c.c1, c2: BigInt(input.c.c2) },
Expand All @@ -54,29 +43,25 @@ describe('getDefaultResponseTransformerForSolanaRpc', () => {
${'numeric response'} | ${[[]]} | ${10} | ${10}
`(
'performs no `bigint` upcasts on $description when the allowlist is of the form in test case $#',
async ({ allowedKeyPaths, expectation, input }) => {
expect.assertions(1);
({ allowedKeyPaths, expectation, input }) => {
const transformer = getDefaultResponseTransformerForSolanaRpc({
allowedNumericKeyPaths: { getFoo: allowedKeyPaths },
});
const request = { methodName: 'getFoo' } as RpcRequest;
const response = createMockResponse({ result: input });
const transformedResponse = transformer(response, request);
await expect(transformedResponse.json()).resolves.toStrictEqual(expectation);
const response = { result: input };
expect(transformer(response, request)).toStrictEqual(expectation);
},
);
});
describe('given a JSON RPC error as input', () => {
const request = {} as RpcRequest;
const response = createMockResponse({
const response = {
error: { code: SOLANA_ERROR__JSON_RPC__PARSE_ERROR, message: 'o no' },
});
};

it('throws it as a SolanaError', async () => {
expect.assertions(1);
it('throws it as a SolanaError', () => {
const transformer = getDefaultResponseTransformerForSolanaRpc();
const transformedResponse = transformer(response, request);
await expect(transformedResponse.json()).rejects.toThrow(
expect(() => transformer(response, request)).toThrow(
new SolanaError(SOLANA_ERROR__JSON_RPC__PARSE_ERROR, { __serverMessage: 'o no' }),
);
});
Expand Down
6 changes: 3 additions & 3 deletions packages/rpc-transformers/src/response-transformer-result.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { createJsonRpcResponseTransformer } from '@solana/rpc-spec';
import { RpcResponseTransformer } from '@solana/rpc-spec';

type JsonRpcResponse = { result: unknown };

export function getResultResponseTransformer() {
return createJsonRpcResponseTransformer(json => (json as JsonRpcResponse).result);
export function getResultResponseTransformer(): RpcResponseTransformer {
return json => (json as JsonRpcResponse).result;
}
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import { getSolanaErrorFromJsonRpcError } from '@solana/errors';
import { createJsonRpcResponseTransformer } from '@solana/rpc-spec';
import { RpcResponseTransformer } from '@solana/rpc-spec';

type JsonRpcResponse = { error: Parameters<typeof getSolanaErrorFromJsonRpcError>[0] } | { result: unknown };

export function getThrowSolanaErrorResponseTransformer() {
return createJsonRpcResponseTransformer(json => {
export function getThrowSolanaErrorResponseTransformer(): RpcResponseTransformer {
return json => {
const jsonRpcResponse = json as JsonRpcResponse;
if ('error' in jsonRpcResponse) {
throw getSolanaErrorFromJsonRpcError(jsonRpcResponse.error);
}
return jsonRpcResponse;
});
};
}
11 changes: 2 additions & 9 deletions packages/rpc-transformers/src/tree-traversal.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,4 @@
import {
createJsonRpcResponseTransformer,
RpcRequest,
RpcRequestTransformer,
RpcResponseTransformer,
} from '@solana/rpc-spec';
import { RpcRequest, RpcRequestTransformer, RpcResponseTransformer } from '@solana/rpc-spec';

export type KeyPathWildcard = { readonly __brand: unique symbol };
export type KeyPath = ReadonlyArray<KeyPath | KeyPathWildcard | number | string>;
Expand Down Expand Up @@ -61,7 +56,5 @@ export function getTreeWalkerResponseTransformer<TState extends TraversalState>(
visitors: NodeVisitor[],
initialState: TState,
): RpcResponseTransformer {
return createJsonRpcResponseTransformer(json => {
return getTreeWalker(visitors)(json, initialState);
});
return json => getTreeWalker(visitors)(json, initialState);
}
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,7 @@ describe('createHttpTransport and `AbortSignal`', () => {
} as unknown as Response);
const sendPromise = makeHttpRequest({ payload: 123, signal: abortSignal });
abortController.abort('I got bored waiting');
const response = await sendPromise;
await expect(response.json()).resolves.toMatchObject({
ok: true,
});
await expect(sendPromise).resolves.toMatchObject({ ok: true });
});
});
});
5 changes: 1 addition & 4 deletions packages/rpc-transport-http/src/http-transport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,6 @@ export function createHttpTransport(config: Config): RpcTransport {
statusCode: response.status,
});
}
return Object.freeze({
json: () => response.json(),
text: () => response.text(),
});
return await response.json();
};
}
Loading

0 comments on commit e1dfb2c

Please sign in to comment.