Flipper | Stop exceptions being thrown for indicating that no Flipper desktop client is available

Summary:
Originally, Flipper would use exceptions for the control flow of determining if there's no desktop client available. It would catch an exception and then kick off another attempt. This is cool, but if you are debugging an application where Flipper is enabled, and you set a breakpoint to detect all thrown exceptions, then you'll see an exception thrown inside Flipper logic every couple of seconds. That makes it hard to debug what you're trying to do.

There's a workaround where you can simply open Flipper, but that's a bit annoying.

This diff changes the logic in Flipper to not use exceptions for the part of connection where it's doing the initial connect. We could apply this to `doCertificateExchange` as well, but that's actually an error there so I don't see the need to do it there as well.

Reviewed By: jknoxville

Differential Revision: D21016737

fbshipit-source-id: cbdfe2794b4d38a9e3b8304ebee845655bb26ae5

xplat/Flipper/FlipperConnectionManagerImpl.cpp

@@ -129,32 +129,30 @@ void FlipperConnectionManagerImpl::startSync() {
     if (isClientSetupStep) {
       doCertificateExchange();
     } else {
-      connectSecurely();
+      if (!connectSecurely()) {
+        // The expected code path when flipper desktop is not running.
+        // Don't count as a failed attempt, or it would invalidate the
+        // connection files for no reason. On iOS devices, we can always connect
+        // to the local port forwarding server even when it can't connect to
+        // flipper. In that case we get a Network error instead of a Port not
+        // open error, so we treat them the same.
+        step->fail(
+            "No route to flipper found. Is flipper desktop running? Retrying...");
+        reconnect();
+      }
     }
     step->complete();
   } catch (const folly::AsyncSocketException& e) {
-    if (e.getType() == folly::AsyncSocketException::NOT_OPEN ||
+    if (e.getType() == folly::AsyncSocketException::SSL_ERROR) {
-        e.getType() == folly::AsyncSocketException::NETWORK_ERROR) {
+      auto message = std::string(e.what()) +
-      // The expected code path when flipper desktop is not running.
+          "\nMake sure the date and time of your device is up to date.";
-      // Don't count as a failed attempt, or it would invalidate the connection
+      log(message);
-      // files for no reason. On iOS devices, we can always connect to the local
+      step->fail(message);
-      // port forwarding server even when it can't connect to flipper. In that
-      // case we get a Network error instead of a Port not open error, so we
-      // treat them the same.
-      step->fail(
-          "No route to flipper found. Is flipper desktop running? Retrying...");
     } else {
-      if (e.getType() == folly::AsyncSocketException::SSL_ERROR) {
+      log(e.what());
-        auto message = std::string(e.what()) +
+      step->fail(e.what());
-            "\nMake sure the date and time of your device is up to date.";
-        log(message);
-        step->fail(message);
-      } else {
-        log(e.what());
-        step->fail(e.what());
-      }
-      failedConnectionAttempts_++;
     }
+    failedConnectionAttempts_++;
     reconnect();
   } catch (const std::exception& e) {
     log(e.what());
@@ -194,7 +192,7 @@ void FlipperConnectionManagerImpl::doCertificateExchange() {
   requestSignedCertFromFlipper();
 }
 
-void FlipperConnectionManagerImpl::connectSecurely() {
+bool FlipperConnectionManagerImpl::connectSecurely() {
   rsocket::SetupParameters parameters;
   folly::SocketAddress address;
 
@@ -216,7 +214,8 @@ void FlipperConnectionManagerImpl::connectSecurely() {
       contextStore_->getSSLContext();
   auto connectingSecurely = flipperState_->start("Connect securely");
   connectionIsTrusted_ = true;
-  client_ =
+
+  auto newClient =
       rsocket::RSocket::createConnectedClient(
           std::make_unique<rsocket::TcpConnectionFactory>(
               *connectionEventBase_->getEventBase(),
@@ -227,9 +226,24 @@ void FlipperConnectionManagerImpl::connectSecurely() {
           std::chrono::seconds(connectionKeepaliveSeconds), // keepaliveInterval
           nullptr, // stats
           std::make_shared<ConnectionEvents>(this))
+          .thenError<folly::AsyncSocketException>([](const auto& e) {
+            if (e.getType() == folly::AsyncSocketException::NOT_OPEN ||
+                e.getType() == folly::AsyncSocketException::NETWORK_ERROR) {
+              // This is the state where no Flipper desktop client is connected.
+              // We don't want an exception thrown here.
+              return std::unique_ptr<rsocket::RSocketClient>(nullptr);
+            }
+            throw e;
+          })
           .get();
+  if (newClient.get() == nullptr) {
+    return false;
+  }
+
+  client_ = std::move(newClient);
   connectingSecurely->complete();
   failedConnectionAttempts_ = 0;
+  return true;
 }
 
 void FlipperConnectionManagerImpl::reconnect() {

xplat/Flipper/FlipperConnectionManagerImpl.h

@@ -69,7 +69,7 @@ class FlipperConnectionManagerImpl : public FlipperConnectionManager {
 
   void startSync();
   void doCertificateExchange();
-  void connectSecurely();
+  bool connectSecurely();
   bool isCertificateExchangeNeeded();
   void requestSignedCertFromFlipper();
   bool isRunningInOwnThread();