From 07292f837d4f95b60c86efdbb6eb8103dbd5b44a Mon Sep 17 00:00:00 2001
From: Lorenzo Blasa <lblasa@meta.com>
Date: Mon, 17 Jul 2023 06:54:35 -0700
Subject: [PATCH] Clear event handler after error or close

Summary:
Previous approach was a bit flawed as the `isOpen()` API returns true by the time close is called.

This was OK in the case of errors, as the open flag was set to false hence preventing us to report a close after the error. It is not OK on healthy disconnect situations as these events were not getting reported.

In this case, a better solution is just to clear the event handler after a close or error because in either case we are no longer interested in dispatching any other events to the handler.

Reviewed By: antonk52

Differential Revision: D47510883

fbshipit-source-id: 883a3f87f24f71fe44a624590a310fe2563cbd8a
---
 .../flipper/android/FlipperSocketImpl.java    | 44 ++++++++++---------
 1 file changed, 24 insertions(+), 20 deletions(-)

diff --git a/android/src/main/java/com/facebook/flipper/android/FlipperSocketImpl.java b/android/src/main/java/com/facebook/flipper/android/FlipperSocketImpl.java
index 464dc8222..7479889e5 100644
--- a/android/src/main/java/com/facebook/flipper/android/FlipperSocketImpl.java
+++ b/android/src/main/java/com/facebook/flipper/android/FlipperSocketImpl.java
@@ -67,6 +67,22 @@ class FlipperSocketImpl extends WebSocketClient implements FlipperSocket {
     this.mEventHandler = eventHandler;
   }
 
+  private void clearEventHandler() {
+    this.mEventHandler =
+        new FlipperSocketEventHandler() {
+          @Override
+          public void onConnectionEvent(SocketEvent event) {}
+
+          @Override
+          public void onMessageReceived(String message) {}
+
+          @Override
+          public FlipperObject onAuthenticationChallengeReceived() {
+            return null;
+          }
+        };
+  }
+
   @Override
   public void flipperConnect() {
     if ((this.isOpen())) {
@@ -139,14 +155,10 @@ 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);
+    // Clear the existing event handler as to ensure no other events are processed after the close
+    // is handled.
+    this.clearEventHandler();
   }
 
   /**
@@ -158,6 +170,7 @@ class FlipperSocketImpl extends WebSocketClient implements FlipperSocket {
    */
   @Override
   public void onError(Exception ex) {
+
     // Check the exception for OpenSSL error and change the event type.
     // Required for Flipper as the current implementation treats these errors differently.
     if (ex instanceof javax.net.ssl.SSLHandshakeException) {
@@ -165,23 +178,14 @@ class FlipperSocketImpl extends WebSocketClient implements FlipperSocket {
     } else {
       this.mEventHandler.onConnectionEvent(FlipperSocketEventHandler.SocketEvent.ERROR);
     }
+    // Clear the existing event handler as to ensure no other events are processed after the close
+    // is handled.
+    this.clearEventHandler();
   }
 
   @Override
   public void flipperDisconnect() {
-    this.mEventHandler =
-        new FlipperSocketEventHandler() {
-          @Override
-          public void onConnectionEvent(SocketEvent event) {}
-
-          @Override
-          public void onMessageReceived(String message) {}
-
-          @Override
-          public FlipperObject onAuthenticationChallengeReceived() {
-            return null;
-          }
-        };
+    this.clearEventHandler();
     super.close();
   }