Add requestResponse handler for incoming calls

Summary:
Flipper exposes a call() api to plugins which lets them call their sdk component, and it returns a promise with the response.
Currently this is done by sending a fireAndForget request, noting the id of the request, and then receiving fireAndForget requests and matching up the ids to give the result back to the right plugin promise.

Instead, it will be simpler to use rsocket requestResponse, instead of fireAndForget, which is for this exact use case. This diff adds a requestResponse handler to the SDK, so that it can deal with such requests and respond accordingly, while preserving the current functionality if it receives a fireAndForget.

So this part is backwards compatible and should be safe to land in isolation.

A later diff will change the desktop app to use requestResponse, which may not be backwards compatible, so that will have to be deployed more carefully.

Reviewed By: passy

Differential Revision: D13974049

fbshipit-source-id: b371d94a86b1f186375161ed8f2242a462ce418f

android/src/main/cpp/sonar.cpp

@@ -174,8 +174,8 @@ class JFlipperConnectionImpl : public jni::HybridClass<JFlipperConnectionImpl, J
 
   void receive(const std::string method, jni::alias_ref<JFlipperReceiver> receiver) {
     auto global = make_global(receiver);
-    _connection->receive(std::move(method), [global] (const folly::dynamic& params, std::unique_ptr<FlipperResponder> responder) {
-      global->receive(params, std::move(responder));
+    _connection->receive(std::move(method), [global] (const folly::dynamic& params, std::shared_ptr<FlipperResponder> responder) {
+      global->receive(params, responder);
     });
   }
 

iOS/FlipperKit/CppBridge/FlipperCppBridgingConnection.mm

@@ -34,10 +34,10 @@
 - (void)receive:(NSString *)method withBlock:(SonarReceiver)receiver
 {
     const auto lambda = [receiver](const folly::dynamic &message,
-                                   std::unique_ptr<facebook::flipper::FlipperResponder> responder) {
+                                   std::shared_ptr<facebook::flipper::FlipperResponder> responder) {
       @autoreleasepool {
         FlipperCppBridgingResponder *const objCResponder =
-        [[FlipperCppBridgingResponder alloc] initWithCppResponder:std::move(responder)];
+        [[FlipperCppBridgingResponder alloc] initWithCppResponder:responder];
         receiver(facebook::cxxutils::convertFollyDynamicToId(message), objCResponder);
       }
     };

iOS/FlipperKit/CppBridge/FlipperCppBridgingResponder.h

@@ -14,5 +14,5 @@ that forwards messages to the underlying C++ responder. This class allows
 pure Objective-C plugins to send messages to the underlying responder.
 */
 @interface FlipperCppBridgingResponder : NSObject <FlipperResponder>
-- (instancetype)initWithCppResponder:(std::unique_ptr<facebook::flipper::FlipperResponder>)responder;
+- (instancetype)initWithCppResponder:(std::shared_ptr<facebook::flipper::FlipperResponder>)responder;
 @end

iOS/FlipperKit/CppBridge/FlipperCppBridgingResponder.mm

@@ -10,17 +10,17 @@
 #import <FBCxxUtils/FBCxxFollyDynamicConvert.h>
 
 @implementation FlipperCppBridgingResponder {
-  std::unique_ptr<facebook::flipper::FlipperResponder> responder_;
+  std::shared_ptr<facebook::flipper::FlipperResponder> responder_;
 }
 
-- (instancetype)initWithCppResponder:(std::unique_ptr<facebook::flipper::FlipperResponder>)responder
+- (instancetype)initWithCppResponder:(std::shared_ptr<facebook::flipper::FlipperResponder>)responder
 {
   if (!responder) {
     return nil;
   }
 
   if (self = [super init]) {
-    responder_ = std::move(responder);
+    responder_ = responder;
   }
 
   return self;

iOS/FlipperKitTests/FlipperClientTests.mm

@@ -14,9 +14,11 @@
 #import <FlipperKit/FlipperClient+Testing.h>
 #import <FlipperKit/FlipperConnection.h>
 #import <FlipperKitTestUtils/BlockBasedSonarPlugin.h>
+#import <FlipperTestLib/FlipperResponderMock.h>
 #import <FlipperTestLib/FlipperConnectionManagerMock.h>
 #import <FlipperTestLib/FlipperPluginMock.h>
 #import <folly/json.h>
+#import <vector>
 
 @interface FlipperClientTests : XCTestCase
 
@@ -64,9 +66,15 @@ FlipperClient *objcClient;
     [objcClient removePlugin:cat];
 
     folly::dynamic message = folly::dynamic::object("id", 1)("method", "getPlugins");
-    socket->callbacks->onMessageReceived(message);
-    folly::dynamic expected = folly::dynamic::object("id", 1)("success", folly::dynamic::object("plugins", folly::dynamic::array()));
-    XCTAssertEqual(socket->messages.back(), expected);
+
+    std::vector<folly::dynamic> successes = std::vector<folly::dynamic>();
+    std::vector<folly::dynamic> errors = std::vector<folly::dynamic>();
+    std::unique_ptr<facebook::flipper::FlipperResponderMock> responder = std::make_unique<facebook::flipper::FlipperResponderMock>(&successes, &errors);
+    socket->callbacks->onMessageReceived(message, std::move(responder));
+    folly::dynamic expected = folly::dynamic::object("plugins", folly::dynamic::array());
+    XCTAssertEqual(successes.size(), 1);
+    XCTAssertEqual(errors.size(), 0);
+    XCTAssertEqual(successes[0], expected);
 }
 
 - (void) testPluginActivatedInBackgroundMode {
@@ -109,7 +117,9 @@ FlipperClient *objcClient;
     [objcClient start];
 
     folly::dynamic messageInit = folly::dynamic::object("method", "init")("params", folly::dynamic::object("plugin", "cat"));
-    socket->callbacks->onMessageReceived(messageInit);
+    std::unique_ptr<facebook::flipper::FlipperResponder> responder = std::make_unique<facebook::flipper::FlipperResponderMock>();
+
+    socket->callbacks->onMessageReceived(messageInit, std::move(responder));
     XCTAssertTrue(pluginConnected);
     [objcClient stop];
     XCTAssertFalse(pluginConnected);
@@ -173,9 +183,12 @@ FlipperClient *objcClient;
     [objcClient start];
 
     folly::dynamic messageInit = folly::dynamic::object("method", "init")("params", folly::dynamic::object("plugin", "PluginIdentifier"));
-    socket->callbacks->onMessageReceived(messageInit);
+    std::unique_ptr<facebook::flipper::FlipperResponder> responder1 = std::make_unique<facebook::flipper::FlipperResponderMock>();
+    socket->callbacks->onMessageReceived(messageInit, std::move(responder1));
     folly::dynamic message = folly::dynamic::object("id", 1)("method", "execute")("params", folly::dynamic::object("api", "PluginIdentifier")("method", "MethodName"));
-    socket->callbacks->onMessageReceived(message);
+    std::unique_ptr<facebook::flipper::FlipperResponder> responder2 = std::make_unique<facebook::flipper::FlipperResponderMock>();
+
+    socket->callbacks->onMessageReceived(message, std::move(responder2));
 
     XCTAssertTrue(isCalled);
 }
@@ -194,7 +207,8 @@ FlipperClient *objcClient;
     [objcClient start];
 
     folly::dynamic message = folly::dynamic::object("id", 1)("method", "execute")("params", folly::dynamic::object("api", "PluginIdentifier")("method", "MethodName"));
-    socket->callbacks->onMessageReceived(message);
+    std::unique_ptr<facebook::flipper::FlipperResponder> responder = std::make_unique<facebook::flipper::FlipperResponderMock>();
+    socket->callbacks->onMessageReceived(message, std::move(responder));
 
     XCTAssertTrue(isCalled);
 }
@@ -215,9 +229,14 @@ FlipperClient *objcClient;
     [objcClient start];
 
     folly::dynamic message = folly::dynamic::object("id", 1)("method", "execute")("params", folly::dynamic::object("api", "PluginIdentifier")("method", "MethodName"));
+    std::vector<folly::dynamic> successes = std::vector<folly::dynamic>();
+    std::vector<folly::dynamic> errors = std::vector<folly::dynamic>();
+    std::unique_ptr<facebook::flipper::FlipperResponderMock> responder = std::make_unique<facebook::flipper::FlipperResponderMock>(&successes, &errors);
 
-    XCTAssertNoThrow(socket->callbacks->onMessageReceived(message)); // This will call
+    XCTAssertNoThrow(socket->callbacks->onMessageReceived(message, std::move(responder)));
     XCTAssertTrue(isCalled);
+    XCTAssertEqual(successes.size(), 0);
+    XCTAssertEqual(errors.size(), 1);
 }
 
 @end

xplat/Flipper/FireAndForgetBasedFlipperResponder.h

@@ -0,0 +1,48 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the LICENSE
+ * file in the root directory of this source tree.
+ */
+#pragma once
+
+#include "FlipperResponder.h"
+#include "FlipperConnectionManager.h"
+#include <folly/json.h>
+
+namespace facebook {
+namespace flipper {
+
+/* Responder for responding to legacy flipper applications.
+   Originally, flipper desktop used fireAndForget for all messages, so calling
+   the SDK would send a fire and forget message, to which the SDK would respond
+   with another one, with an id field that flipper uses to map it to the
+   original request. This Responder should be used when such requests are
+   received.
+ */
+class FireAndForgetBasedFlipperResponder : public FlipperResponder {
+ public:
+  FireAndForgetBasedFlipperResponder(
+      FlipperConnectionManager* socket,
+      int64_t responseID)
+      : socket_(socket), responseID_(responseID) {}
+
+  void success(const folly::dynamic& response) const override {
+    const folly::dynamic message =
+        folly::dynamic::object("id", responseID_)("success", response);
+    socket_->sendMessage(message);
+  }
+
+  void error(const folly::dynamic& response) const override {
+    const folly::dynamic message =
+        folly::dynamic::object("id", responseID_)("error", response);
+    socket_->sendMessage(message);
+  }
+
+ private:
+  FlipperConnectionManager* socket_;
+  int64_t responseID_;
+};
+
+} // namespace flipper
+} // namespace facebook

xplat/Flipper/FlipperClient.cpp

@@ -10,6 +10,7 @@
 #include <stdexcept>
 #include <vector>
 #include "ConnectionContextStore.h"
+#include "FireAndForgetBasedFlipperResponder.h"
 #include "FlipperConnectionImpl.h"
 #include "FlipperConnectionManagerImpl.h"
 #include "FlipperResponderImpl.h"
@@ -154,18 +155,17 @@ void FlipperClient::onDisconnected() {
   });
 }
 
