-
-
Notifications
You must be signed in to change notification settings - Fork 241
Open
Labels
Description
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:
- 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.
- The engine serializes errors before they are exposed to callers of
- 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.
- 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.
- The engine was originally implemented using the conventions of the Before Times, when async function calls looked like this:
- 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.
- JSON-RPC notifications, i.e. requests without the
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