Socket connect no longer synchronous and blocking

Summary:
Never really liked this code. Before this change, calls to connect were blocking.

Because of this, we had to make use of promises and a bit of really not that good-looking code.

So, this change makes connect non-blocking meaning that we make full use of our event handler.

These changes contain:
- CSR is not getting generated after each failed attempt.
- Connect is no longer blocking.
- Do not report events via the handler when explicitly disconnecting.

Reviewed By: jknoxville

Differential Revision: D46853228

fbshipit-source-id: 00e6a9c7c039a756175fe14982959e078d92bacb

android/src/main/cpp/sonar.cpp

@@ -276,9 +276,9 @@ class JFlipperWebSocket : public facebook::flipper::FlipperSocket {
     messageHandler_ = std::move(messageHandler);
   }
 
-  virtual bool connect(FlipperConnectionManager* manager) override {
+  virtual void connect(FlipperConnectionManager* manager) override {
     if (socket_ != nullptr) {
-      return true;
+      return;
     }
 
     std::string connectionURL = endpoint_.secure ? "wss://" : "ws://";
@@ -297,38 +297,9 @@ class JFlipperWebSocket : public facebook::flipper::FlipperSocket {
 
     auto secure = endpoint_.secure;
 
-    std::promise<bool> promise;
-    auto connected = promise.get_future();
-
-    connecting_ = true;
-
     socket_ = make_global(JFlipperSocketImpl::create(connectionURL));
     socket_->setEventHandler(JFlipperSocketEventHandlerImpl::newObjectCxxArgs(
-        [this, &promise, eventHandler = eventHandler_](SocketEvent event) {
-          /**
-             Only fulfill the promise the first time the event handler is used.
-             If the open event is received, then set the promise value to true.
-             For any other event, consider a failure and set to false.
-           */
-          if (this->connecting_) {
-            this->connecting_ = false;
-            if (event == SocketEvent::OPEN) {
-              promise.set_value(true);
-            } else if (event == SocketEvent::SSL_ERROR) {
-              try {
-                promise.set_exception(
-                    std::make_exception_ptr(folly::AsyncSocketException(
-                        folly::AsyncSocketException::SSL_ERROR,
-                        "SSL handshake failed")));
-              } catch (...) {
-                // set_exception() may throw an exception
-                // In that case, just set the value to false.
-                promise.set_value(false);
-              }
-            } else {
-              promise.set_value(false);
-            }
-          }
+        [eventHandler = eventHandler_](SocketEvent event) {
           eventHandler(event);
         },
         [messageHandler = messageHandler_](const std::string& message) {
@@ -348,8 +319,6 @@ class JFlipperWebSocket : public facebook::flipper::FlipperSocket {
           return JFlipperObject::create(std::move(object_));
         }));
     socket_->connect();
-
-    return connected.get();
   }
 
   virtual void disconnect() override {
@@ -409,7 +378,6 @@ class JFlipperWebSocket : public facebook::flipper::FlipperSocket {
   facebook::flipper::SocketMessageHandler messageHandler_;
 
   jni::global_ref<JFlipperSocketImpl> socket_;
-  bool connecting_;
 };
 
 class JFlipperSocketProvider : public facebook::flipper::FlipperSocketProvider {

android/src/main/java/com/facebook/flipper/android/FlipperSocketImpl.java

@@ -117,7 +117,7 @@ class FlipperSocketImpl extends WebSocketClient implements FlipperSocket {
 
       this.connect();
     } catch (Exception e) {
-      Log.e("Flipper", "Failed to initialize the socket before connect. " + e.getMessage());
+      Log.e("flipper", "Failed to initialize the socket before connect. Error: " + e.getMessage());
       this.mEventHandler.onConnectionEvent(FlipperSocketEventHandler.SocketEvent.ERROR);
     }
   }
@@ -139,6 +139,13 @@ class FlipperSocketImpl extends WebSocketClient implements FlipperSocket {
 
   @Override
   public void onClose(int code, String reason, boolean remote) {
+    /**
+     * If the socket is not yet open, don't report the close event. Usually, onError is invoked
+     * instead which is the one that needs reporting.
+     */
+    if (!this.isOpen()) {
+      return;
+    }
     this.mEventHandler.onConnectionEvent(FlipperSocketEventHandler.SocketEvent.CLOSE);
   }
 
@@ -162,7 +169,6 @@ class FlipperSocketImpl extends WebSocketClient implements FlipperSocket {
 
   @Override
   public void flipperDisconnect() {
-    this.mEventHandler.onConnectionEvent(FlipperSocketEventHandler.SocketEvent.CLOSE);
     this.mEventHandler =
         new FlipperSocketEventHandler() {
           @Override