Skip to content

[json-rpc-engine] Rewrite JsonRpcEngine for safety, ergonomics, and debuggability #6088

@rekmarks

Description

@rekmarks

The venerable JsonRpcEngine is the backbone MetaMask's JSON-RPC stack. Being venerable, it reliably works, although it has a number of issues that we can't comprehensively fix without a full rewrite. Chief among these problems are:

  1. Middleware errors are automatically serialized
    • The engine serializes errors before they are exposed to callers of engine.handle().
    • This is because the engine is responsible both for middleware processing and the server-like responsibilities of our JSON-RPC stack, in this case ensuring that response objects are JSON-serializable.
      • This mixing of the responsibilities of middleware processing and JSON-RPC server functionality also complicates the engine implementation.
  2. Request and response objects are ambiently mutable
    • All middleware can freely mutate request and response objects at any point in time via their request or return handlers.
    • This is a Principle of Least Authority (POLA) violation, and can make debugging middleware pipelines difficult.
  3. It uses mixed callback and Promise / async signatures
    • The engine was originally implemented using the conventions of the Before Times, when async function calls looked like this:
      • asyncFn(arg, function (err, result) { ... })
    • After Promises and async / await were introduced, we slapped those capabilities on top of the original callback implementation.
    • This is confusing, and vastly complicates the implementation of the engine.
  4. It forces consumers to handle JSON-RPC notifications separately
    • JSON-RPC notifications, i.e. requests without the id property, should never be responded to.
    • Support for notifications was not included in the original implementation, and was tacked-on some years ago.
    • They complicate the engine's implementation, and are processed separately from requests, in an un-ergonomic fashion.

To address these problems, we will fully rewrite JsonRpcEngine into a new set of composable modules. The rewrite is motivated by the complexities surfaced in previous work referenced in #2024.

The goals of the rewrite include:

  • Provide an incremental migration path from the existing JsonRpcEngine to the new modules
    • It being the backbone of our JSON-RPC stack, migrating even one of our primary JSON-RPC pipelines at once is probably untenable
  • Separate "middleware processing" and "JSON-RPC server" functionality into two distinct modules
    • This will allow the middleware processor to throw native errors, and the server to serialize them
  • Handle JSON-RPC notifications "natively" (not separately from requests)
  • Use Promises and async / await, without any callbacks
  • Reduce ambient authority in middleware processing
    • Make request objects immutable
    • Conceal response objects; middleware can return a result or throw an error
    • Expose a mutable "context" object for middleware-to-middleware communication
      • We're not gonna bend over backwards to make this type safe
  • Generally, simplify the implementation and make the interface(s) more ergonomic

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions