Skip to content

Conversation

@rekmarks
Copy link
Member

Explanation

References

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed, highlighting breaking changes as necessary
  • I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes

Base automatically changed from rekm/ejrpcm-tests to main October 29, 2025 21:41
@rekmarks rekmarks force-pushed the rekm/ejrpcm-rewrite branch 2 times, most recently from 9183a6e to db9e23c Compare October 31, 2025 21:00
@rekmarks rekmarks changed the base branch from main to rekm/ejrpcp-v2 October 31, 2025 21:01
@rekmarks rekmarks force-pushed the rekm/ejrpcm-rewrite branch 3 times, most recently from 6f67888 to 9708208 Compare October 31, 2025 22:36
@rekmarks rekmarks force-pushed the rekm/ejrpcp-v2 branch 2 times, most recently from fbc467a to c0feb25 Compare November 4, 2025 17:28
@rekmarks rekmarks force-pushed the rekm/ejrpcm-rewrite branch 3 times, most recently from 279b796 to 61eb128 Compare November 4, 2025 21:25
@rekmarks rekmarks force-pushed the rekm/ejrpcm-rewrite branch from 61eb128 to cf32da1 Compare November 5, 2025 06:31
Base automatically changed from rekm/ejrpcp-v2 to main November 5, 2025 19:18
rekmarks added a commit that referenced this pull request Nov 5, 2025
## Explanation

Per #6327, migrates `@metamask/eth-json-rpc-provider` to
`JsonRpcEngineV2` following its recent introduction. Intended to be
closely followed by #6976 and subsequently released.

The `InternalProvider` is updated to use `JsonRpcServer` under the hood.
It can be constructed with a `JsonRpcServer` or legacy engine, but the
latter will be wrapped by a `JsonRpcServer` in order to ensure
consistent behavior across all internal providers. Meanwhile,
`providerFromEngine()` is removed, it being a useless wrapper over the
`InternalProvider` constructor.

Elsewhere in the monorepo, these changes revealed a discrepancy in
behavior between the error handling of the legacy engine and
`asV2Middleware()`, which the latter is amended to resolve.

In addition, the changes to the `InternalProvider` revealed that the
legacy engine tolerates responses with `{ result: undefined }`. This is
impossible to express using the V2 engine, which caused some
`NetworkController` tests reliant on the legacy behavior to fail.
Further investigation proved that these `undefined` results would error
elsewhere in our JSON-RPC pipelines, so we simply remove these test
cases. The upshot is that we no longer retry `undefined` results for
"child requests" in the `retryOnEmpty` middleware. It is unclear if this
was occurring in practice, and it ought to be treated as a breach of
contract by the RPC endpoint if it does.

## References

- Progresses: #6327

## Checklist

- [x] I've updated the test suite for new or updated code as appropriate
- [x] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [x] I've communicated my changes to consumers by [updating changelogs
for packages I've
changed](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs),
highlighting breaking changes as necessary
- [x] I've prepared draft pull requests for clients and consumer
packages to resolve any breaking changes
- Not intended to be shipped without #6976, but it appears we could if
we wanted to:
    - MetaMask/metamask-extension#37521
    - MetaMask/metamask-mobile#22154

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> Migrates provider to JsonRpcEngineV2 with updated InternalProvider and
helpers, adjusts error/empty-result handling, and updates dependent
packages/tests.
> 
> - **eth-json-rpc-provider**:
> - Migrate to `JsonRpcEngineV2`; `InternalProvider` now wraps legacy
engines via `asV2Middleware`/`JsonRpcEngineV2` and returns results
directly.
> - Add `providerFromMiddlewareV2`; deprecate `providerFromMiddleware`;
remove `providerFromEngine`.
> - Switch ID generation from `uuid` to `nanoid`; update exports/tests
accordingly.
> - **json-rpc-engine (v2)**:
> - Add/expand V2 types and exports (`MiddlewareConstraint`,
`MergedContextOf`, etc.).
> - Update `asV2Middleware` to deserialize errors and ignore `{ error:
undefined }` while forwarding results; add tests.
> - Rename `unserializeError` to `deserializeError` and adjust
usage/tests.
> - **eth-json-rpc-middleware**:
> - Change `retryOnEmpty` to treat only `null` and `"<nil>"` as empty
(stop retrying `undefined`).
> - **network-controller**:
> - Replace `providerFromEngine` usage with `new InternalProvider({
engine })`.
> - Align tests with new empty-result behavior and provider
construction.
> - **eth-block-tracker tests**:
> - Construct providers via `InternalProvider` instead of
`providerFromEngine`.
> - **Tooling/Config**:
>   - Add TS/Jest path mapping for `@metamask/json-rpc-engine/v2`.
> 
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
4a63eea. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
@rekmarks
Copy link
Member Author

rekmarks commented Nov 5, 2025

Going to reopen this on top of #7061.

@rekmarks rekmarks closed this Nov 5, 2025
@rekmarks rekmarks deleted the rekm/ejrpcm-rewrite branch November 5, 2025 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants