FlipperResponder: no response = success

Summary:
If an exception is thrown from the plugin, FlipperClient.cpp will catch it and respond with an error response.
If the object goes out of scope with no response being returned, then return a success response in the destructor.

Reviewed By: passy

Differential Revision: D14024259

fbshipit-source-id: 52e419dd23fc3882e8b92b593e8c1e1ea90e2b26

xplat/Flipper/FireAndForgetBasedFlipperResponder.h

@@ -27,13 +27,13 @@ class FireAndForgetBasedFlipperResponder : public FlipperResponder {
       int64_t responseID)
       : socket_(socket), responseID_(responseID) {}
 
-  void success(const folly::dynamic& response) const override {
+  void success(const folly::dynamic& response) override {
     const folly::dynamic message =
         folly::dynamic::object("id", responseID_)("success", response);
     socket_->sendMessage(message);
   }
 
-  void error(const folly::dynamic& response) const override {
+  void error(const folly::dynamic& response) override {
     const folly::dynamic message =
         folly::dynamic::object("id", responseID_)("error", response);
     socket_->sendMessage(message);

xplat/Flipper/FlipperResponder.h

@@ -1,11 +1,9 @@
-/*
+/**
- *  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>
@@ -24,12 +22,12 @@ class FlipperResponder {
   /**
    * Deliver a successful response to the Flipper desktop app.
    */
-  virtual void success(const folly::dynamic& response) const = 0;
+  virtual void success(const folly::dynamic& response) = 0;
 
   /**
    * Inform the Flipper desktop app of an error in handling the request.
    */
-  virtual void error(const folly::dynamic& response) const = 0;
+  virtual void error(const folly::dynamic& response) = 0;
 };
 
 } // namespace flipper

xplat/Flipper/FlipperResponderImpl.h

@@ -24,19 +24,29 @@ class FlipperResponderImpl : public FlipperResponder {
           downstreamObserver)
       : downstreamObserver_(downstreamObserver) {}
 
-  void success(const folly::dynamic& response) const override {
+  void success(const folly::dynamic& response) override {
     const folly::dynamic message = folly::dynamic::object("success", response);
+    isCompleted = true;
     downstreamObserver_->onSuccess(message);
   }
 
-  void error(const folly::dynamic& response) const override {
+  void error(const folly::dynamic& response) override {
     const folly::dynamic message = folly::dynamic::object("error", response);
+    isCompleted = true;
     downstreamObserver_->onSuccess(message);
   }
 
+  ~FlipperResponderImpl() {
+    if (!isCompleted) {
+      downstreamObserver_->onSuccess(
+          folly::dynamic::object("success", folly::dynamic::object()));
+    }
+  }
+
  private:
   std::shared_ptr<yarpl::single::SingleObserver<folly::dynamic>>
       downstreamObserver_;
+  bool isCompleted = false;
 };
 
 } // namespace flipper

xplat/FlipperTestLib/FlipperResponderMock.h

@@ -1,11 +1,9 @@
-/*
+/**
- *  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 <Flipper/FlipperResponder.h>
@@ -22,13 +20,13 @@ class FlipperResponderMock : public FlipperResponder {
       std::vector<folly::dynamic>* errors = nullptr)
       : successes_(successes), errors_(errors) {}
 
-  void success(const folly::dynamic& response) const override {
+  void success(const folly::dynamic& response) override {
     if (successes_) {
       successes_->push_back(response);
     }
   }
 
-  void error(const folly::dynamic& response) const override {
+  void error(const folly::dynamic& response) override {
     if (errors_) {
       errors_->push_back(response);
     }

xplat/FlipperTests/FlipperResponderImplTests.cpp

@@ -18,6 +18,9 @@ namespace test {
 
 using folly::dynamic;
 
+void assertIsSuccess(folly::dynamic d);
+void assertIsError(folly::dynamic d);
+
 TEST(FlipperResponderImplTest, testSuccessWrapper) {
   auto dynamicSingle =
       yarpl::single::Single<folly::dynamic>::create([](auto observer) mutable {
@@ -30,6 +33,7 @@ TEST(FlipperResponderImplTest, testSuccessWrapper) {
 
   to->awaitTerminalEvent();
   auto output = to->getOnSuccessValue();
+  assertIsSuccess(output);
   EXPECT_EQ(output["success"]["my"], "object");
 }
 
@@ -45,9 +49,37 @@ TEST(FlipperResponderImplTest, testErrorWrapper) {
 
   to->awaitTerminalEvent();
   auto output = to->getOnSuccessValue();
+  assertIsError(output);
   EXPECT_EQ(output["error"]["my"], "object");
 }
 
+TEST(FlipperResponderImplTest, testNoExplicitResponseReturnsSuccess) {
+  auto to = yarpl::single::SingleTestObserver<folly::dynamic>::create();
+  {
+    auto dynamicSingle = yarpl::single::Single<folly::dynamic>::create(
+        [](auto observer) mutable {
+          observer->onSubscribe(yarpl::single::SingleSubscriptions::empty());
+          auto responder = std::make_shared<FlipperResponderImpl>(observer);
+        });
+    dynamicSingle->subscribe(to);
+  }
+
+  to->awaitTerminalEvent();
+  auto output = to->getOnSuccessValue();
+  assertIsSuccess(output);
+  EXPECT_TRUE(output["success"].empty());
+}
+
+void assertIsSuccess(folly::dynamic d) {
+  EXPECT_NE(d.find("success"), d.items().end());
+  EXPECT_EQ(d.find("error"), d.items().end());
+}
+
+void assertIsError(folly::dynamic d) {
+  EXPECT_NE(d.find("error"), d.items().end());
+  EXPECT_EQ(d.find("success"), d.items().end());
+}
+
 } // namespace test
 } // namespace flipper
 } // namespace facebook