Handle JSON-RPC notifications

[rekmarks]

This PR adds JSON-RPC 2.0-compliant notification handling for JsonRpcEngine.

• JSON-RPC notifications are defined as JSON-RPC request objects without an id property.
• A new constructor parameter, notificationHandler, is introduced. This parameter is a function that accepts JSON-RPC notification objects and returns void | Promise<void>.
• When JsonRpcEngine.handle is called, if a notificationHandler exists, any request objects duck-typed as notifications will be handled as such. This means that:
  • Validation errors that occur after duck-typing will be ignored. At the moment, this just means that no error will be thrown if the method field is not a string.
  • If basic validation succeeds, the notification object will be passed to the handler function without touching the middleware stack.
  • The response from handle() will be undefined.
  • No error will be returned or thrown, unless the notification handler itself throws or rejects.
    • Notification handlers should not throw or reject, and it is the implementer's responsibility to ensure that they do not.
• If JsonRpcEngine.handle is called and no notificationHandler exists, notifications will be treated just like requests. This is the current behavior.

[mcmire]

Did a quick pass over this. I'll try to look at this more closely tomorrow.

[shanejonas]

I think we should note that this is for "client side" sending notifications to the server. I believe "server side" notifications end up being an event emitter which isnt in json-rpc-engine but json-rpc-middleware-stream: https://github.com/MetaMask/json-rpc-middleware-stream/blob/main/src/createStreamMiddleware.ts#L71

[mcmire]

JsonRpcEngine sure is getting complicated isn't it 😅 Still, these seem like reasonable updates. LGTM.

[mcmire]

This seems like it could cause all of kinds of runtime bugs. Glad we're using TypeScript 😬

[mcmire]

I am not 100% sure on this, but my guess is that it's because there is no correlation between this._notificationHandler and the type of callerReq. If there is a way to tell TypeScript that when this_.notificationHandler is given then all requests must be notifications then this could work, but I'm not sure quite how to do that. This seems sufficient. I appreciate the comment!

[rekmarks]

@shanejonas not when I'm done with it 😎
MetaMask/json-rpc-middleware-stream#24