diff --git a/jest.config.packages.js b/jest.config.packages.js index ce6f1f267d..b3406afaf1 100644 --- a/jest.config.packages.js +++ b/jest.config.packages.js @@ -80,6 +80,9 @@ module.exports = { // Here we ensure that Jest resolves `@metamask/*` imports to the uncompiled source code for packages that live in this repo. // NOTE: This must be synchronized with the `paths` option in `tsconfig.packages.json`. moduleNameMapper: { + '^@metamask/json-rpc-engine/v2$': [ + '/../json-rpc-engine/src/v2/index.ts', + ], '^@metamask/(.+)$': [ '/../$1/src', // Some @metamask/* packages we are referencing aren't in this monorepo, diff --git a/packages/eth-block-tracker/tests/withBlockTracker.ts b/packages/eth-block-tracker/tests/withBlockTracker.ts index 39c227da90..ca866459f5 100644 --- a/packages/eth-block-tracker/tests/withBlockTracker.ts +++ b/packages/eth-block-tracker/tests/withBlockTracker.ts @@ -1,8 +1,4 @@ -import { providerFromEngine } from '@metamask/eth-json-rpc-provider'; -import type { - // Eip1193Request, - InternalProvider, -} from '@metamask/eth-json-rpc-provider'; +import { InternalProvider } from '@metamask/eth-json-rpc-provider'; import { JsonRpcEngine } from '@metamask/json-rpc-engine'; import type { Json } from '@metamask/utils'; import util from 'util'; @@ -98,7 +94,7 @@ function getFakeProvider({ }); } - const provider = providerFromEngine(new JsonRpcEngine()); + const provider = new InternalProvider({ engine: new JsonRpcEngine() }); jest .spyOn(provider, 'request') .mockImplementation(async (eip1193Request): Promise => { diff --git a/packages/eth-json-rpc-middleware/CHANGELOG.md b/packages/eth-json-rpc-middleware/CHANGELOG.md index d8f6c686f1..673180d189 100644 --- a/packages/eth-json-rpc-middleware/CHANGELOG.md +++ b/packages/eth-json-rpc-middleware/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **BREAKING:** Use `InternalProvider` instead of `SafeEventEmitterProvider` ([#6796](https://github.com/MetaMask/core/pull/6796)) - Wherever a `SafeEventEmitterProvider` was expected, an `InternalProvider` is now expected instead. +- **BREAKING:** Stop retrying `undefined` results for methods that include a block tag parameter ([#7001](https://github.com/MetaMask/core/pull/7001)) + - The `retryOnEmpty` middleware will now throw an error if it encounters an `undefined` result when dispatching + a request with a later block number than the originally requested block number. + - In practice, this should happen rarely if ever. - Migrate all uses of `interface` to `type` ([#6885](https://github.com/MetaMask/core/pull/6885)) ## [21.0.0] diff --git a/packages/eth-json-rpc-middleware/src/retryOnEmpty.ts b/packages/eth-json-rpc-middleware/src/retryOnEmpty.ts index 2b454ff4fe..f9447a8dbc 100644 --- a/packages/eth-json-rpc-middleware/src/retryOnEmpty.ts +++ b/packages/eth-json-rpc-middleware/src/retryOnEmpty.ts @@ -21,11 +21,7 @@ import { timeout } from './utils/timeout'; const log = createModuleLogger(projectLogger, 'retry-on-empty'); // empty values used to determine if a request should be retried // `` comes from https://github.com/ethereum/go-ethereum/issues/16925 -const emptyValues: (string | null | undefined)[] = [ - undefined, - null, - '\u003cnil\u003e', -]; +const emptyValues = [null, '\u003cnil\u003e']; /** * Creates a middleware that retries requests with empty responses. diff --git a/packages/eth-json-rpc-middleware/test/util/helpers.ts b/packages/eth-json-rpc-middleware/test/util/helpers.ts index dd4b6ce2d2..6530868827 100644 --- a/packages/eth-json-rpc-middleware/test/util/helpers.ts +++ b/packages/eth-json-rpc-middleware/test/util/helpers.ts @@ -1,8 +1,5 @@ import { PollingBlockTracker } from '@metamask/eth-block-tracker'; -import { - providerFromEngine, - type InternalProvider, -} from '@metamask/eth-json-rpc-provider'; +import { InternalProvider } from '@metamask/eth-json-rpc-provider'; import { JsonRpcEngine, type JsonRpcMiddleware, @@ -96,7 +93,7 @@ export function createFinalMiddlewareWithDefaultResult< */ export function createProviderAndBlockTracker() { const engine = new JsonRpcEngine(); - const provider = providerFromEngine(engine); + const provider = new InternalProvider({ engine }); const blockTracker = new PollingBlockTracker({ provider, diff --git a/packages/eth-json-rpc-provider/CHANGELOG.md b/packages/eth-json-rpc-provider/CHANGELOG.md index f108a4df33..b564b68d55 100644 --- a/packages/eth-json-rpc-provider/CHANGELOG.md +++ b/packages/eth-json-rpc-provider/CHANGELOG.md @@ -7,11 +7,29 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add `providerFromMiddlewareV2` ([#7001](https://github.com/MetaMask/core/pull/7001)) + - This accepts the new middleware from `@metamask/json-rpc-engine/v2`. + ### Changed - **BREAKING:** Replace `SafeEventEmitterProvider` with `InternalProvider` ([#6796](https://github.com/MetaMask/core/pull/6796)) - The new class is behaviorally equivalent to the previous version except it does not extend `SafeEventEmitter`. - `SafeEventEmitterProvider` is for now still exported as a deprecated alias of `InternalProvider` for backwards compatibility. +- **BREAKING:** Migrate from `JsonRpcEngine` to `JsonRpcEngineV2` ([#7001](https://github.com/MetaMask/core/pull/7001)) + - Legacy `JsonRpcEngine` instances are wrapped in a `JsonRpcEngineV2` internally wherever they appear. + This change should mostly be unobservable. However, due to differences in error handling, this may be breaking for consumers. + +### Deprecated + +- Deprecate `providerFromMiddleware` ([#7001](https://github.com/MetaMask/core/pull/7001)) + - Use `providerFromMiddlewareV2` instead, which supports the new middleware from `@metamask/json-rpc-engine/v2`. + +### Removed + +- **BREAKING:** Remove `providerFromEngine` ([#7001](https://github.com/MetaMask/core/pull/7001)) + - Use `InternalProvider` directly instead. ## [5.0.1] diff --git a/packages/eth-json-rpc-provider/package.json b/packages/eth-json-rpc-provider/package.json index 1796e2e443..c897f59db8 100644 --- a/packages/eth-json-rpc-provider/package.json +++ b/packages/eth-json-rpc-provider/package.json @@ -56,7 +56,7 @@ "@metamask/json-rpc-engine": "^10.1.1", "@metamask/rpc-errors": "^7.0.2", "@metamask/utils": "^11.8.1", - "uuid": "^8.3.2" + "nanoid": "^3.3.8" }, "devDependencies": { "@ethersproject/providers": "^5.7.0", diff --git a/packages/eth-json-rpc-provider/src/index.test.ts b/packages/eth-json-rpc-provider/src/index.test.ts index 1c94f265e9..1fa713b79b 100644 --- a/packages/eth-json-rpc-provider/src/index.test.ts +++ b/packages/eth-json-rpc-provider/src/index.test.ts @@ -2,12 +2,12 @@ import * as allExports from '.'; describe('Package exports', () => { it('has expected exports', () => { - expect(Object.keys(allExports)).toMatchInlineSnapshot(` + expect(Object.keys(allExports).sort()).toMatchInlineSnapshot(` Array [ "InternalProvider", "SafeEventEmitterProvider", - "providerFromEngine", "providerFromMiddleware", + "providerFromMiddlewareV2", ] `); }); diff --git a/packages/eth-json-rpc-provider/src/index.ts b/packages/eth-json-rpc-provider/src/index.ts index a16f98b675..a4bab7a5db 100644 --- a/packages/eth-json-rpc-provider/src/index.ts +++ b/packages/eth-json-rpc-provider/src/index.ts @@ -1,6 +1,5 @@ import { InternalProvider } from './internal-provider'; -export * from './provider-from-engine'; export * from './provider-from-middleware'; /** diff --git a/packages/eth-json-rpc-provider/src/internal-provider.test.ts b/packages/eth-json-rpc-provider/src/internal-provider.test.ts index 3e6a897746..6e102647d9 100644 --- a/packages/eth-json-rpc-provider/src/internal-provider.test.ts +++ b/packages/eth-json-rpc-provider/src/internal-provider.test.ts @@ -1,13 +1,13 @@ import { Web3Provider } from '@ethersproject/providers'; import EthQuery from '@metamask/eth-query'; import EthJsQuery from '@metamask/ethjs-query'; -import { JsonRpcEngine } from '@metamask/json-rpc-engine'; -import { providerErrors } from '@metamask/rpc-errors'; +import { asV2Middleware, JsonRpcEngine } from '@metamask/json-rpc-engine'; +import type { JsonRpcMiddleware } from '@metamask/json-rpc-engine/v2'; +import { JsonRpcEngineV2 } from '@metamask/json-rpc-engine/v2'; +import { providerErrors, rpcErrors } from '@metamask/rpc-errors'; import { type JsonRpcRequest, type Json } from '@metamask/utils'; import { BrowserProvider } from 'ethers'; import { promisify } from 'util'; -// eslint-disable-next-line import-x/namespace -import * as uuid from 'uuid'; import { InternalProvider, @@ -16,29 +16,55 @@ import { jest.mock('uuid'); -/** - * Creates a mock JSON-RPC engine that returns a predefined response for a specific method. - * - * @param method - The RPC method to mock. - * @param response - The response to return for the mocked method. - * @returns A JSON-RPC engine instance with the mocked method. - */ -function createMockEngine(method: string, response: Json) { +type ResultParam = Json | ((req?: JsonRpcRequest) => Json); + +const createLegacyEngine = (method: string, result: ResultParam) => { const engine = new JsonRpcEngine(); engine.push((req, res, next, end) => { if (req.method === method) { - res.result = response; + res.result = typeof result === 'function' ? result(req) : result; return end(); } return next(); }); return engine; -} +}; + +const createV2Engine = (method: string, result: ResultParam) => { + return JsonRpcEngineV2.create>({ + middleware: [ + ({ request, next }) => { + if (request.method === method) { + return typeof result === 'function' ? result(request) : result; + } + return next(); + }, + ], + }); +}; + +describe('legacy constructor', () => { + it('can be constructed with an engine', () => { + const provider = new InternalProvider({ + engine: createLegacyEngine('eth_blockNumber', 42), + }); + expect(provider).toBeDefined(); + }); +}); -describe('InternalProvider', () => { +describe.each([ + { + createRpcHandler: createLegacyEngine, + name: 'JsonRpcEngine', + }, + { + createRpcHandler: createV2Engine, + name: 'JsonRpcServer', + }, +] as const)('InternalProvider with $name', ({ createRpcHandler }) => { it('returns the correct block number with @metamask/eth-query', async () => { const provider = new InternalProvider({ - engine: createMockEngine('eth_blockNumber', 42), + engine: createRpcHandler('eth_blockNumber', 42), }); const ethQuery = new EthQuery(provider); @@ -49,7 +75,7 @@ describe('InternalProvider', () => { it('returns the correct block number with @metamask/ethjs-query', async () => { const provider = new InternalProvider({ - engine: createMockEngine('eth_blockNumber', 42), + engine: createRpcHandler('eth_blockNumber', 42), }); const ethJsQuery = new EthJsQuery(provider); @@ -60,7 +86,7 @@ describe('InternalProvider', () => { it('returns the correct block number with Web3Provider', async () => { const provider = new InternalProvider({ - engine: createMockEngine('eth_blockNumber', 42), + engine: createRpcHandler('eth_blockNumber', 42), }); const web3Provider = new Web3Provider(provider); @@ -71,27 +97,27 @@ describe('InternalProvider', () => { it('returns the correct block number with BrowserProvider', async () => { const provider = new InternalProvider({ - engine: createMockEngine('eth_blockNumber', 42), + engine: createRpcHandler('eth_blockNumber', 42), }); const browserProvider = new BrowserProvider(provider); const response = await browserProvider.send('eth_blockNumber', []); expect(response).toBe(42); + + browserProvider.destroy(); }); describe('request', () => { it('handles a successful JSON-RPC object request', async () => { - const engine = new JsonRpcEngine(); let req: JsonRpcRequest | undefined; - engine.push((_req, res, _next, end) => { - req = _req; - res.result = 42; - end(); + const rpcHandler = createRpcHandler('test', (request) => { + req = request; + return 42; }); - const provider = new InternalProvider({ engine }); - const exampleRequest = { - id: 1, + const provider = new InternalProvider({ engine: rpcHandler }); + const request = { + id: '1', jsonrpc: '2.0' as const, method: 'test', params: { @@ -100,10 +126,10 @@ describe('InternalProvider', () => { }, }; - const result = await provider.request(exampleRequest); + const result = await provider.request(request); expect(req).toStrictEqual({ - id: 1, + id: expect.any(String), jsonrpc: '2.0' as const, method: 'test', params: { @@ -115,27 +141,24 @@ describe('InternalProvider', () => { }); it('handles a successful EIP-1193 object request', async () => { - const engine = new JsonRpcEngine(); let req: JsonRpcRequest | undefined; - engine.push((_req, res, _next, end) => { - req = _req; - res.result = 42; - end(); + const rpcHandler = createRpcHandler('test', (request) => { + req = request; + return 42; }); - const provider = new InternalProvider({ engine }); - const exampleRequest = { + const provider = new InternalProvider({ engine: rpcHandler }); + const request = { method: 'test', params: { param1: 'value1', param2: 'value2', }, }; - jest.spyOn(uuid, 'v4').mockReturnValueOnce('mock-id'); - const result = await provider.request(exampleRequest); + const result = await provider.request(request); expect(req).toStrictEqual({ - id: 'mock-id', + id: expect.any(String), jsonrpc: '2.0' as const, method: 'test', params: { @@ -147,61 +170,43 @@ describe('InternalProvider', () => { }); it('handles a failure with a non-JSON-RPC error', async () => { - const engine = new JsonRpcEngine(); - engine.push((_req, _res, _next, end) => { - end( - providerErrors.custom({ - code: 1001, - message: 'Test error', - data: { - cause: 'Test cause', - }, - }), - ); + const rpcHandler = createRpcHandler('test', () => { + throw providerErrors.custom({ + code: 1001, + message: 'Test error', + data: { cause: 'Test cause' }, + }); }); - const provider = new InternalProvider({ engine }); - const exampleRequest = { - id: 1, + const provider = new InternalProvider({ engine: rpcHandler }); + const request = { + id: '1', jsonrpc: '2.0' as const, method: 'test', }; - await expect(async () => - provider.request(exampleRequest), - ).rejects.toThrow( - expect.objectContaining({ + await expect(async () => provider.request(request)).rejects.toThrow( + providerErrors.custom({ code: 1001, message: 'Test error', data: { cause: 'Test cause' }, - stack: expect.stringContaining('internal-provider.test.ts:'), }), ); }); it('handles a failure with a JSON-RPC error', async () => { - const engine = new JsonRpcEngine(); - engine.push(() => { + const rpcHandler = createRpcHandler('test', () => { throw new Error('Test error'); }); - const provider = new InternalProvider({ engine }); - const exampleRequest = { - id: 1, + const provider = new InternalProvider({ engine: rpcHandler }); + const request = { + id: '1', jsonrpc: '2.0' as const, method: 'test', }; - await expect(async () => - provider.request(exampleRequest), - ).rejects.toThrow( - expect.objectContaining({ - code: -32603, + await expect(async () => provider.request(request)).rejects.toThrow( + rpcErrors.internal({ message: 'Test error', - data: { - cause: expect.objectContaining({ - stack: expect.stringContaining('internal-provider.test.ts:'), - message: 'Test error', - }), - }, }), ); }); @@ -209,17 +214,15 @@ describe('InternalProvider', () => { describe('sendAsync', () => { it('handles a successful JSON-RPC object request', async () => { - const engine = new JsonRpcEngine(); let req: JsonRpcRequest | undefined; - engine.push((_req, res, _next, end) => { - req = _req; - res.result = 42; - end(); + const rpcHandler = createRpcHandler('test', (request) => { + req = request; + return 42; }); - const provider = new InternalProvider({ engine }); + const provider = new InternalProvider({ engine: rpcHandler }); const promisifiedSendAsync = promisify(provider.sendAsync); - const exampleRequest = { - id: 1, + const request = { + id: '1', jsonrpc: '2.0' as const, method: 'test', params: { @@ -228,10 +231,10 @@ describe('InternalProvider', () => { }, }; - const response = await promisifiedSendAsync(exampleRequest); + const response = await promisifiedSendAsync(request); expect(req).toStrictEqual({ - id: 1, + id: expect.any(String), jsonrpc: '2.0' as const, method: 'test', params: { @@ -243,28 +246,25 @@ describe('InternalProvider', () => { }); it('handles a successful EIP-1193 object request', async () => { - const engine = new JsonRpcEngine(); let req: JsonRpcRequest | undefined; - engine.push((_req, res, _next, end) => { - req = _req; - res.result = 42; - end(); + const rpcHandler = createRpcHandler('test', (request) => { + req = request; + return 42; }); - const provider = new InternalProvider({ engine }); + const provider = new InternalProvider({ engine: rpcHandler }); const promisifiedSendAsync = promisify(provider.sendAsync); - const exampleRequest = { + const request = { method: 'test', params: { param1: 'value1', param2: 'value2', }, }; - jest.spyOn(uuid, 'v4').mockReturnValueOnce('mock-id'); - const response = await promisifiedSendAsync(exampleRequest); + const response = await promisifiedSendAsync(request); expect(req).toStrictEqual({ - id: 'mock-id', + id: expect.any(String), jsonrpc: '2.0' as const, method: 'test', params: { @@ -276,51 +276,74 @@ describe('InternalProvider', () => { }); it('handles a failed request', async () => { - const engine = new JsonRpcEngine(); - engine.push((_req, _res, _next, _end) => { + const rpcHandler = createRpcHandler('test', () => { throw new Error('Test error'); }); - const provider = new InternalProvider({ engine }); + const provider = new InternalProvider({ engine: rpcHandler }); const promisifiedSendAsync = promisify(provider.sendAsync); - const exampleRequest = { - id: 1, + const request = { + id: '1', jsonrpc: '2.0' as const, method: 'test', }; - await expect(async () => - promisifiedSendAsync(exampleRequest), - ).rejects.toThrow('Test error'); + await expect(async () => promisifiedSendAsync(request)).rejects.toThrow( + 'Test error', + ); + }); + + it('handles an error thrown by the JSON-RPC handler', async () => { + let rpcHandler = createRpcHandler('test', () => null); + // Transform the engine into a server so we can mock the "handle" method. + // The "handle" method should never throw, but we should be resilient to it anyway. + rpcHandler = + // eslint-disable-next-line jest/no-conditional-in-test + 'push' in rpcHandler + ? JsonRpcEngineV2.create({ middleware: [asV2Middleware(rpcHandler)] }) + : rpcHandler; + jest + .spyOn(rpcHandler, 'handle') + .mockRejectedValue(new Error('Test error')); + const provider = new InternalProvider({ engine: rpcHandler }); + const promisifiedSendAsync = promisify(provider.sendAsync); + const request = { + id: '1', + jsonrpc: '2.0' as const, + method: 'test', + }; + + await expect(async () => promisifiedSendAsync(request)).rejects.toThrow( + 'Test error', + ); }); }); describe('send', () => { it('throws if a callback is not provided', () => { - const engine = new JsonRpcEngine(); - const provider = new InternalProvider({ engine }); - const exampleRequest = { - id: 1, + const rpcHandler = createRpcHandler('test', 42); + const provider = new InternalProvider({ engine: rpcHandler }); + const request = { + id: '1', jsonrpc: '2.0' as const, method: 'test', }; - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - expect(() => (provider.send as any)(exampleRequest)).toThrow(''); + // @ts-expect-error - Destructive testing. + expect(() => provider.send(request)).toThrow( + 'Must provide callback to "send" method.', + ); }); it('handles a successful JSON-RPC object request', async () => { - const engine = new JsonRpcEngine(); let req: JsonRpcRequest | undefined; - engine.push((_req, res, _next, end) => { - req = _req; - res.result = 42; - end(); + const rpcHandler = createRpcHandler('test', (request) => { + req = request; + return 42; }); - const provider = new InternalProvider({ engine }); + const provider = new InternalProvider({ engine: rpcHandler }); const promisifiedSend = promisify(provider.send); - const exampleRequest = { - id: 1, + const request = { + id: '1', jsonrpc: '2.0' as const, method: 'test', params: { @@ -329,10 +352,10 @@ describe('InternalProvider', () => { }, }; - const response = await promisifiedSend(exampleRequest); + const response = await promisifiedSend(request); expect(req).toStrictEqual({ - id: 1, + id: expect.any(String), jsonrpc: '2.0' as const, method: 'test', params: { @@ -344,28 +367,25 @@ describe('InternalProvider', () => { }); it('handles a successful EIP-1193 object request', async () => { - const engine = new JsonRpcEngine(); let req: JsonRpcRequest | undefined; - engine.push((_req, res, _next, end) => { - req = _req; - res.result = 42; - end(); + const rpcHandler = createRpcHandler('test', (request) => { + req = request; + return 42; }); - const provider = new InternalProvider({ engine }); + const provider = new InternalProvider({ engine: rpcHandler }); const promisifiedSend = promisify(provider.send); - const exampleRequest = { + const request = { method: 'test', params: { param1: 'value1', param2: 'value2', }, }; - jest.spyOn(uuid, 'v4').mockReturnValueOnce('mock-id'); - const response = await promisifiedSend(exampleRequest); + const response = await promisifiedSend(request); expect(req).toStrictEqual({ - id: 'mock-id', + id: expect.any(String), jsonrpc: '2.0' as const, method: 'test', params: { @@ -377,19 +397,18 @@ describe('InternalProvider', () => { }); it('handles a failed request', async () => { - const engine = new JsonRpcEngine(); - engine.push((_req, _res, _next, _end) => { + const rpcHandler = createRpcHandler('test', () => { throw new Error('Test error'); }); - const provider = new InternalProvider({ engine }); + const provider = new InternalProvider({ engine: rpcHandler }); const promisifiedSend = promisify(provider.send); - const exampleRequest = { - id: 1, + const request = { + id: '1', jsonrpc: '2.0' as const, method: 'test', }; - await expect(async () => promisifiedSend(exampleRequest)).rejects.toThrow( + await expect(async () => promisifiedSend(request)).rejects.toThrow( 'Test error', ); }); @@ -398,7 +417,6 @@ describe('InternalProvider', () => { describe('convertEip1193RequestToJsonRpcRequest', () => { it('generates a unique id if id is not provided', () => { - jest.spyOn(uuid, 'v4').mockReturnValueOnce('mock-id'); const eip1193Request = { method: 'test', params: { param1: 'value1', param2: 'value2' }, @@ -408,24 +426,7 @@ describe('convertEip1193RequestToJsonRpcRequest', () => { convertEip1193RequestToJsonRpcRequest(eip1193Request); expect(jsonRpcRequest).toStrictEqual({ - id: 'mock-id', - jsonrpc: '2.0', - method: 'test', - params: { param1: 'value1', param2: 'value2' }, - }); - }); - - it('uses the provided id if id is provided', () => { - const eip1193Request = { - id: '123', - method: 'test', - params: { param1: 'value1', param2: 'value2' }, - }; - const jsonRpcRequest = - convertEip1193RequestToJsonRpcRequest(eip1193Request); - - expect(jsonRpcRequest).toStrictEqual({ - id: '123', + id: expect.any(String), jsonrpc: '2.0', method: 'test', params: { param1: 'value1', param2: 'value2' }, @@ -433,7 +434,6 @@ describe('convertEip1193RequestToJsonRpcRequest', () => { }); it('uses the default jsonrpc version if not provided', () => { - jest.spyOn(uuid, 'v4').mockReturnValueOnce('mock-id'); const eip1193Request = { method: 'test', params: { param1: 'value1', param2: 'value2' }, @@ -443,7 +443,7 @@ describe('convertEip1193RequestToJsonRpcRequest', () => { convertEip1193RequestToJsonRpcRequest(eip1193Request); expect(jsonRpcRequest).toStrictEqual({ - id: 'mock-id', + id: expect.any(String), jsonrpc: '2.0', method: 'test', params: { param1: 'value1', param2: 'value2' }, @@ -451,7 +451,6 @@ describe('convertEip1193RequestToJsonRpcRequest', () => { }); it('uses the provided jsonrpc version if provided', () => { - jest.spyOn(uuid, 'v4').mockReturnValueOnce('mock-id'); const eip1193Request = { jsonrpc: '2.0' as const, method: 'test', @@ -462,7 +461,7 @@ describe('convertEip1193RequestToJsonRpcRequest', () => { convertEip1193RequestToJsonRpcRequest(eip1193Request); expect(jsonRpcRequest).toStrictEqual({ - id: 'mock-id', + id: expect.any(String), jsonrpc: '2.0', method: 'test', params: { param1: 'value1', param2: 'value2' }, @@ -470,7 +469,6 @@ describe('convertEip1193RequestToJsonRpcRequest', () => { }); it('uses an empty object as params if not provided', () => { - jest.spyOn(uuid, 'v4').mockReturnValueOnce('mock-id'); const eip1193Request = { method: 'test', }; @@ -479,7 +477,7 @@ describe('convertEip1193RequestToJsonRpcRequest', () => { convertEip1193RequestToJsonRpcRequest(eip1193Request); expect(jsonRpcRequest).toStrictEqual({ - id: 'mock-id', + id: expect.any(String), jsonrpc: '2.0', method: 'test', }); diff --git a/packages/eth-json-rpc-provider/src/internal-provider.ts b/packages/eth-json-rpc-provider/src/internal-provider.ts index 9c1a59c0df..920416c731 100644 --- a/packages/eth-json-rpc-provider/src/internal-provider.ts +++ b/packages/eth-json-rpc-provider/src/internal-provider.ts @@ -1,49 +1,35 @@ -import type { JsonRpcEngine } from '@metamask/json-rpc-engine'; -import { JsonRpcError } from '@metamask/rpc-errors'; +import { asV2Middleware, type JsonRpcEngine } from '@metamask/json-rpc-engine'; import type { - Json, - JsonRpcId, - JsonRpcParams, - JsonRpcRequest, - JsonRpcVersion2, + ContextConstraint, + MiddlewareContext, +} from '@metamask/json-rpc-engine/v2'; +import { JsonRpcEngineV2 } from '@metamask/json-rpc-engine/v2'; +import type { JsonRpcSuccess } from '@metamask/utils'; +import { + type Json, + type JsonRpcId, + type JsonRpcParams, + type JsonRpcRequest, + type JsonRpcVersion2, } from '@metamask/utils'; -import { v4 as uuidV4 } from 'uuid'; +import { nanoid } from 'nanoid'; /** * A JSON-RPC request conforming to the EIP-1193 specification. */ -type Eip1193Request = { +type Eip1193Request = { id?: JsonRpcId; jsonrpc?: JsonRpcVersion2; method: string; params?: Params; }; -/** - * Converts an EIP-1193 request to a JSON-RPC request. - * - * @param eip1193Request - The EIP-1193 request to convert. - * @returns The corresponding JSON-RPC request. - */ -export function convertEip1193RequestToJsonRpcRequest< - Params extends JsonRpcParams, ->( - eip1193Request: Eip1193Request, -): JsonRpcRequest> { - const { id = uuidV4(), jsonrpc = '2.0', method, params } = eip1193Request; - return params - ? { - id, - jsonrpc, - method, - params, - } - : { - id, - jsonrpc, - method, - }; -} +type Options< + Request extends JsonRpcRequest = JsonRpcRequest, + Context extends ContextConstraint = MiddlewareContext, +> = { + engine: JsonRpcEngine | JsonRpcEngineV2; +}; /** * An Ethereum provider. @@ -52,16 +38,21 @@ export function convertEip1193RequestToJsonRpcRequest< * It is not compliant with any Ethereum provider standard. */ export class InternalProvider { - readonly #engine: JsonRpcEngine; + readonly #engine: JsonRpcEngineV2; /** - * Construct a InternalProvider from a JSON-RPC engine. + * Construct a InternalProvider from a JSON-RPC server or legacy engine. * * @param options - Options. * @param options.engine - The JSON-RPC engine used to process requests. */ - constructor({ engine }: { engine: JsonRpcEngine }) { - this.#engine = engine; + constructor({ engine }: Options) { + this.#engine = + 'push' in engine + ? JsonRpcEngineV2.create({ + middleware: [asV2Middleware(engine)], + }) + : engine; } /** @@ -75,24 +66,7 @@ export class InternalProvider { ): Promise { const jsonRpcRequest = convertEip1193RequestToJsonRpcRequest(eip1193Request); - const response = await this.#engine.handle< - Params | Record, - Result - >(jsonRpcRequest); - - if ('result' in response) { - return response.result; - } - - const error = new JsonRpcError( - response.error.code, - response.error.message, - response.error.data, - ); - if ('stack' in response.error) { - error.stack = response.error.stack; - } - throw error; + return (await this.#handle(jsonRpcRequest)).result; } /** @@ -103,17 +77,18 @@ export class InternalProvider { * * @param eip1193Request - The request to send. * @param callback - A function that is called upon the success or failure of the request. - * @deprecated Please use `request` instead. + * @deprecated Use {@link request} instead. This method is retained solely for backwards + * compatibility with certain libraries. */ sendAsync = ( eip1193Request: Eip1193Request, - // TODO: Replace `any` with type + // Non-polluting `any` that acts like a constraint. // eslint-disable-next-line @typescript-eslint/no-explicit-any callback: (error: unknown, providerRes?: any) => void, ) => { const jsonRpcRequest = convertEip1193RequestToJsonRpcRequest(eip1193Request); - this.#engine.handle(jsonRpcRequest, callback); + this.#handleWithCallback(jsonRpcRequest, callback); }; /** @@ -124,7 +99,8 @@ export class InternalProvider { * * @param eip1193Request - The request to send. * @param callback - A function that is called upon the success or failure of the request. - * @deprecated Please use `request` instead. + * @deprecated Use {@link request} instead. This method is retained solely for backwards + * compatibility with certain libraries. */ send = ( eip1193Request: Eip1193Request, @@ -137,6 +113,62 @@ export class InternalProvider { } const jsonRpcRequest = convertEip1193RequestToJsonRpcRequest(eip1193Request); - this.#engine.handle(jsonRpcRequest, callback); + this.#handleWithCallback(jsonRpcRequest, callback); }; + + readonly #handle = async ( + jsonRpcRequest: JsonRpcRequest, + ): Promise> => { + const { id, jsonrpc } = jsonRpcRequest; + // This typecast is technicaly unsafe, but we need it to preserve the provider's + // public interface, which allows you to typecast results. + const result = (await this.#engine.handle( + jsonRpcRequest, + )) as unknown as Result; + + return { + id, + jsonrpc, + result, + }; + }; + + readonly #handleWithCallback = ( + jsonRpcRequest: JsonRpcRequest, + callback: (error: unknown, providerRes?: unknown) => void, + ): void => { + /* eslint-disable promise/no-callback-in-promise */ + this.#handle(jsonRpcRequest) + // A resolution will always be a successful response + .then((response) => callback(null, response)) + .catch((error) => { + callback(error); + }); + /* eslint-enable promise/no-callback-in-promise */ + }; +} + +/** + * Convert an EIP-1193 request to a JSON-RPC request. + * + * @param eip1193Request - The EIP-1193 request to convert. + * @returns The JSON-RPC request. + */ +export function convertEip1193RequestToJsonRpcRequest( + eip1193Request: Eip1193Request, +): JsonRpcRequest { + const { id = nanoid(), jsonrpc = '2.0', method, params } = eip1193Request; + + return params + ? { + id, + jsonrpc, + method, + params, + } + : { + id, + jsonrpc, + method, + }; } diff --git a/packages/eth-json-rpc-provider/src/provider-from-engine.test.ts b/packages/eth-json-rpc-provider/src/provider-from-engine.test.ts deleted file mode 100644 index ca90a1deac..0000000000 --- a/packages/eth-json-rpc-provider/src/provider-from-engine.test.ts +++ /dev/null @@ -1,46 +0,0 @@ -import { JsonRpcEngine } from '@metamask/json-rpc-engine'; -import { providerErrors } from '@metamask/rpc-errors'; - -import { providerFromEngine } from './provider-from-engine'; - -describe('providerFromEngine', () => { - it('handle a successful request', async () => { - const engine = new JsonRpcEngine(); - engine.push((_req, res, _next, end) => { - res.result = 42; - end(); - }); - const provider = providerFromEngine(engine); - const exampleRequest = { - id: 1, - jsonrpc: '2.0' as const, - method: 'test', - }; - - const response = await provider.request(exampleRequest); - - expect(response).toBe(42); - }); - - it('handle a failed request', async () => { - const engine = new JsonRpcEngine(); - engine.push((_req, _res, _next, end) => { - end( - providerErrors.custom({ - code: 1001, - message: 'Test error', - }), - ); - }); - const provider = providerFromEngine(engine); - const exampleRequest = { - id: 1, - jsonrpc: '2.0' as const, - method: 'test', - }; - - await expect(async () => provider.request(exampleRequest)).rejects.toThrow( - 'Test error', - ); - }); -}); diff --git a/packages/eth-json-rpc-provider/src/provider-from-engine.ts b/packages/eth-json-rpc-provider/src/provider-from-engine.ts deleted file mode 100644 index 0f90662507..0000000000 --- a/packages/eth-json-rpc-provider/src/provider-from-engine.ts +++ /dev/null @@ -1,13 +0,0 @@ -import type { JsonRpcEngine } from '@metamask/json-rpc-engine'; - -import { InternalProvider } from './internal-provider'; - -/** - * Construct an Ethereum provider from the given JSON-RPC engine. - * - * @param engine - The JSON-RPC engine to construct a provider from. - * @returns An Ethereum provider. - */ -export function providerFromEngine(engine: JsonRpcEngine): InternalProvider { - return new InternalProvider({ engine }); -} diff --git a/packages/eth-json-rpc-provider/src/provider-from-middleware.test.ts b/packages/eth-json-rpc-provider/src/provider-from-middleware.test.ts index c3e73d3153..6c20713e0b 100644 --- a/packages/eth-json-rpc-provider/src/provider-from-middleware.test.ts +++ b/packages/eth-json-rpc-provider/src/provider-from-middleware.test.ts @@ -1,13 +1,21 @@ -import type { JsonRpcMiddleware } from '@metamask/json-rpc-engine'; +import type { JsonRpcMiddleware as LegacyJsonRpcMiddleware } from '@metamask/json-rpc-engine'; +import type { JsonRpcMiddleware } from '@metamask/json-rpc-engine/v2'; import { providerErrors } from '@metamask/rpc-errors'; +import type { Json, JsonRpcParams, JsonRpcRequest } from '@metamask/utils'; -import { providerFromMiddleware } from './provider-from-middleware'; +import { + providerFromMiddleware, + providerFromMiddlewareV2, +} from './provider-from-middleware'; describe('providerFromMiddleware', () => { it('handle a successful request', async () => { - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const middleware: JsonRpcMiddleware = (_req, res, _next, end) => { + const middleware: LegacyJsonRpcMiddleware = ( + _req, + res, + _next, + end, + ) => { res.result = 42; end(); }; @@ -43,3 +51,35 @@ describe('providerFromMiddleware', () => { ); }); }); + +describe('providerFromMiddlewareV2', () => { + it('handle a successful request', async () => { + const middleware: JsonRpcMiddleware = () => 42; + const provider = providerFromMiddlewareV2(middleware); + const exampleRequest = { + id: 1, + jsonrpc: '2.0' as const, + method: 'test', + }; + + const response = await provider.request(exampleRequest); + + expect(response).toBe(42); + }); + + it('handle a failed request', async () => { + const middleware: JsonRpcMiddleware = () => { + throw new Error('Test error'); + }; + const provider = providerFromMiddlewareV2(middleware); + const exampleRequest = { + id: 1, + jsonrpc: '2.0' as const, + method: 'test', + }; + + await expect(async () => provider.request(exampleRequest)).rejects.toThrow( + 'Test error', + ); + }); +}); diff --git a/packages/eth-json-rpc-provider/src/provider-from-middleware.ts b/packages/eth-json-rpc-provider/src/provider-from-middleware.ts index e77d487464..123e2772f2 100644 --- a/packages/eth-json-rpc-provider/src/provider-from-middleware.ts +++ b/packages/eth-json-rpc-provider/src/provider-from-middleware.ts @@ -1,22 +1,57 @@ -import { JsonRpcEngine } from '@metamask/json-rpc-engine'; -import type { JsonRpcMiddleware } from '@metamask/json-rpc-engine'; -import type { Json, JsonRpcParams } from '@metamask/utils'; +import { asV2Middleware } from '@metamask/json-rpc-engine'; +import type { JsonRpcMiddleware as LegacyJsonRpcMiddleware } from '@metamask/json-rpc-engine'; +import type { + JsonRpcMiddleware, + ResultConstraint, +} from '@metamask/json-rpc-engine/v2'; +import { + JsonRpcEngineV2, + type ContextConstraint, +} from '@metamask/json-rpc-engine/v2'; +import type { Json, JsonRpcParams, JsonRpcRequest } from '@metamask/utils'; -import type { InternalProvider } from './internal-provider'; -import { providerFromEngine } from './provider-from-engine'; +import { InternalProvider } from './internal-provider'; /** * Construct an Ethereum provider from the given middleware. * * @param middleware - The middleware to construct a provider from. * @returns An Ethereum provider. + * @deprecated Use `JsonRpcEngineV2` middleware and {@link providerFromMiddlewareV2} instead. */ export function providerFromMiddleware< Params extends JsonRpcParams, Result extends Json, ->(middleware: JsonRpcMiddleware): InternalProvider { - const engine: JsonRpcEngine = new JsonRpcEngine(); - engine.push(middleware); - const provider: InternalProvider = providerFromEngine(engine); - return provider; +>(middleware: LegacyJsonRpcMiddleware): InternalProvider { + return providerFromMiddlewareV2( + // This function is generic on the Params and Result types to match the legacy JsonRpcMiddleware type. + // However, since the V2 JsonRpcMiddleware type is not generic on the Params, we need to elide this + // parameter by upcasting the request type to JsonRpcRequest, or we get an error due to contravariance + // since JsonRpcRequest is not assignable to JsonRpcRequest. + asV2Middleware(middleware) as JsonRpcMiddleware, + ); +} + +/** + * Construct an Ethereum provider from the given middleware. + * + * @param middleware - The middleware to construct a provider from. + * @returns An Ethereum provider. + */ +export function providerFromMiddlewareV2< + Request extends JsonRpcRequest, + Middleware extends JsonRpcMiddleware< + Request, + ResultConstraint, + ContextConstraint + >, +>(middleware: Middleware): InternalProvider { + return new InternalProvider({ + engine: JsonRpcEngineV2.create({ + // This function is generic in order to accept middleware functions with narrower types than + // the plain JsonRpcMiddleware type. However, since InternalProvider is non-generic, + // we need to upcast the middleware to avoid a type error. + middleware: [middleware as JsonRpcMiddleware], + }), + }); } diff --git a/packages/json-rpc-engine/CHANGELOG.md b/packages/json-rpc-engine/CHANGELOG.md index da4d5238af..2f2d9c57f0 100644 --- a/packages/json-rpc-engine/CHANGELOG.md +++ b/packages/json-rpc-engine/CHANGELOG.md @@ -9,9 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- `JsonRpcEngineV2` ([#6176](https://github.com/MetaMask/core/pull/6176), [#6971](https://github.com/MetaMask/core/pull/6971), [#6975](https://github.com/MetaMask/core/pull/6975), [#6990](https://github.com/MetaMask/core/pull/6990), [#6991](https://github.com/MetaMask/core/pull/6991), [#7032](https://github.com/MetaMask/core/pull/7032)) - - This is a complete rewrite of `JsonRpcEngine`, intended to replace the original implementation. - See the readme for details. +- Add `JsonRpcEngineV2` ([#6176](https://github.com/MetaMask/core/pull/6176), [#6971](https://github.com/MetaMask/core/pull/6971), [#6975](https://github.com/MetaMask/core/pull/6975), [#6990](https://github.com/MetaMask/core/pull/6990), [#6991](https://github.com/MetaMask/core/pull/6991), [#7001](https://github.com/MetaMask/core/pull/7001), [#7032](https://github.com/MetaMask/core/pull/7032)) + - This is a complete rewrite of `JsonRpcEngine`, intended to replace the original implementation. See the readme for details. ## [10.1.1] diff --git a/packages/json-rpc-engine/src/asV2Middleware.test.ts b/packages/json-rpc-engine/src/asV2Middleware.test.ts index d268b4cf3f..b91ef4c557 100644 --- a/packages/json-rpc-engine/src/asV2Middleware.test.ts +++ b/packages/json-rpc-engine/src/asV2Middleware.test.ts @@ -171,6 +171,21 @@ describe('asV2Middleware', () => { ); }); + it('does not forward undefined errors from legacy middleware', async () => { + const legacyMiddleware = jest.fn((_req, res, _next, end) => { + res.error = undefined; + res.result = 42; + end(); + }); + + const v2Engine = JsonRpcEngineV2.create({ + middleware: [asV2Middleware(legacyMiddleware)], + }); + + const result = await v2Engine.handle(makeRequest()); + expect(result).toBe(42); + }); + it('allows v2 engine to continue when legacy middleware does not end', async () => { const legacyMiddleware = jest.fn((_req, _res, next) => { next(); diff --git a/packages/json-rpc-engine/src/asV2Middleware.ts b/packages/json-rpc-engine/src/asV2Middleware.ts index 34f4bd3487..14333cbb4e 100644 --- a/packages/json-rpc-engine/src/asV2Middleware.ts +++ b/packages/json-rpc-engine/src/asV2Middleware.ts @@ -19,7 +19,7 @@ import { fromLegacyRequest, propagateToContext, propagateToRequest, - unserializeError, + deserializeError, } from './v2/compatibility-utils'; import type { // Used in docs. @@ -49,8 +49,9 @@ export function asV2Middleware< export function asV2Middleware< Params extends JsonRpcParams, Request extends JsonRpcRequest, + Result extends Json, >( - ...middleware: LegacyMiddleware[] + ...middleware: LegacyMiddleware[] ): JsonRpcMiddleware; /** @@ -69,7 +70,9 @@ export function asV2Middleware< ): JsonRpcMiddleware { const legacyMiddleware = typeof engineOrMiddleware === 'function' - ? mergeMiddleware([engineOrMiddleware, ...rest]) + ? // mergeMiddleware uses .asMiddleware() internally, which is necessary for our purposes. + // See comment on this below. + mergeMiddleware([engineOrMiddleware, ...rest]) : engineOrMiddleware.asMiddleware(); return async ({ request, context, next }) => { @@ -101,8 +104,11 @@ export function asV2Middleware< }); propagateToContext(req, context); - if (hasProperty(response, 'error')) { - throw unserializeError(response.error); + // Mimic the behavior of JsonRpcEngine.#handle(), which only treats truthy errors as errors. + // Legacy middleware may violate the invariant that response objects have either a result or an + // error property. In practice, we may see response objects with results and `{ error: undefined }`. + if (hasProperty(response, 'error') && response.error) { + throw deserializeError(response.error); } else if (hasProperty(response, 'result')) { return response.result as ResultConstraint; } diff --git a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts index fe89e800ef..a6c4dce794 100644 --- a/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts +++ b/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts @@ -73,6 +73,9 @@ type ConstructorOptions< >; }; +/** + * The request type of a middleware. + */ export type RequestOf = Middleware extends JsonRpcMiddleware< infer Request, @@ -91,11 +94,23 @@ type ContextOf = ? C : never; -export type MergedContextOf< - // Non-polluting `any` constraint. - // eslint-disable-next-line @typescript-eslint/no-explicit-any - Middleware extends JsonRpcMiddleware, -> = MergeContexts>; +/** + * A constraint for {@link JsonRpcMiddleware} generic parameters. + */ +// Non-polluting `any` constraint. +/* eslint-disable @typescript-eslint/no-explicit-any */ +export type MiddlewareConstraint = JsonRpcMiddleware< + any, + ResultConstraint, + MiddlewareContext +>; +/* eslint-enable @typescript-eslint/no-explicit-any */ + +/** + * The context supertype of a middleware type. + */ +export type MergedContextOf = + MergeContexts>; const INVALID_ENGINE = Symbol('Invalid engine'); diff --git a/packages/json-rpc-engine/src/v2/JsonRpcServer.ts b/packages/json-rpc-engine/src/v2/JsonRpcServer.ts index bc0bbd0ed6..dbf782aec6 100644 --- a/packages/json-rpc-engine/src/v2/JsonRpcServer.ts +++ b/packages/json-rpc-engine/src/v2/JsonRpcServer.ts @@ -11,8 +11,8 @@ import { hasProperty, isObject } from '@metamask/utils'; import type { JsonRpcMiddleware, MergedContextOf, + MiddlewareConstraint, RequestOf, - ResultConstraint, } from './JsonRpcEngineV2'; import { JsonRpcEngineV2 } from './JsonRpcEngineV2'; import type { JsonRpcCall } from './utils'; @@ -20,7 +20,7 @@ import { getUniqueId } from '../getUniqueId'; type OnError = (error: unknown) => void; -type Options = { +type Options = { onError?: OnError; } & ( | { @@ -58,14 +58,7 @@ const jsonrpc = '2.0' as const; * ``` */ export class JsonRpcServer< - Middleware extends JsonRpcMiddleware< - // Non-polluting `any` constraint. - /* eslint-disable @typescript-eslint/no-explicit-any */ - any, - ResultConstraint, - any - /* eslint-enable @typescript-eslint/no-explicit-any */ - > = JsonRpcMiddleware, + Middleware extends MiddlewareConstraint = JsonRpcMiddleware, > { readonly #engine: JsonRpcEngineV2< RequestOf, diff --git a/packages/json-rpc-engine/src/v2/MiddlewareContext.ts b/packages/json-rpc-engine/src/v2/MiddlewareContext.ts index 8eb291dcc7..073b7e0c76 100644 --- a/packages/json-rpc-engine/src/v2/MiddlewareContext.ts +++ b/packages/json-rpc-engine/src/v2/MiddlewareContext.ts @@ -122,6 +122,9 @@ export type MergeContexts = ExcludeNever>>> >; +/** + * A constraint for {@link MiddlewareContext} generic parameters. + */ // Non-polluting `any` constraint. // eslint-disable-next-line @typescript-eslint/no-explicit-any export type ContextConstraint = MiddlewareContext; diff --git a/packages/json-rpc-engine/src/v2/compatibility-utils.test.ts b/packages/json-rpc-engine/src/v2/compatibility-utils.test.ts index 9e75753daa..8e26b3ede9 100644 --- a/packages/json-rpc-engine/src/v2/compatibility-utils.test.ts +++ b/packages/json-rpc-engine/src/v2/compatibility-utils.test.ts @@ -7,7 +7,7 @@ import { makeContext, propagateToContext, propagateToRequest, - unserializeError, + deserializeError, } from './compatibility-utils'; import { MiddlewareContext } from './MiddlewareContext'; import { stringify } from './utils'; @@ -348,7 +348,7 @@ describe('compatibility-utils', () => { }); }); - describe('unserializeError', () => { + describe('deserializeError', () => { // Requires some special handling due to the possible existence or // non-existence of Error.isError describe('Error.isError', () => { @@ -382,7 +382,7 @@ describe('compatibility-utils', () => { isError.mockReturnValueOnce(true); const originalError = new Error('test error'); - const result = unserializeError(originalError); + const result = deserializeError(originalError); expect(result).toBe(originalError); }); @@ -390,14 +390,14 @@ describe('compatibility-utils', () => { isError.mockReturnValueOnce(false); const originalError = new Error('test error'); - const result = unserializeError(originalError); + const result = deserializeError(originalError); expect(result).toBe(originalError); }); }); it('creates a new Error when thrown value is a string', () => { const errorMessage = 'test error message'; - const result = unserializeError(errorMessage); + const result = deserializeError(errorMessage); expect(result).toBeInstanceOf(Error); expect(result.message).toBe(errorMessage); @@ -406,7 +406,7 @@ describe('compatibility-utils', () => { it.each([42, true, false, null, undefined, Symbol('test')])( 'creates a new Error with stringified message for non-object values', (value) => { - const result = unserializeError(value); + const result = deserializeError(value); expect(result).toBeInstanceOf(Error); expect(result.message).toBe(`Unknown error: ${stringify(value)}`); @@ -421,7 +421,7 @@ describe('compatibility-utils', () => { data: { foo: 'bar' }, }; - const result = unserializeError(thrownValue); + const result = deserializeError(thrownValue); expect(result).toBeInstanceOf(JsonRpcError); expect(result).toMatchObject({ @@ -439,7 +439,7 @@ describe('compatibility-utils', () => { data: { foo: 'bar' }, }; - const result = unserializeError(thrownValue); + const result = deserializeError(thrownValue); expect(result).toBeInstanceOf(Error); expect(result).not.toBeInstanceOf(JsonRpcError); @@ -455,7 +455,7 @@ describe('compatibility-utils', () => { message: 'test error message', }; - const result = unserializeError(thrownValue); + const result = deserializeError(thrownValue); expect(result).toBeInstanceOf(Error); expect(result).not.toBeInstanceOf(JsonRpcError); @@ -469,7 +469,7 @@ describe('compatibility-utils', () => { stack: stackTrace, }; - const result = unserializeError(thrownValue); + const result = deserializeError(thrownValue); expect(result).toBeInstanceOf(Error); expect(result.stack).toBe(stackTrace); @@ -485,7 +485,7 @@ describe('compatibility-utils', () => { data, }; - const result = unserializeError(thrownValue) as JsonRpcError; + const result = deserializeError(thrownValue) as JsonRpcError; expect(result.cause).toBe(cause); expect(result.data).toStrictEqual({ @@ -499,7 +499,7 @@ describe('compatibility-utils', () => { code: 1234, }; - const result = unserializeError(thrownValue); + const result = deserializeError(thrownValue); expect(result.message).toBe('Unknown error'); }); @@ -510,7 +510,7 @@ describe('compatibility-utils', () => { message: 42, }; - const result = unserializeError(thrownValue); + const result = deserializeError(thrownValue); expect(result.message).toBe('Unknown error'); }); @@ -521,7 +521,7 @@ describe('compatibility-utils', () => { message: 42, }; - const result = unserializeError(thrownValue); + const result = deserializeError(thrownValue); expect(result.message).toBe('Internal JSON-RPC error.'); }); diff --git a/packages/json-rpc-engine/src/v2/compatibility-utils.ts b/packages/json-rpc-engine/src/v2/compatibility-utils.ts index 03257b6a2e..cdd65cb0de 100644 --- a/packages/json-rpc-engine/src/v2/compatibility-utils.ts +++ b/packages/json-rpc-engine/src/v2/compatibility-utils.ts @@ -135,7 +135,7 @@ export function propagateToRequest( * @param thrown - The thrown value to unserialize. * @returns The unserialized error. */ -export function unserializeError(thrown: unknown): Error | JsonRpcError { +export function deserializeError(thrown: unknown): Error | JsonRpcError { // @ts-expect-error - New, but preferred if available. // See: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/isError if (typeof Error.isError === 'function' && Error.isError(thrown)) { diff --git a/packages/json-rpc-engine/src/v2/index.ts b/packages/json-rpc-engine/src/v2/index.ts index 29560da7df..ba2c932e42 100644 --- a/packages/json-rpc-engine/src/v2/index.ts +++ b/packages/json-rpc-engine/src/v2/index.ts @@ -4,13 +4,16 @@ export { createScaffoldMiddleware } from './createScaffoldMiddleware'; export { JsonRpcEngineV2 } from './JsonRpcEngineV2'; export type { JsonRpcMiddleware, + MergedContextOf, MiddlewareParams, + MiddlewareConstraint, Next, + RequestOf, ResultConstraint, } from './JsonRpcEngineV2'; export { JsonRpcServer } from './JsonRpcServer'; export { MiddlewareContext } from './MiddlewareContext'; -export type { EmptyContext } from './MiddlewareContext'; +export type { EmptyContext, ContextConstraint } from './MiddlewareContext'; export { isNotification, isRequest, JsonRpcEngineError } from './utils'; export type { Json, diff --git a/packages/network-controller/CHANGELOG.md b/packages/network-controller/CHANGELOG.md index 879bc61d25..8e2ec5d161 100644 --- a/packages/network-controller/CHANGELOG.md +++ b/packages/network-controller/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **BREAKING:** Use `InternalProvider` instead of `SafeEventEmitterProvider` ([#6796](https://github.com/MetaMask/core/pull/6796)) - Providers accessible either via network clients or global proxies no longer emit events (or inherit from EventEmitter, for that matter). +- **BREAKING:** Stop retrying `undefined` results for methods that include a block tag parameter ([#7001](https://github.com/MetaMask/core/pull/7001)) + - The network client middleware, via `@metamask/eth-json-rpc-middleware`, will now throw an error if it encounters an + `undefined` result when dispatching a request with a later block number than the originally requested block number. + - In practice, this should happen rarely if ever. - Bump `@metamask/controller-utils` from `^11.14.1` to `^11.15.0` ([#7003](https://github.com/MetaMask/core/pull/7003)) ### Fixed diff --git a/packages/network-controller/src/create-network-client.ts b/packages/network-controller/src/create-network-client.ts index 515c625816..c4cb46ec4d 100644 --- a/packages/network-controller/src/create-network-client.ts +++ b/packages/network-controller/src/create-network-client.ts @@ -12,9 +12,8 @@ import { createFetchMiddleware, createRetryOnEmptyMiddleware, } from '@metamask/eth-json-rpc-middleware'; -import type { InternalProvider } from '@metamask/eth-json-rpc-provider'; import { - providerFromEngine, + InternalProvider, providerFromMiddleware, } from '@metamask/eth-json-rpc-provider'; import { @@ -174,7 +173,7 @@ export function createNetworkClient({ engine.push(networkMiddleware); - const provider = providerFromEngine(engine); + const provider = new InternalProvider({ engine }); const destroy = () => { // TODO: Either fix this lint violation or explain why it's necessary to ignore. diff --git a/packages/network-controller/tests/network-client/block-hash-in-response.ts b/packages/network-controller/tests/network-client/block-hash-in-response.ts index 0bfae7d9a7..a3f3e379de 100644 --- a/packages/network-controller/tests/network-client/block-hash-in-response.ts +++ b/packages/network-controller/tests/network-client/block-hash-in-response.ts @@ -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; diff --git a/packages/network-controller/tests/network-client/block-param.ts b/packages/network-controller/tests/network-client/block-param.ts index 6492ab9b38..f71326c6bd 100644 --- a/packages/network-controller/tests/network-client/block-param.ts +++ b/packages/network-controller/tests/network-client/block-param.ts @@ -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, @@ -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, @@ -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 = { @@ -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, diff --git a/packages/network-controller/tests/network-client/no-block-param.ts b/packages/network-controller/tests/network-client/no-block-param.ts index 97b6cbd10e..d79f4357f0 100644 --- a/packages/network-controller/tests/network-client/no-block-param.ts +++ b/packages/network-controller/tests/network-client/no-block-param.ts @@ -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; diff --git a/tsconfig.packages.json b/tsconfig.packages.json index 8d0d5aee5e..b02edfcb6b 100644 --- a/tsconfig.packages.json +++ b/tsconfig.packages.json @@ -13,6 +13,7 @@ * `jest.config.packages.js`. */ "paths": { + "@metamask/json-rpc-engine/v2": ["../json-rpc-engine/src/v2/index.ts"], "@metamask/*": ["../*/src"] }, "strict": true, diff --git a/yarn.lock b/yarn.lock index 3046040af5..37784add1d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3612,10 +3612,10 @@ __metadata: ethers: "npm:^6.12.0" jest: "npm:^27.5.1" jest-it-up: "npm:^2.0.2" + nanoid: "npm:^3.3.8" ts-jest: "npm:^27.1.4" typedoc: "npm:^0.24.8" typescript: "npm:~5.2.2" - uuid: "npm:^8.3.2" languageName: unknown linkType: soft