Fix rpc and tests (#15570)

app/scripts/lib/metaRPCClientFactory.js

@@ -3,6 +3,8 @@ import SafeEventEmitter from 'safe-event-emitter';
 import createRandomId from '../../../shared/modules/random-id';
 import { TEN_SECONDS_IN_MILLISECONDS } from '../../../ui/helpers/constants/critical-error';
 
+class DisconnectError extends Error {}
+
 class MetaRPCClient {
   constructor(connectionStream) {
     this.connectionStream = connectionStream;
@@ -12,6 +14,7 @@ class MetaRPCClient {
     this.connectionStream.on('data', this.handleResponse.bind(this));
     this.connectionStream.on('end', this.close.bind(this));
     this.responseHandled = {};
+    this.DisconnectError = DisconnectError;
   }
 
   send(id, payload, cb) {
@@ -47,6 +50,13 @@ class MetaRPCClient {
   close() {
     this.notificationChannel.removeAllListeners();
     this.uncaughtErrorChannel.removeAllListeners();
+    // fail all unfinished requests
+    for (const [id, handler] of this.requests) {
+      if (!this.responseHandled[id]) {
+        this.responseHandled[id] = true;
+        handler(new DisconnectError('disconnected'));
+      }
+    }
   }
 
   handleResponse(data) {

app/scripts/lib/metaRPCClientFactory.test.js

@@ -1,3 +1,4 @@
+/* eslint-disable jest/no-done-callback */
 import { obj as createThoughStream } from 'through2';
 import metaRPCClientFactory from './metaRPCClientFactory';
 
@@ -9,13 +10,14 @@ describe('metaRPCClientFactory', () => {
     const metaRPCClient = metaRPCClientFactory(streamTest);
     metaRPCClient.foo();
   });
-  it('should be able to make an rpc request/response with the method and params and node-style callback', () => {
+  it('should be able to make an rpc request/response with the method and params and node-style callback', (done) => {
     const streamTest = createThoughStream();
     const metaRPCClient = metaRPCClientFactory(streamTest);
 
     // make a "foo" method call
     metaRPCClient.foo('bar', (_, result) => {
       expect(result).toStrictEqual('foobarbaz');
+      done();
     });
 
     // fake a response
@@ -27,7 +29,7 @@ describe('metaRPCClientFactory', () => {
       });
     });
   });
-  it('should be able to make an rpc request/error with the method and params and node-style callback', () => {
+  it('should be able to make an rpc request/error with the method and params and node-style callback', (done) => {
     const streamTest = createThoughStream();
     const metaRPCClient = metaRPCClientFactory(streamTest);
 
@@ -35,6 +37,7 @@ describe('metaRPCClientFactory', () => {
     metaRPCClient.foo('bar', (err) => {
       expect(err.message).toStrictEqual('foo-message');
       expect(err.code).toStrictEqual(1);
+      done();
     });
 
     metaRPCClient.requests.forEach((_, key) => {
@@ -49,7 +52,7 @@ describe('metaRPCClientFactory', () => {
     });
   });
 
-  it('should be able to make an rpc request/response with the method and params and node-style callback with multiple instances of metaRPCClientFactory and the same connectionStream', () => {
+  it('should be able to make an rpc request/response with the method and params and node-style callback with multiple instances of metaRPCClientFactory and the same connectionStream', (done) => {
     const streamTest = createThoughStream();
     const metaRPCClient = metaRPCClientFactory(streamTest);
     const metaRPCClient2 = metaRPCClientFactory(streamTest);
@@ -59,6 +62,7 @@ describe('metaRPCClientFactory', () => {
       expect(result).toStrictEqual('foobarbaz');
       metaRPCClient2.baz('bar', (err) => {
         expect(err).toBeNull();
+        done();
       });
     });
 
@@ -81,12 +85,13 @@ describe('metaRPCClientFactory', () => {
     });
   });
 
-  it('should be able to handle notifications', () => {
+  it('should be able to handle notifications', (done) => {
     const streamTest = createThoughStream();
     const metaRPCClient = metaRPCClientFactory(streamTest);
 
     metaRPCClient.onNotification((notification) => {
       expect(notification.method).toStrictEqual('foobarbaz');
+      done();
     });
 
     // send a notification
@@ -97,12 +102,13 @@ describe('metaRPCClientFactory', () => {
     });
   });
 
-  it('should be able to handle errors with no id', () => {
+  it('should be able to handle errors with no id', (done) => {
     const streamTest = createThoughStream();
     const metaRPCClient = metaRPCClientFactory(streamTest);
 
     metaRPCClient.onUncaughtError((error) => {
       expect(error.code).toStrictEqual(1);
+      done();
     });
 
     streamTest.write({
@@ -114,12 +120,13 @@ describe('metaRPCClientFactory', () => {
     });
   });
 
-  it('should be able to handle errors with null id', () => {
+  it('should be able to handle errors with null id', (done) => {
     const streamTest = createThoughStream();
     const metaRPCClient = metaRPCClientFactory(streamTest);
 
     metaRPCClient.onUncaughtError((error) => {
       expect(error.code).toStrictEqual(1);
+      done();
     });
 
     streamTest.write({
@@ -132,7 +139,7 @@ describe('metaRPCClientFactory', () => {
     });
   });
 
-  it('should be able to handle no message within TIMEOUT secs', async () => {
+  it('should be able to handle no message within TIMEOUT secs for getState', async () => {
     jest.useFakeTimers();
     const streamTest = createThoughStream();
     const metaRPCClient = metaRPCClientFactory(streamTest);
@@ -148,4 +155,17 @@ describe('metaRPCClientFactory', () => {
 
     jest.useRealTimers();
   });
+
+  it('should fail all pending actions with a DisconnectError when the stream ends', (done) => {
+    const streamTest = createThoughStream();
+    const metaRPCClient = metaRPCClientFactory(streamTest);
+
+    metaRPCClient.foo('bar', (err) => {
+      expect(err).toBeInstanceOf(metaRPCClient.DisconnectError);
+      expect(err.message).toStrictEqual('disconnected');
+      done();
+    });
+
+    streamTest.emit('end');
+  });
 });

app/scripts/ui.js

@@ -197,7 +197,8 @@ async function displayCriticalError(err, metamaskState) {
  */
 function connectToAccountManager(connectionStream, cb) {
   const mx = setupMultiplex(connectionStream);
-  setupControllerConnection(mx.createStream('controller'), cb);
+  const controllerConnectionStream = mx.createStream('controller');
+  setupControllerConnection(controllerConnectionStream, cb);
   setupWeb3Connection(mx.createStream('provider'));
 }
 
@@ -219,10 +220,10 @@ function setupWeb3Connection(connectionStream) {
 /**
  * Establishes a streamed connection to the background account manager
  *
- * @param {PortDuplexStream} connectionStream - PortStream instance establishing a background connection
+ * @param {PortDuplexStream} controllerConnectionStream - PortStream instance establishing a background connection
  * @param {Function} cb - Called when the remote account manager connection is established
  */
-function setupControllerConnection(connectionStream, cb) {
+function setupControllerConnection(controllerConnectionStream, cb) {
-  const backgroundRPC = metaRPCClientFactory(connectionStream);
+  const backgroundRPC = metaRPCClientFactory(controllerConnectionStream);
   cb(null, backgroundRPC);
 }