Improve Android packager reconnect messaging

Author: ya-nsh

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerBase.kt

@@ -198,6 +198,8 @@ public abstract class DevSupportManagerBase(
   private var isShakeDetectorStarted = false
   private var isDevSupportEnabled = false
   private var isPackagerConnected = false
+  private val packagerConnectionStatusNotifier =
+      PackagerConnectionStatusNotifier(devLoadingViewManager)
   private val errorCustomizers: MutableList<ErrorCustomizer> = mutableListOf()
   private var packagerLocationCustomizer: PackagerLocationCustomizer? = null
   private val jSExecutorDescription: String?
@@ -966,12 +968,14 @@ public abstract class DevSupportManagerBase(
           javaClass.simpleName,
           object : PackagerCommandListener {
             override fun onPackagerConnected() {
+              packagerConnectionStatusNotifier.onPackagerConnected()
               isPackagerConnected = true
               perfMonitorOverlayManager?.enable()
               perfMonitorOverlayManager?.startBackgroundTrace()
             }
 
             override fun onPackagerDisconnected() {
+              packagerConnectionStatusNotifier.onPackagerDisconnected()
               isPackagerConnected = false
               perfMonitorOverlayManager?.disable()
               perfMonitorOverlayManager?.stopBackgroundTrace()

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/PackagerConnectionStatusNotifier.kt

@@ -0,0 +1,37 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+package com.facebook.react.devsupport
+
+import com.facebook.react.devsupport.interfaces.DevLoadingViewManager
+
+internal class PackagerConnectionStatusNotifier(
+    private val devLoadingViewManager: DevLoadingViewManager?
+) {
+  private var hasConnected = false
+  private var connectionLost = false
+
+  fun onPackagerConnected() {
+    if (connectionLost) {
+      devLoadingViewManager?.showMessage(RECONNECTED_MESSAGE)
+    }
+    hasConnected = true
+    connectionLost = false
+  }
+
+  fun onPackagerDisconnected() {
+    if (hasConnected && !connectionLost) {
+      connectionLost = true
+      devLoadingViewManager?.showMessage(CONNECTION_LOST_MESSAGE)
+    }
+  }
+
+  private companion object {
+    const val CONNECTION_LOST_MESSAGE = "Connection to Metro lost. Retrying..."
+    const val RECONNECTED_MESSAGE = "Reconnected to Metro."
+  }
+}

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/packagerconnection/ReconnectingWebSocket.kt

@@ -42,6 +42,7 @@ public class ReconnectingWebSocket(
   private val okHttpClient = DevSupportHttpClient.websocketClient
   private var closed = false
   private var suppressConnectionErrors = false
+  private var connected = false
   private var webSocket: WebSocket? = null
 
   public fun connect() {
@@ -95,6 +96,7 @@ public class ReconnectingWebSocket(
   @Synchronized
   override fun onOpen(webSocket: WebSocket, response: Response) {
     this.webSocket = webSocket
+    connected = true
     suppressConnectionErrors = false
 
     connectionCallback?.onConnected()
@@ -106,7 +108,7 @@ public class ReconnectingWebSocket(
       abort("Websocket exception", t)
     }
     if (!closed) {
-      connectionCallback?.onDisconnected()
+      notifyDisconnectedIfConnected()
       reconnect()
     }
   }
@@ -125,11 +127,18 @@ public class ReconnectingWebSocket(
   override fun onClosed(webSocket: WebSocket, code: Int, reason: String) {
     this.webSocket = null
     if (!closed) {
-      connectionCallback?.onDisconnected()
+      notifyDisconnectedIfConnected()
       reconnect()
     }
   }
 
+  private fun notifyDisconnectedIfConnected() {
+    if (connected) {
+      connected = false
+      connectionCallback?.onDisconnected()
+    }
+  }
+
   @Synchronized
   @Throws(IOException::class)
   public fun sendMessage(message: String) {

packages/react-native/ReactAndroid/src/test/java/com/facebook/react/devsupport/PackagerConnectionStatusNotifierTest.kt

@@ -0,0 +1,68 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+package com.facebook.react.devsupport
+
+import com.facebook.react.devsupport.interfaces.DevLoadingViewManager
+import org.assertj.core.api.Assertions.assertThat
+import org.junit.Test
+
+class PackagerConnectionStatusNotifierTest {
+
+  private val devLoadingViewManager = RecordingDevLoadingViewManager()
+  private val notifier = PackagerConnectionStatusNotifier(devLoadingViewManager)
+
+  @Test
+  fun testInitialConnectionDoesNotShowReconnectedMessage() {
+    notifier.onPackagerConnected()
+
+    assertThat(devLoadingViewManager.messages).isEmpty()
+  }
+
+  @Test
+  fun testLostConnectionShowsRetryingOnceUntilReconnect() {
+    notifier.onPackagerConnected()
+
+    notifier.onPackagerDisconnected()
+    notifier.onPackagerDisconnected()
+
+    assertThat(devLoadingViewManager.messages)
+        .containsExactly("Connection to Metro lost. Retrying...")
+  }
+
+  @Test
+  fun testReconnectAfterLossShowsReconnectedMessage() {
+    notifier.onPackagerConnected()
+    notifier.onPackagerDisconnected()
+
+    notifier.onPackagerConnected()
+
+    assertThat(devLoadingViewManager.messages)
+        .containsExactly("Connection to Metro lost. Retrying...", "Reconnected to Metro.")
+  }
+
+  private class RecordingDevLoadingViewManager : DevLoadingViewManager {
+    val messages = mutableListOf<String>()
+
+    override fun showMessage(message: String) {
+      messages.add(message)
+    }
+
+    override fun showMessage(
+        message: String,
+        color: Double?,
+        backgroundColor: Double?,
+        dismissButton: Boolean?,
+    ) {
+      messages.add(message)
+    }
+
+    override fun updateProgress(status: String?, done: Int?, total: Int?, percent: Int?) = Unit
+
+    override fun hide() = Unit
+  }
+}

packages/react-native/ReactAndroid/src/test/java/com/facebook/react/packagerconnection/ReconnectingWebSocketTest.kt

@@ -0,0 +1,57 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+package com.facebook.react.packagerconnection
+
+import com.facebook.react.packagerconnection.ReconnectingWebSocket.ConnectionCallback
+import java.io.IOException
+import okhttp3.Response
+import okhttp3.WebSocket
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.mockito.kotlin.mock
+import org.mockito.kotlin.never
+import org.mockito.kotlin.times
+import org.mockito.kotlin.verify
+import org.robolectric.RobolectricTestRunner
+
+@RunWith(RobolectricTestRunner::class)
+class ReconnectingWebSocketTest {
+
+  @Test
+  fun testConnectionFailureBeforeOpenDoesNotNotifyDisconnected() {
+    val connectionCallback = mock<ConnectionCallback>()
+    val reconnectingWebSocket = createWebSocket(connectionCallback)
+
+    reconnectingWebSocket.onFailure(mock<WebSocket>(), IOException("failed"), null)
+
+    verify(connectionCallback, never()).onConnected()
+    verify(connectionCallback, never()).onDisconnected()
+  }
+
+  @Test
+  fun testConnectionFailureAfterOpenNotifiesDisconnectedOnceUntilReconnect() {
+    val connectionCallback = mock<ConnectionCallback>()
+    val reconnectingWebSocket = createWebSocket(connectionCallback)
+    val webSocket = mock<WebSocket>()
+
+    reconnectingWebSocket.onOpen(webSocket, mock<Response>())
+    reconnectingWebSocket.onFailure(webSocket, IOException("failed"), null)
+    reconnectingWebSocket.onFailure(mock<WebSocket>(), IOException("retry failed"), null)
+    reconnectingWebSocket.onOpen(mock<WebSocket>(), mock<Response>())
+
+    verify(connectionCallback, times(2)).onConnected()
+    verify(connectionCallback, times(1)).onDisconnected()
+  }
+
+  private fun createWebSocket(connectionCallback: ConnectionCallback): ReconnectingWebSocket =
+      ReconnectingWebSocket(
+          "ws://localhost:8081/message?role=android",
+          messageCallback = null,
+          connectionCallback = connectionCallback,
+      )
+}