-void FlipperClient::onMessageReceived(const dynamic& message) {
-  performAndReportError([this, &message]() {
+void FlipperClient::onMessageReceived(
+    const dynamic& message,
+    std::unique_ptr<FlipperResponder> uniqueResponder) {
+    // Convert to shared pointer so we can hold on to it while passing it to the plugin, and still use it
+    // to respond with an error if we catch an exception.
+    std::shared_ptr<FlipperResponder> responder = std::move(uniqueResponder);
+  try {
     std::lock_guard<std::mutex> lock(mutex_);
     const auto& method = message["method"];
     const auto& params = message.getDefault("params");
 
-    std::unique_ptr<FlipperResponderImpl> responder;
-    if (message.find("id") != message.items().end()) {
-      responder.reset(
-          new FlipperResponderImpl(socket_.get(), message["id"].getInt()));
-    }
-
     if (method == "getPlugins") {
       dynamic identifiers = dynamic::array();
       for (const auto& elem : plugins_) {
@@ -179,9 +179,12 @@ void FlipperClient::onMessageReceived(const dynamic& message) {
     if (method == "init") {
       const auto identifier = params["plugin"].getString();
       if (plugins_.find(identifier) == plugins_.end()) {
-        throw std::out_of_range(
-            "plugin " + identifier + " not found for method " +
-            method.getString());
+        std::string errorMessage = "Plugin " + identifier +
+            " not found for method " + method.getString();
+        log(errorMessage);
+        responder->error(folly::dynamic::object("message", errorMessage)(
+            "name", "PluginNotFound"));
+        return;
       }
       const auto plugin = plugins_.at(identifier);
       if (!plugin.get()->runInBackground()) {
@@ -196,9 +199,12 @@ void FlipperClient::onMessageReceived(const dynamic& message) {
     if (method == "deinit") {
       const auto identifier = params["plugin"].getString();
       if (plugins_.find(identifier) == plugins_.end()) {
-        throw std::out_of_range(
-            "plugin " + identifier + " not found for method " +
-            method.getString());
+        std::string errorMessage = "Plugin " + identifier +
+            " not found for method " + method.getString();
+        log(errorMessage);
+        responder->error(folly::dynamic::object("message", errorMessage)(
+            "name", "PluginNotFound"));
+        return;
       }
       const auto plugin = plugins_.at(identifier);
       if (!plugin.get()->runInBackground()) {
@@ -210,23 +216,42 @@ void FlipperClient::onMessageReceived(const dynamic& message) {
     if (method == "execute") {
       const auto identifier = params["api"].getString();
       if (connections_.find(identifier) == connections_.end()) {
-        throw std::out_of_range(
-            "connection " + identifier + " not found for method " +
-            method.getString());
+        std::string errorMessage = "Connection " + identifier +
+            " not found for method " + method.getString();
+        log(errorMessage);
+        responder->error(folly::dynamic::object("message", errorMessage)(
+            "name", "ConnectionNotFound"));
+        return;
       }
       const auto& conn = connections_.at(params["api"].getString());
       conn->call(
           params["method"].getString(),
           params.getDefault("params"),
-          std::move(responder));
+          responder);
       return;
     }
 
     dynamic response =
         dynamic::object("message", "Received unknown method: " + method);
     responder->error(response);
-  });
+  } catch (std::exception& e) {
+    log(std::string("Error: ") + e.what());
+    if (responder) {
+      responder->error(dynamic::object("message", e.what())(
+          "stacktrace", callstack())("name", e.what()));
     }
+  } catch (...) {
+    log("Unknown error suppressed in FlipperClient");
+    if (responder) {
+      responder->error(dynamic::object(
+          "message",
+          "Unknown error during " + message["method"] + ". " +
+              folly::toJson(message))("stacktrace", callstack())(
+          "name", "Unknown"));
+    }
+  }
+}
+
 std::string FlipperClient::callstack() {
 #if __APPLE__
   // For some iOS apps, __Unwind_Backtrace symbol wasn't found in sandcastle

xplat/Flipper/FlipperClient.h

@@ -71,7 +71,9 @@ class FlipperClient : public FlipperConnectionManager::Callbacks {
 
   void onDisconnected() override;
 
-  void onMessageReceived(const folly::dynamic& message) override;
+  void onMessageReceived(
+      const folly::dynamic& message,
+      std::unique_ptr<FlipperResponder>) override;
 
   void addPlugin(std::shared_ptr<FlipperPlugin> plugin);
 

xplat/Flipper/FlipperConnection.h

@@ -23,7 +23,7 @@ with corresponding identifiers.
 class FlipperConnection {
  public:
   using FlipperReceiver = std::function<
-      void(const folly::dynamic&, std::unique_ptr<FlipperResponder>)>;
+      void(const folly::dynamic&, std::shared_ptr<FlipperResponder>)>;
 
   virtual ~FlipperConnection() {}
 

xplat/Flipper/FlipperConnectionImpl.h

@@ -1,17 +1,16 @@
-/*
- *  Copyright (c) 2018-present, Facebook, Inc.
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
  *
  * This source code is licensed under the MIT license found in the LICENSE
  * file in the root directory of this source tree.
- *
  */
-
 #pragma once
 
-#include "FlipperConnection.h"
-#include "FlipperConnectionManager.h"
 #include <map>
 #include <string>
+#include "FlipperConnection.h"
+#include "FlipperConnectionManager.h"
+#include "Log.h"
 
 namespace facebook {
 namespace flipper {
@@ -24,11 +23,14 @@ class FlipperConnectionImpl : public FlipperConnection {
   void call(
       const std::string& method,
       const folly::dynamic& params,
-      std::unique_ptr<FlipperResponder> responder) {
+      std::shared_ptr<FlipperResponder> responder) {
     if (receivers_.find(method) == receivers_.end()) {
-      throw std::out_of_range("receiver " + method + " not found.");
+      std::string errorMessage = "Receiver " + method + " not found.";
+      log("Error: " + errorMessage);
+      responder->error(folly::dynamic::object("message", errorMessage));
+      return;
     }
-    receivers_.at(method)(params, std::move(responder));
+    receivers_.at(method)(params, responder);
   }
 
   void send(const std::string& method, const folly::dynamic& params) override {

xplat/Flipper/FlipperConnectionManager.h

@@ -1,14 +1,13 @@
-/*
- *  Copyright (c) 2018-present, Facebook, Inc.
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
  *
  * This source code is licensed under the MIT license found in the LICENSE
  * file in the root directory of this source tree.
- *
  */
-
 #pragma once
 
 #include <folly/json.h>
+#include "FlipperResponder.h"
 
 namespace facebook {
 namespace flipper {
@@ -56,7 +55,9 @@ class FlipperConnectionManager::Callbacks {
 
   virtual void onDisconnected() = 0;
 
-  virtual void onMessageReceived(const folly::dynamic& message) = 0;
+  virtual void onMessageReceived(
+      const folly::dynamic& message,
+      std::unique_ptr<FlipperResponder>) = 0;
 };
 
 } // namespace flipper

xplat/Flipper/FlipperConnectionManagerImpl.cpp

@@ -16,8 +16,11 @@
 #include <stdexcept>
 #include <thread>
 #include "ConnectionContextStore.h"
+#include "FireAndForgetBasedFlipperResponder.h"
+#include "FlipperResponderImpl.h"
 #include "FlipperStep.h"
 #include "Log.h"
+#include "yarpl/Single.h"
 
 #define WRONG_THREAD_EXIT_MSG \
   "ERROR: Aborting flipper initialization because it's not running in the flipper thread."
@@ -30,6 +33,8 @@ static constexpr int maxPayloadSize = 0xFFFFFF;
 namespace facebook {
 namespace flipper {
 
+rsocket::Payload toRSocketPayload(dynamic data);
+
 class ConnectionEvents : public rsocket::RSocketConnectionEvents {
  private:
   FlipperConnectionManagerImpl* websocket_;
@@ -72,7 +77,54 @@ class Responder : public rsocket::RSocketResponder {
       rsocket::Payload request,
       rsocket::StreamId streamId) {
     const auto payload = request.moveDataToString();
-    websocket_->callbacks_->onMessageReceived(folly::parseJson(payload));
+    std::unique_ptr<FireAndForgetBasedFlipperResponder> responder;
+    auto message = folly::parseJson(payload);
+    if (message.find("id") != message.items().end()) {
+      auto id = message["id"].getInt();
+      responder =
+          std::make_unique<FireAndForgetBasedFlipperResponder>(websocket_, id);
+    }
+
+    websocket_->callbacks_->onMessageReceived(
+        folly::parseJson(payload), std::move(responder));
+  }
+
+  std::shared_ptr<yarpl::single::Single<rsocket::Payload>>
+  handleRequestResponse(rsocket::Payload request, rsocket::StreamId streamId) {
+    const auto requestString = request.moveDataToString();
+
+    auto dynamicSingle = yarpl::single::Single<folly::dynamic>::create(
+        [payload = std::move(requestString), this](auto observer) {
+          auto responder = std::make_unique<FlipperResponderImpl>(observer);
+          websocket_->callbacks_->onMessageReceived(
+              folly::parseJson(payload), std::move(responder));
+        });
+
+    auto rsocketSingle = yarpl::single::Single<rsocket::Payload>::create(
+        [payload = std::move(requestString), dynamicSingle, this](
+            auto observer) {
+          observer->onSubscribe(
+              yarpl::single::SingleSubscriptions::empty());
+          dynamicSingle->subscribe(
+              [observer, this](folly::dynamic d) {
+                websocket_->connectionEventBase_->runInEventBaseThread(
+                    [observer, d]() {
+                      try {
+                        observer->onSuccess(toRSocketPayload(d));
+
+                      } catch (std::exception& e) {
+                        log(e.what());
+                        observer->onError(e);
+                      }
+                    });
+              },
+              [observer, this](folly::exception_wrapper e) {
+                websocket_->connectionEventBase_->runInEventBaseThread(
+                    [observer, e]() { observer->onError(e); });
+              });
+        });
+
+    return rsocketSingle;
   }
 };
 
@@ -236,24 +288,18 @@ void FlipperConnectionManagerImpl::setCallbacks(Callbacks* callbacks) {
 
 void FlipperConnectionManagerImpl::sendMessage(const folly::dynamic& message) {
   flipperEventBase_->add([this, message]() {
-    std::string json = folly::toJson(message);
-    rsocket::Payload payload = rsocket::Payload(json);
-    auto payloadLength = payload.data->computeChainDataLength();
-
-    DCHECK_LE(payloadLength, maxPayloadSize);
-    if (payloadLength > maxPayloadSize) {
-      auto logMessage =
-          std::string(
-              "Error: Skipping sending message larger than max rsocket payload: ") +
-          json;
-      log(logMessage);
-      return;
-    }
+    try {
+      rsocket::Payload payload = toRSocketPayload(message);
       if (client_) {
         client_->getRequester()
             ->fireAndForget(std::move(payload))
             ->subscribe([]() {});
       }
+    } catch (std::length_error& e) {
+      // Skip sending messages that are too large.
+      log(e.what());
+      return;
+    }
   });
 }
 
@@ -341,5 +387,21 @@ bool FlipperConnectionManagerImpl::isRunningInOwnThread() {
   return flipperEventBase_->isInEventBaseThread();
 }
 
+rsocket::Payload toRSocketPayload(dynamic data) {
+  std::string json = folly::toJson(data);
+  rsocket::Payload payload = rsocket::Payload(json);
+  auto payloadLength = payload.data->computeChainDataLength();
+
+  DCHECK_LE(payloadLength, maxPayloadSize);
+  if (payloadLength > maxPayloadSize) {
+    auto logMessage =
+        std::string(
+            "Error: Skipping sending message larger than max rsocket payload: ") +
+        json;
+    throw new std::length_error(logMessage);
+  }
+  return payload;
+}
+
 } // namespace flipper
 } // namespace facebook

xplat/Flipper/FlipperResponderImpl.h

@@ -1,40 +1,42 @@
-/*
- *  Copyright (c) 2018-present, Facebook, Inc.
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
  *
  * This source code is licensed under the MIT license found in the LICENSE
  * file in the root directory of this source tree.
- *
  */
-
 #pragma once
 
-#include "FlipperResponder.h"
-#include "FlipperConnectionManager.h"
+#include <folly/io/async/EventBase.h>
 #include <folly/json.h>
+#include <rsocket/RSocketResponder.h>
+#include "FlipperConnectionManager.h"
+#include "FlipperResponder.h"
 
 namespace facebook {
 namespace flipper {
 
+/* Responder to encapsulate yarpl observables and hide them from flipper core +
+ * plugins */
 class FlipperResponderImpl : public FlipperResponder {
  public:
-  FlipperResponderImpl(FlipperConnectionManager* socket, int64_t responseID)
-      : socket_(socket), responseID_(responseID) {}
+  FlipperResponderImpl(
+      std::shared_ptr<yarpl::single::SingleObserver<folly::dynamic>>
+          downstreamObserver)
+      : downstreamObserver_(downstreamObserver) {}
 
   void success(const folly::dynamic& response) const override {
-    const folly::dynamic message =
-        folly::dynamic::object("id", responseID_)("success", response);
-    socket_->sendMessage(message);
+    const folly::dynamic message = folly::dynamic::object("success", response);
+    downstreamObserver_->onSuccess(message);
   }
 
   void error(const folly::dynamic& response) const override {
-    const folly::dynamic message =
-        folly::dynamic::object("id", responseID_)("error", response);
-    socket_->sendMessage(message);
+    const folly::dynamic message = folly::dynamic::object("error", response);
+    downstreamObserver_->onSuccess(message);
   }
 
  private:
-  FlipperConnectionManager* socket_;
-  int64_t responseID_;
+  std::shared_ptr<yarpl::single::SingleObserver<folly::dynamic>>
+      downstreamObserver_;
 };
 
 } // namespace flipper

xplat/FlipperTests/FlipperClientTests.cpp

@@ -7,6 +7,7 @@
 #include <Flipper/FlipperClient.h>
 #include <FlipperTestLib/FlipperConnectionManagerMock.h>
 #include <FlipperTestLib/FlipperPluginMock.h>
+#include <FlipperTestLib/FlipperResponderMock.h>
 
 #include <folly/json.h>
 #include <gtest/gtest.h>
@@ -17,9 +18,32 @@ namespace test {
 
 using folly::dynamic;
 
-auto state = std::make_shared<FlipperState>();
+class FlipperClientTest : public ::testing::Test {
+ protected:
+  std::unique_ptr<FlipperClient> client;
+  FlipperConnectionManagerMock* socket;
+  std::shared_ptr<FlipperState> state;
 
-TEST(FlipperClientTests, testSaneMocks) {
+  std::vector<folly::dynamic> successes;
+  std::vector<folly::dynamic> failures;
+
+  void SetUp() override {
+    successes.clear();
+    failures.clear();
+
+    state.reset(new FlipperState());
+
+    socket = new FlipperConnectionManagerMock;
+    client = std::make_unique<FlipperClient>(
+        std::unique_ptr<FlipperConnectionManagerMock>{socket}, state);
+  }
+
+  std::unique_ptr<FlipperResponderMock> getResponder() {
+    return std::make_unique<FlipperResponderMock>(&successes, &failures);
+  }
+};
+
+TEST_F(FlipperClientTest, testSaneMocks) {
   FlipperConnectionManagerMock socket;
   socket.start();
   EXPECT_TRUE(socket.isOpen());
@@ -30,82 +54,61 @@ TEST(FlipperClientTests, testSaneMocks) {
   EXPECT_EQ(plugin.identifier(), "Test");
 }
 
-TEST(FlipperClientTests, testGetPlugins) {
-  auto socket = new FlipperConnectionManagerMock;
-  FlipperClient client(
-      std::unique_ptr<FlipperConnectionManagerMock>{socket}, state);
-  client.start();
+TEST_F(FlipperClientTest, testGetPlugins) {
+  client->start();
 
-  client.addPlugin(std::make_shared<FlipperPluginMock>("Cat"));
-  client.addPlugin(std::make_shared<FlipperPluginMock>("Dog"));
+  client->addPlugin(std::make_shared<FlipperPluginMock>("Cat"));
+  client->addPlugin(std::make_shared<FlipperPluginMock>("Dog"));
 
   dynamic message = dynamic::object("id", 1)("method", "getPlugins");
-  socket->callbacks->onMessageReceived(message);
+  socket->callbacks->onMessageReceived(message, getResponder());
 
-  dynamic expected = dynamic::object("id", 1)(
-      "success", dynamic::object("plugins", dynamic::array("Cat", "Dog")));
-  EXPECT_EQ(socket->messages.back(), expected);
+  dynamic expected = dynamic::object("plugins", dynamic::array("Cat", "Dog"));
+  EXPECT_EQ(successes[0], expected);
+  EXPECT_EQ(failures.size(), 0);
 }
 
-TEST(FlipperClientTests, testGetPlugin) {
-  auto socket = new FlipperConnectionManagerMock;
-  FlipperClient client(
-      std::unique_ptr<FlipperConnectionManagerMock>{socket}, state);
-
+TEST_F(FlipperClientTest, testGetPlugin) {
   const auto catPlugin = std::make_shared<FlipperPluginMock>("Cat");
-  client.addPlugin(catPlugin);
+  client->addPlugin(catPlugin);
   const auto dogPlugin = std::make_shared<FlipperPluginMock>("Dog");
-  client.addPlugin(dogPlugin);
+  client->addPlugin(dogPlugin);
 
-  EXPECT_EQ(catPlugin, client.getPlugin("Cat"));
-  EXPECT_EQ(dogPlugin, client.getPlugin("Dog"));
+  EXPECT_EQ(catPlugin, client->getPlugin("Cat"));
+  EXPECT_EQ(dogPlugin, client->getPlugin("Dog"));
 }
 
-TEST(FlipperClientTests, testGetPluginWithDowncast) {
-  auto socket = new FlipperConnectionManagerMock;
-  FlipperClient client(
-      std::unique_ptr<FlipperConnectionManagerMock>{socket}, state);
-
+TEST_F(FlipperClientTest, testGetPluginWithDowncast) {
   const auto catPlugin = std::make_shared<FlipperPluginMock>("Cat");
-  client.addPlugin(catPlugin);
-  EXPECT_EQ(catPlugin, client.getPlugin<FlipperPluginMock>("Cat"));
+  client->addPlugin(catPlugin);
+  EXPECT_EQ(catPlugin, client->getPlugin<FlipperPluginMock>("Cat"));
 }
 
-TEST(FlipperClientTests, testRemovePlugin) {
-  auto socket = new FlipperConnectionManagerMock;
-  FlipperClient client(
-      std::unique_ptr<FlipperConnectionManagerMock>{socket}, state);
-  client.start();
+TEST_F(FlipperClientTest, testRemovePlugin) {
+  client->start();
 
   auto plugin = std::make_shared<FlipperPluginMock>("Test");
-  client.addPlugin(plugin);
-  client.removePlugin(plugin);
+  client->addPlugin(plugin);
+  client->removePlugin(plugin);
 
   dynamic message = dynamic::object("id", 1)("method", "getPlugins");
-  socket->callbacks->onMessageReceived(message);
+  auto responder = std::make_shared<FlipperResponderMock>();
+  socket->callbacks->onMessageReceived(message, getResponder());
 
-  dynamic expected = dynamic::object("id", 1)(
-      "success", dynamic::object("plugins", dynamic::array()));
-  EXPECT_EQ(socket->messages.back(), expected);
+  dynamic expected = dynamic::object("plugins", dynamic::array());
+  EXPECT_EQ(successes[0], expected);
+  EXPECT_EQ(failures.size(), 0);
 }
 
-TEST(FlipperClientTests, testStartStop) {
-  auto socket = new FlipperConnectionManagerMock;
-  FlipperClient client(
-      std::unique_ptr<FlipperConnectionManagerMock>{socket}, state);
-
-  client.start();
+TEST_F(FlipperClientTest, testStartStop) {
+  client->start();
   EXPECT_TRUE(socket->isOpen());
 
-  client.stop();
+  client->stop();
   EXPECT_FALSE(socket->isOpen());
 }
 
-TEST(FlipperClientTests, testConnectDisconnect) {
-  auto socket = new FlipperConnectionManagerMock;
-  FlipperClient client(
-      std::unique_ptr<FlipperConnectionManagerMock>{socket}, state);
-
+TEST_F(FlipperClientTest, testConnectDisconnect) {
   bool pluginConnected = false;
   const auto connectionCallback = [&](std::shared_ptr<FlipperConnection> conn) {
     pluginConnected = true;
@@ -113,23 +116,20 @@ TEST(FlipperClientTests, testConnectDisconnect) {
   const auto disconnectionCallback = [&]() { pluginConnected = false; };
   auto plugin = std::make_shared<FlipperPluginMock>(
       "Test", connectionCallback, disconnectionCallback);
-  client.addPlugin(plugin);
+  client->addPlugin(plugin);
 
-  client.start();
+  client->start();
   dynamic messageInit = dynamic::object("method", "init")(
       "params", dynamic::object("plugin", "Test"));
-  socket->callbacks->onMessageReceived(messageInit);
+  auto responder = std::make_shared<FlipperResponderMock>();
+  socket->callbacks->onMessageReceived(messageInit, getResponder());
   EXPECT_TRUE(pluginConnected);
 
-  client.stop();
+  client->stop();
   EXPECT_FALSE(pluginConnected);
 }
 
-TEST(FlipperClientTests, testInitDeinit) {
-  auto socket = new FlipperConnectionManagerMock;
-  FlipperClient client(
-      std::unique_ptr<FlipperConnectionManagerMock>{socket}, state);
-
+TEST_F(FlipperClientTest, testInitDeinit) {
   bool pluginConnected = false;
   const auto connectionCallback = [&](std::shared_ptr<FlipperConnection> conn) {
     pluginConnected = true;
@@ -138,37 +138,42 @@ TEST(FlipperClientTests, testInitDeinit) {
   auto plugin = std::make_shared<FlipperPluginMock>(
       "Test", connectionCallback, disconnectionCallback);
 
-  client.start();
-  client.addPlugin(plugin);
+  client->start();
+  client->addPlugin(plugin);
   EXPECT_FALSE(pluginConnected);
 
   dynamic expected = dynamic::object("method", "refreshPlugins");
   EXPECT_EQ(socket->messages.front(), expected);
 
+  {
     dynamic messageInit = dynamic::object("method", "init")(
         "params", dynamic::object("plugin", "Test"));
-  socket->callbacks->onMessageReceived(messageInit);
+    auto responder = std::make_shared<FlipperResponderMock>();
+    socket->callbacks->onMessageReceived(messageInit, getResponder());
     EXPECT_TRUE(pluginConnected);
+  }
 
+  {
     dynamic messageDeinit = dynamic::object("method", "deinit")(
         "params", dynamic::object("plugin", "Test"));
-  socket->callbacks->onMessageReceived(messageDeinit);
-  EXPECT_FALSE(pluginConnected);
-
-  dynamic messageReinit = dynamic::object("method", "init")(
-      "params", dynamic::object("plugin", "Test"));
-  socket->callbacks->onMessageReceived(messageReinit);
-  EXPECT_TRUE(pluginConnected);
-
-  client.stop();
+    auto responder = std::make_shared<FlipperResponderMock>();
+    socket->callbacks->onMessageReceived(messageDeinit, getResponder());
     EXPECT_FALSE(pluginConnected);
   }
 
-TEST(FlipperClientTests, testRemovePluginWhenConnected) {
-  auto socket = new FlipperConnectionManagerMock;
-  FlipperClient client(
-      std::unique_ptr<FlipperConnectionManagerMock>{socket}, state);
+  {
+    dynamic messageReinit = dynamic::object("method", "init")(
+        "params", dynamic::object("plugin", "Test"));
+    auto responder = std::make_shared<FlipperResponderMock>();
+    socket->callbacks->onMessageReceived(messageReinit, getResponder());
+    EXPECT_TRUE(pluginConnected);
+  }
 
+  client->stop();
+  EXPECT_FALSE(pluginConnected);
+}
+
+TEST_F(FlipperClientTest, testRemovePluginWhenConnected) {
   bool pluginConnected = false;
   const auto connectionCallback = [&](std::shared_ptr<FlipperConnection> conn) {
     pluginConnected = true;
@@ -177,75 +182,76 @@ TEST(FlipperClientTests, testRemovePluginWhenConnected) {
   auto plugin = std::make_shared<FlipperPluginMock>(
       "Test", connectionCallback, disconnectionCallback);
 
-  client.addPlugin(plugin);
-  client.start();
-  client.removePlugin(plugin);
+  client->addPlugin(plugin);
+  client->start();
+  client->removePlugin(plugin);
   EXPECT_FALSE(pluginConnected);
 
   dynamic expected = dynamic::object("method", "refreshPlugins");
   EXPECT_EQ(socket->messages.back(), expected);
 }
 
-TEST(FlipperClientTests, testUnhandleableMethod) {
-  auto socket = new FlipperConnectionManagerMock;
-  FlipperClient client(
-      std::unique_ptr<FlipperConnectionManagerMock>{socket}, state);
-
+TEST_F(FlipperClientTest, testUnhandleableMethod) {
   auto plugin = std::make_shared<FlipperPluginMock>("Test");
-  client.addPlugin(plugin);
+  client->addPlugin(plugin);
 
+  {
     dynamic messageInit = dynamic::object("method", "init")(
         "params", dynamic::object("plugin", "Test"));
-  socket->callbacks->onMessageReceived(messageInit);
-
-  dynamic messageExecute = dynamic::object("id", 1)("method", "unexpected");
-  socket->callbacks->onMessageReceived(messageExecute);
-
-  dynamic expected = dynamic::object("id", 1)(
-      "error",
-      dynamic::object("message", "Received unknown method: unexpected"));
-  EXPECT_EQ(socket->messages.back(), expected);
+    auto responder = std::make_shared<FlipperResponderMock>();
+    socket->callbacks->onMessageReceived(messageInit, getResponder());
   }
 
-TEST(FlipperClientTests, testExecute) {
-  auto socket = new FlipperConnectionManagerMock;
-  FlipperClient client(
-      std::unique_ptr<FlipperConnectionManagerMock>{socket}, state);
-  client.start();
+  {
+    dynamic messageExecute = dynamic::object("id", 1)("method", "unexpected");
+    auto responder = std::make_shared<FlipperResponderMock>();
+    socket->callbacks->onMessageReceived(messageExecute, getResponder());
+  }
+
+  dynamic expected =
+      dynamic::object("message", "Received unknown method: unexpected");
+  EXPECT_EQ(failures[0], expected);
+  EXPECT_EQ(successes.size(), 0);
+}
+
+TEST_F(FlipperClientTest, testExecute) {
+  client->start();
 
   const auto connectionCallback = [](std::shared_ptr<FlipperConnection> conn) {
     const auto receiver = [](const dynamic& params,
-                             std::unique_ptr<FlipperResponder> responder) {
+                             std::shared_ptr<FlipperResponder> responder) {
       dynamic payload = dynamic::object("message", "yes_i_hear_u");
       responder->success(payload);
     };
     conn->receive("plugin_can_u_hear_me", receiver);
   };
   auto plugin = std::make_shared<FlipperPluginMock>("Test", connectionCallback);
-  client.addPlugin(plugin);
+  client->addPlugin(plugin);
 
+  {
     dynamic messageInit = dynamic::object("method", "init")(
         "params", dynamic::object("plugin", "Test"));
-  socket->callbacks->onMessageReceived(messageInit);
+    auto responder = std::make_shared<FlipperResponderMock>();
+    socket->callbacks->onMessageReceived(messageInit, getResponder());
+  }
 
+  {
     dynamic messageUnexpected = dynamic::object("id", 1)("method", "execute")(
         "params",
         dynamic::object("api", "Test")("method", "plugin_can_u_hear_me"));
-  socket->callbacks->onMessageReceived(messageUnexpected);
-
-  dynamic expected = dynamic::object("id", 1)(
-      "success", dynamic::object("message", "yes_i_hear_u"));
-  EXPECT_EQ(socket->messages.back(), expected);
+    auto responder = std::make_shared<FlipperResponderMock>();
+    socket->callbacks->onMessageReceived(messageUnexpected, getResponder());
   }
 
-TEST(FlipperClientTests, testExecuteWithParams) {
-  auto socket = new FlipperConnectionManagerMock;
-  FlipperClient client(
-      std::unique_ptr<FlipperConnectionManagerMock>{socket}, state);
+  dynamic expected = dynamic::object("message", "yes_i_hear_u");
+  EXPECT_EQ(successes[0], expected);
+  EXPECT_EQ(failures.size(), 0);
+}
 
+TEST_F(FlipperClientTest, testExecuteWithParams) {
   const auto connectionCallback = [&](std::shared_ptr<FlipperConnection> conn) {
     const auto receiver = [](const dynamic& params,
-                             std::unique_ptr<FlipperResponder> responder) {
+                             std::shared_ptr<FlipperResponder> responder) {
       const auto& first = params["first"].asString();
       const auto& second = params["second"].asString();
       std::map<std::string, std::string> m{{"dog", "woof"}, {"cat", "meow"}};
@@ -255,62 +261,56 @@ TEST(FlipperClientTests, testExecuteWithParams) {
     conn->receive("animal_sounds", receiver);
   };
   auto plugin = std::make_shared<FlipperPluginMock>("Test", connectionCallback);
-  client.addPlugin(plugin);
+  client->addPlugin(plugin);
 
+  {
     dynamic messageInit = dynamic::object("method", "init")(
         "params", dynamic::object("plugin", "Test"));
-  socket->callbacks->onMessageReceived(messageInit);
+    auto responder = std::make_shared<FlipperResponderMock>();
+    socket->callbacks->onMessageReceived(messageInit, getResponder());
+  }
 
+  {
     dynamic messageExecute = dynamic::object("id", 1)("method", "execute")(
         "params",
         dynamic::object("api", "Test")("method", "animal_sounds")(
             "params", dynamic::object("first", "dog")("second", "cat")));
-  socket->callbacks->onMessageReceived(messageExecute);
-
-  dynamic expected = dynamic::object("id", 1)(
-      "success", dynamic::object("dog", "woof")("cat", "meow"));
-  EXPECT_EQ(socket->messages.back(), expected);
+    auto responder = std::make_shared<FlipperResponderMock>();
+    socket->callbacks->onMessageReceived(messageExecute, getResponder());
   }
 
-TEST(FlipperClientTests, testExceptionUnknownPlugin) {
-  auto socket = new FlipperConnectionManagerMock;
-  FlipperClient client(
-      std::unique_ptr<FlipperConnectionManagerMock>{socket}, state);
-  client.start();
+  dynamic expected = dynamic::object("dog", "woof")("cat", "meow");
+  EXPECT_EQ(successes[0], expected);
+  EXPECT_EQ(failures.size(), 0);
+}
+
+TEST_F(FlipperClientTest, testExceptionUnknownPlugin) {
+  client->start();
 
   dynamic messageInit = dynamic::object("method", "init")(
       "params", dynamic::object("plugin", "Unknown"));
-  socket->callbacks->onMessageReceived(messageInit);
-  EXPECT_EQ(
-      socket->messages.back()["error"]["message"],
-      "plugin Unknown not found for method init");
-  EXPECT_EQ(
-      socket->messages.back()["error"]["name"],
-      "plugin Unknown not found for method init");
+  auto responder = std::make_shared<FlipperResponderMock>();
+  socket->callbacks->onMessageReceived(messageInit, getResponder());
+
+  auto failure = failures[0];
+  EXPECT_EQ(failure["message"], "Plugin Unknown not found for method init");
+  EXPECT_EQ(failure["name"], "PluginNotFound");
 }
 
-TEST(FlipperClientTests, testExceptionUnknownApi) {
-  auto socket = new FlipperConnectionManagerMock;
-  FlipperClient client(
-      std::unique_ptr<FlipperConnectionManagerMock>{socket}, state);
-  client.start();
+TEST_F(FlipperClientTest, testExceptionUnknownApi) {
+  client->start();
 
   dynamic messageInit = dynamic::object("method", "execute")(
       "params", dynamic::object("api", "Unknown"));
-  socket->callbacks->onMessageReceived(messageInit);
+  auto responder = std::make_shared<FlipperResponderMock>();
+  socket->callbacks->onMessageReceived(messageInit, getResponder());
+  auto failure = failures[0];
   EXPECT_EQ(
-      socket->messages.back()["error"]["message"],
-      "connection Unknown not found for method execute");
-  EXPECT_EQ(
-      socket->messages.back()["error"]["name"],
-      "connection Unknown not found for method execute");
+      failure["message"], "Connection Unknown not found for method execute");
+  EXPECT_EQ(failure["name"], "ConnectionNotFound");
 }
 
-TEST(FlipperClientTests, testBackgroundPluginActivated) {
-  auto socket = new FlipperConnectionManagerMock;
-  FlipperClient client(
-      std::unique_ptr<FlipperConnectionManagerMock>{socket}, state);
-
+TEST_F(FlipperClientTest, testBackgroundPluginActivated) {
   bool pluginConnected = false;
   const auto connectionCallback = [&](std::shared_ptr<FlipperConnection> conn) {
     pluginConnected = true;
@@ -319,18 +319,14 @@ TEST(FlipperClientTests, testBackgroundPluginActivated) {
   auto plugin = std::make_shared<FlipperPluginMock>(
       "Test", connectionCallback, disconnectionCallback, true);
 
-  client.addPlugin(plugin);
-  client.start();
+  client->addPlugin(plugin);
+  client->start();
   EXPECT_TRUE(pluginConnected);
-  client.stop();
+  client->stop();
   EXPECT_FALSE(pluginConnected);
 }
 
-TEST(FlipperClientTests, testNonBackgroundPluginNotActivated) {
-  auto socket = new FlipperConnectionManagerMock;
-  FlipperClient client(
-      std::unique_ptr<FlipperConnectionManagerMock>{socket}, state);
-
+TEST_F(FlipperClientTest, testNonBackgroundPluginNotActivated) {
   bool pluginConnected = false;
   const auto connectionCallback = [&](std::shared_ptr<FlipperConnection> conn) {
     pluginConnected = true;
@@ -339,18 +335,14 @@ TEST(FlipperClientTests, testNonBackgroundPluginNotActivated) {
   auto plugin = std::make_shared<FlipperPluginMock>(
       "Test", connectionCallback, disconnectionCallback, false);
 
-  client.addPlugin(plugin);
-  client.start();
+  client->addPlugin(plugin);
+  client->start();
   EXPECT_FALSE(pluginConnected);
-  client.stop();
+  client->stop();
   EXPECT_FALSE(pluginConnected);
 }
 
-TEST(FlipperClientTests, testCrashInDidConnectDisConnectIsSuppressed) {
-  auto socket = new FlipperConnectionManagerMock;
-  FlipperClient client(
-      std::unique_ptr<FlipperConnectionManagerMock>{socket}, state);
-
+TEST_F(FlipperClientTest, testCrashInDidConnectDisConnectIsSuppressed) {
   const auto connectionCallback = [&](std::shared_ptr<FlipperConnection> conn) {
     throw std::runtime_error("Runtime Error in test");
   };
@@ -360,19 +352,15 @@ TEST(FlipperClientTests, testCrashInDidConnectDisConnectIsSuppressed) {
   auto plugin = std::make_shared<FlipperPluginMock>(
       "Test", connectionCallback, disconnectionCallback, true);
 
-  client.addPlugin(plugin);
+  client->addPlugin(plugin);
 
-  EXPECT_NO_FATAL_FAILURE(client.start());
-  EXPECT_NO_FATAL_FAILURE(client.stop());
+  EXPECT_NO_FATAL_FAILURE(client->start());
+  EXPECT_NO_FATAL_FAILURE(client->stop());
 }
 
-TEST(
-    FlipperClientTests,
+TEST_F(
+    FlipperClientTest,
     testNonStandardCrashInDidConnectDisConnectIsSuppressed) {
-  auto socket = new FlipperConnectionManagerMock;
-  FlipperClient client(
-      std::unique_ptr<FlipperConnectionManagerMock>{socket}, state);
-
   const auto connectionCallback = [&](std::shared_ptr<FlipperConnection> conn) {
     throw "Non standard exception";
   };
@@ -380,10 +368,10 @@ TEST(
   auto plugin = std::make_shared<FlipperPluginMock>(
       "Test", connectionCallback, disconnectionCallback, true);
 
-  client.addPlugin(plugin);
+  client->addPlugin(plugin);
 
-  EXPECT_NO_FATAL_FAILURE(client.start());
-  EXPECT_NO_FATAL_FAILURE(client.stop());
+  EXPECT_NO_FATAL_FAILURE(client->start());
+  EXPECT_NO_FATAL_FAILURE(client->stop());
 }
 
 } // namespace test

xplat/FlipperTests/FlipperResponderImplTests.cpp

@@ -0,0 +1,53 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the LICENSE
+ * file in the root directory of this source tree.
+ */
+#include <Flipper/FlipperResponderImpl.h>
+#include "yarpl/Flowable.h"
+#include "yarpl/Single.h"
+#include "yarpl/single/SingleTestObserver.h"
+
+#include <folly/json.h>
+#include <gtest/gtest.h>
+
+namespace facebook {
+namespace flipper {
+namespace test {
+
+using folly::dynamic;
+
+TEST(FlipperResponderImplTest, testSuccessWrapper) {
+  auto dynamicSingle =
+      yarpl::single::Single<folly::dynamic>::create([](auto observer) mutable {
+        observer->onSubscribe(yarpl::single::SingleSubscriptions::empty());
+        auto responder = std::make_shared<FlipperResponderImpl>(observer);
+        responder->success(folly::dynamic::object("my", "object"));
+      });
+  auto to = yarpl::single::SingleTestObserver<folly::dynamic>::create();
+  dynamicSingle->subscribe(to);
+
+  to->awaitTerminalEvent();
+  auto output = to->getOnSuccessValue();
+  EXPECT_EQ(output["success"]["my"], "object");
+}
+
+TEST(FlipperResponderImplTest, testErrorWrapper) {
+  auto dynamicSingle =
+      yarpl::single::Single<folly::dynamic>::create([](auto observer) mutable {
+        observer->onSubscribe(yarpl::single::SingleSubscriptions::empty());
+        auto responder = std::make_shared<FlipperResponderImpl>(observer);
+        responder->error(folly::dynamic::object("my", "object"));
+      });
+  auto to = yarpl::single::SingleTestObserver<folly::dynamic>::create();
+  dynamicSingle->subscribe(to);
+
+  to->awaitTerminalEvent();
+  auto output = to->getOnSuccessValue();
+  EXPECT_EQ(output["error"]["my"], "object");
+}
+
+} // namespace test
+} // namespace flipper
+} // namespace facebook