Skip to content

refactor!: Forbid undefined results from RPC endpoints #7050

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
rekmarks wants to merge 2 commits into from
Closed

refactor!: Forbid undefined results from RPC endpoints #7050

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a238721
refactor!: Add InvalidResponseFromEndpointError
rekmarks Nov 4, 2025
a43d055
test: Remove undefined from empty value retry tests
rekmarks Nov 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/controller-utils/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
"dependencies": {
"@metamask/eth-query": "^4.0.0",
"@metamask/ethjs-unit": "^0.3.0",
"@metamask/rpc-errors": "^7.0.2",
"@metamask/utils": "^11.8.1",
"@spruceid/siwe-parser": "2.1.0",
"@types/bn.js": "^5.1.5",
Expand Down
11 changes: 11 additions & 0 deletions packages/controller-utils/src/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,3 +286,14 @@ export const DAY = HOURS * 24;
* The number of milliseconds in a day.
*/
export const DAYS = DAY;

/**
* Custom JSON-RPC error codes for specific cases.
*
* TODO: These should be moved to `@metamask/rpc-errors`.
*/
export const CUSTOM_RPC_ERRORS = {
unauthorized: -32006,
invalidResponseFromEndpoint: -32007,
httpClientError: -32080,
} as const;
16 changes: 16 additions & 0 deletions packages/controller-utils/src/errors.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { JsonRpcError } from '@metamask/rpc-errors';
import { InvalidResponseFromEndpointError } from './errors';

describe('InvalidResponseFromEndpointError', () => {
it('should be an instance of JsonRpcError', () => {
expect(
new InvalidResponseFromEndpointError({ response: {} }),
).toBeInstanceOf(JsonRpcError);
});

it('should expose the response', () => {
const response = { result: null };
const error = new InvalidResponseFromEndpointError({ response });
expect(error.data?.response).toBe(response);
});
});
21 changes: 21 additions & 0 deletions packages/controller-utils/src/errors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { JsonRpcError } from '@metamask/rpc-errors';
import type { Json } from '@metamask/utils';

import { CUSTOM_RPC_ERRORS } from './constants';

type Options = {
message?: string;
response: Json;
};

export class InvalidResponseFromEndpointError extends JsonRpcError<{
response: Json;
}> {
constructor({ message, response }: Options) {
super(
CUSTOM_RPC_ERRORS.invalidResponseFromEndpoint,
message ?? 'Invalid response from endpoint',
);
this.data = { response };
}
}
2 changes: 2 additions & 0 deletions packages/controller-utils/src/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ describe('@metamask/controller-utils', () => {
"DAY",
"DAYS",
"NETWORKS_BYPASSING_VALIDATION",
"CUSTOM_RPC_ERRORS",
"BNToHex",
"convertHexToDecimal",
"fetchWithErrorHandling",
Expand Down Expand Up @@ -91,6 +92,7 @@ describe('@metamask/controller-utils', () => {
"parseDomainParts",
"isValidSIWEOrigin",
"detectSIWE",
"InvalidResponseFromEndpointError",
]
`);
});
Expand Down
2 changes: 2 additions & 0 deletions packages/controller-utils/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ export {
DAY,
DAYS,
NETWORKS_BYPASSING_VALIDATION,
CUSTOM_RPC_ERRORS,
} from './constants';
export type { NonEmptyArray } from './util';
export {
Expand Down Expand Up @@ -86,3 +87,4 @@ export {
} from './util';
export * from './types';
export * from './siwe';
export * from './errors';
8 changes: 8 additions & 0 deletions packages/eth-json-rpc-middleware/src/fetch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { InvalidResponseFromEndpointError } from '@metamask/controller-utils';
import type { JsonRpcMiddleware } from '@metamask/json-rpc-engine';
import { createAsyncMiddleware } from '@metamask/json-rpc-engine';
import { rpcErrors } from '@metamask/rpc-errors';
Expand Down Expand Up @@ -66,6 +67,13 @@ export function createFetchMiddleware({
});
}

if (jsonRpcResponse.result === undefined) {
throw new InvalidResponseFromEndpointError({
message: 'Received undefined result from endpoint',
response: jsonRpcResponse,
});
}

// Discard the `id` and `jsonrpc` fields in the response body
// (the JSON-RPC engine will fill those in)
res.result = jsonRpcResponse.result;
Expand Down
28 changes: 22 additions & 6 deletions packages/network-controller/src/create-network-client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import type { InfuraNetworkType } from '@metamask/controller-utils';
import { ChainId } from '@metamask/controller-utils';
import {
ChainId,
InvalidResponseFromEndpointError,
} from '@metamask/controller-utils';
import type { PollingBlockTrackerOptions } from '@metamask/eth-block-tracker';
import { PollingBlockTracker } from '@metamask/eth-block-tracker';
import { createInfuraMiddleware } from '@metamask/eth-json-rpc-infura';
Expand Down Expand Up @@ -139,12 +142,25 @@ export function createNetworkClient({

const rpcApiMiddleware =
configuration.type === NetworkClientType.Infura
? createInfuraMiddleware({
rpcService: rpcServiceChain,
options: {
source: 'metamask',
? mergeMiddleware([
// TODO: This should be removed after modifying the behavior of the Infura middleware
(_req, res, next, _end) => {
return next(() => {
if (res.result === undefined) {
throw new InvalidResponseFromEndpointError({
message: 'Received undefined result from endpoint',
response: res,
});
}
});
},
})
createInfuraMiddleware({
rpcService: rpcServiceChain,
options: {
source: 'metamask',
},
}),
])
: createFetchMiddleware({ rpcService: rpcServiceChain });

const rpcProvider = providerFromMiddleware(rpcApiMiddleware);
Expand Down
4 changes: 2 additions & 2 deletions packages/network-controller/src/rpc-service/rpc-service.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
// We use conditions exclusively in this file.
/* eslint-disable jest/no-conditional-in-test */

import { HttpError } from '@metamask/controller-utils';
import { CUSTOM_RPC_ERRORS, HttpError } from '@metamask/controller-utils';
import { errorCodes } from '@metamask/rpc-errors';
import nock from 'nock';
import { FetchError } from 'node-fetch';
import { useFakeTimers } from 'sinon';
import type { SinonFakeTimers } from 'sinon';

import type { AbstractRpcService } from './abstract-rpc-service';
import { CUSTOM_RPC_ERRORS, RpcService } from './rpc-service';
import { RpcService } from './rpc-service';
import { DEFAULT_CIRCUIT_BREAK_DURATION } from '../../../controller-utils/src/create-service-policy';

describe('RpcService', () => {
Expand Down
11 changes: 1 addition & 10 deletions packages/network-controller/src/rpc-service/rpc-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type {
} from '@metamask/controller-utils';
import {
BrokenCircuitError,
CUSTOM_RPC_ERRORS,
CircuitState,
HttpError,
createServicePolicy,
Expand Down Expand Up @@ -133,16 +134,6 @@ export const CONNECTION_ERRORS = [
},
];

/**
* Custom JSON-RPC error codes for specific cases.
*
* These should be moved to `@metamask/rpc-errors` eventually.
*/
export const CUSTOM_RPC_ERRORS = {
unauthorized: -32006,
httpClientError: -32080,
} as const;

/**
* Determines whether the given error represents a failure to reach the network
* after request parameters have been validated.
Expand Down
4 changes: 2 additions & 2 deletions packages/network-controller/tests/network-client/block-hash-in-response.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { CUSTOM_RPC_ERRORS } from '@metamask/controller-utils';
import { errorCodes, rpcErrors } from '@metamask/rpc-errors';

import type { ProviderType } from './helpers';
Expand All @@ -7,7 +8,6 @@ import {
withNetworkClient,
} from './helpers';
import { testsForRpcFailoverBehavior } from './rpc-failover';
import { CUSTOM_RPC_ERRORS } from '../../src/rpc-service/rpc-service';
import { NetworkClientType } from '../../src/types';

type TestsForRpcMethodThatCheckForBlockHashInResponseOptions = {
Expand Down Expand Up @@ -203,7 +203,7 @@ export function testsForRpcMethodsThatCheckForBlockHashInResponse(
});
});

for (const emptyValue of [null, undefined, '\u003cnil\u003e']) {
for (const emptyValue of [null, '\u003cnil\u003e']) {
it(`does not retry an empty response of "${emptyValue}"`, async () => {
const request = { method };
const mockResult = emptyValue;
Expand Down
10 changes: 5 additions & 5 deletions packages/network-controller/tests/network-client/block-param.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { CUSTOM_RPC_ERRORS } from '@metamask/controller-utils';
import { errorCodes, rpcErrors } from '@metamask/rpc-errors';
import type { Hex } from '@metamask/utils';

Expand All @@ -10,7 +11,6 @@ import {
withNetworkClient,
} from './helpers';
import { testsForRpcFailoverBehavior } from './rpc-failover';
import { CUSTOM_RPC_ERRORS } from '../../src/rpc-service/rpc-service';
import { NetworkClientType } from '../../src/types';

type TestsForRpcMethodSupportingBlockParam = {
Expand Down Expand Up @@ -209,7 +209,7 @@ export function testsForRpcMethodSupportingBlockParam(
});
});

for (const emptyValue of [null, undefined, '\u003cnil\u003e']) {
for (const emptyValue of [null, '\u003cnil\u003e']) {
it(`does not retry an empty response of "${emptyValue}"`, async () => {
const request = {
method,
Expand Down Expand Up @@ -1227,7 +1227,7 @@ export function testsForRpcMethodSupportingBlockParam(
});
});

for (const emptyValue of [null, undefined, '\u003cnil\u003e']) {
for (const emptyValue of [null, '\u003cnil\u003e']) {
it(`does not retry an empty response of "${emptyValue}"`, async () => {
const request = {
method,
Expand Down Expand Up @@ -1360,7 +1360,7 @@ export function testsForRpcMethodSupportingBlockParam(
});
});

for (const emptyValue of [null, undefined, '\u003cnil\u003e']) {
for (const emptyValue of [null, '\u003cnil\u003e']) {
if (providerType === 'infura') {
it(`retries up to 10 times if a "${emptyValue}" response is returned, returning successful non-empty response if there is one on the 10th try`, async () => {
const request = {
Expand Down Expand Up @@ -1555,7 +1555,7 @@ export function testsForRpcMethodSupportingBlockParam(
});
});

for (const emptyValue of [null, undefined, '\u003cnil\u003e']) {
for (const emptyValue of [null, '\u003cnil\u003e']) {
it(`does not retry an empty response of "${emptyValue}"`, async () => {
const request = {
method,
Expand Down
4 changes: 2 additions & 2 deletions packages/network-controller/tests/network-client/no-block-param.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { CUSTOM_RPC_ERRORS } from '@metamask/controller-utils';
import { errorCodes, rpcErrors } from '@metamask/rpc-errors';

import type { ProviderType } from './helpers';
Expand All @@ -7,7 +8,6 @@ import {
withNetworkClient,
} from './helpers';
import { testsForRpcFailoverBehavior } from './rpc-failover';
import { CUSTOM_RPC_ERRORS } from '../../src/rpc-service/rpc-service';
import { NetworkClientType } from '../../src/types';

type TestsForRpcMethodAssumingNoBlockParamOptions = {
Expand Down Expand Up @@ -149,7 +149,7 @@ export function testsForRpcMethodAssumingNoBlockParam(
});
});

for (const emptyValue of [null, undefined, '\u003cnil\u003e']) {
for (const emptyValue of [null, '\u003cnil\u003e']) {
it(`does not retry an empty response of "${emptyValue}"`, async () => {
const request = { method };
const mockResult = emptyValue;
Expand Down
1 change: 1 addition & 0 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3120,6 +3120,7 @@ __metadata:
"@metamask/auto-changelog": "npm:^3.4.4"
"@metamask/eth-query": "npm:^4.0.0"
"@metamask/ethjs-unit": "npm:^0.3.0"
"@metamask/rpc-errors": "npm:^7.0.2"
"@metamask/utils": "npm:^11.8.1"
"@spruceid/siwe-parser": "npm:2.1.0"
"@ts-bridge/cli": "npm:^0.6.4"
Expand Down
Loading