From fe0eeafd9869aa27add93cfe0b8fd203fda47827 Mon Sep 17 00:00:00 2001
From: John Knox <jknox@fb.com>
Date: Mon, 11 Feb 2019 14:01:33 -0800
Subject: [PATCH] Fix issue with responses from de-inited plugins

Summary:
Now that flipper is using rsocket requestResponse, the SDK is responding with an error when the connection for this plugin has been torn down.

To avoid flipper interpreting these as bad, keep track of which plugins are inited, and only worry about responses from these.

Reviewed By: danielbuechele

Differential Revision: D13973745

fbshipit-source-id: d4e6916f89b3b562e5dcf23c4fe5b5cb384a6ec4
---
 src/Client.js | 56 +++++++++++++++++++++++++++++++++++++--------------
 src/plugin.js |  4 ++--
 2 files changed, 43 insertions(+), 17 deletions(-)

diff --git a/src/Client.js b/src/Client.js
index ffe3c6243..8a6808769 100644
--- a/src/Client.js
+++ b/src/Client.js
@@ -98,6 +98,7 @@ export default class Client extends EventEmitter {
     this.store = store;
     this.broadcastCallbacks = new Map();
     this.requestCallbacks = new Map();
+    this.activePlugins = new Set();
 
     const client = this;
     this.responder = {
@@ -140,6 +141,7 @@ export default class Client extends EventEmitter {
   connection: ?ReactiveSocket;
   responder: PartialResponder;
   store: Store;
+  activePlugins: Set<string>;
 
   broadcastCallbacks: Map<?string, Map<string, Set<Function>>>;
 
@@ -355,6 +357,8 @@ export default class Client extends EventEmitter {
         params,
       };
 
+      const plugin = params?.api;
+
       console.debug(data, 'message:call');
 
       if (this.sdkVersion < 1) {
@@ -367,21 +371,29 @@ export default class Client extends EventEmitter {
 
       const mark = this.getPerformanceMark(metadata);
       performance.mark(mark);
-      if (this.connection) {
-        this.connection
-          .requestResponse({data: JSON.stringify(data)})
-          .subscribe({
-            onComplete: payload => {
-              const logEventName = this.getLogEventName(data);
-              this.logger.trackTimeSince(mark, logEventName);
-              const response: {|
-                success?: Object,
-                error?: Object,
-              |} = JSON.parse(payload.data);
-              this.onResponse(response, resolve, reject);
-            },
-            onError: reject,
-          });
+      if (this.isAcceptingMessagesFromPlugin(plugin)) {
+        this.connection &&
+          this.connection
+            .requestResponse({data: JSON.stringify(data)})
+            .subscribe({
+              onComplete: payload => {
+                if (this.isAcceptingMessagesFromPlugin(plugin)) {
+                  const logEventName = this.getLogEventName(data);
+                  this.logger.trackTimeSince(mark, logEventName);
+                  const response: {|
+                    success?: Object,
+                    error?: Object,
+                  |} = JSON.parse(payload.data);
+                  this.onResponse(response, resolve, reject);
+                }
+              },
+              // Open fresco then layout and you get errors because responses come back after deinit.
+              onError: e => {
+                if (this.isAcceptingMessagesFromPlugin(plugin)) {
+                  reject(e);
+                }
+              },
+            });
       }
     });
   }
@@ -396,6 +408,10 @@ export default class Client extends EventEmitter {
     this.logger.trackTimeSince(mark, logEventName);
   }
 
+  isAcceptingMessagesFromPlugin(plugin: ?string) {
+    return this.connection && (!plugin || this.activePlugins.has(plugin));
+  }
+
   getPerformanceMark(data: RequestMetadata): string {
     const {method, id} = data;
     return `request_response_${method}_${id}`;
@@ -408,6 +424,16 @@ export default class Client extends EventEmitter {
       : `request_response_${method}`;
   }
 
+  initPlugin(pluginId: string) {
+    this.activePlugins.add(pluginId);
+    this.rawSend('init', {plugin: pluginId});
+  }
+
+  deinitPlugin(pluginId: string) {
+    this.activePlugins.delete(pluginId);
+    this.rawSend('deinit', {plugin: pluginId});
+  }
+
   rawSend(method: string, params?: Object): void {
     const data = {
       method,
diff --git a/src/plugin.js b/src/plugin.js
index 42ec14482..77195c5fb 100644
--- a/src/plugin.js
+++ b/src/plugin.js
@@ -209,12 +209,12 @@ export class FlipperPlugin<S = *, A = *, P = *> extends FlipperBasePlugin<
     // run plugin teardown
     this.teardown();
     if (this.realClient.connected) {
-      this.realClient.rawSend('deinit', {plugin: this.constructor.id});
+      this.realClient.deinitPlugin(this.constructor.id);
     }
   }
 
   _init() {
-    this.realClient.rawSend('init', {plugin: this.constructor.id});
+    this.realClient.initPlugin(this.constructor.id);
     this.init();
   }
 }