Fix bridgeless triggering reloads twice from BridgelessDevSupportMana…

…ger (#44554) Summary: Pull Request resolved: #44554 Noticed than when reload is triggered by Metro (`handleReloadJS`), the application would often get stuck and not respond to further reload commands. Often an IOException would get printed as well, due to concurrent bundle loads happening. Changelog: [Android][Fixed] Improved resiliency of reloads when bundle loading fails Reviewed By: RSNara Differential Revision: D57112152 fbshipit-source-id: b0bf8c8311264504684a137c0910e2eeb008b0c7
facebook · May 15, 2024 · 524e3ee · 524e3ee
1 parent d0bb396
commit 524e3ee
Show file tree

Hide file tree

Showing 9 changed files with 43 additions and 52 deletions.
diff --git a/packages/react-native/ReactAndroid/api/ReactAndroid.api b/packages/react-native/ReactAndroid/api/ReactAndroid.api
@@ -2115,7 +2115,6 @@ public abstract class com/facebook/react/devsupport/DevSupportManagerBase : com/
  public fun openDebugger ()V
  public fun processErrorCustomizers (Landroid/util/Pair;)Landroid/util/Pair;
  public fun registerErrorCustomizer (Lcom/facebook/react/devsupport/interfaces/ErrorCustomizer;)V
- public fun reloadJSFromServer (Ljava/lang/String;)V
  public fun reloadJSFromServer (Ljava/lang/String;Lcom/facebook/react/devsupport/interfaces/BundleLoadCallback;)V
  public fun reloadSettings ()V
  public fun setDevSupportEnabled (Z)V
@@ -2282,7 +2281,6 @@ public class com/facebook/react/devsupport/ReleaseDevSupportManager : com/facebo
  public fun openDebugger ()V
  public fun processErrorCustomizers (Landroid/util/Pair;)Landroid/util/Pair;
  public fun registerErrorCustomizer (Lcom/facebook/react/devsupport/interfaces/ErrorCustomizer;)V
- public fun reloadJSFromServer (Ljava/lang/String;)V
  public fun reloadJSFromServer (Ljava/lang/String;Lcom/facebook/react/devsupport/interfaces/BundleLoadCallback;)V
  public fun reloadSettings ()V
  public fun setDevSupportEnabled (Z)V
@@ -2341,6 +2339,7 @@ public class com/facebook/react/devsupport/WebsocketJavaScriptExecutor$Websocket
 }
 
 public abstract interface class com/facebook/react/devsupport/interfaces/BundleLoadCallback {
+ public fun onError (Ljava/lang/Exception;)V
  public abstract fun onSuccess ()V
 }
 
@@ -2394,7 +2393,6 @@ public abstract interface class com/facebook/react/devsupport/interfaces/DevSupp
  public abstract fun openDebugger ()V
  public abstract fun processErrorCustomizers (Landroid/util/Pair;)Landroid/util/Pair;
  public abstract fun registerErrorCustomizer (Lcom/facebook/react/devsupport/interfaces/ErrorCustomizer;)V
- public abstract fun reloadJSFromServer (Ljava/lang/String;)V
  public abstract fun reloadJSFromServer (Ljava/lang/String;Lcom/facebook/react/devsupport/interfaces/BundleLoadCallback;)V
  public abstract fun reloadSettings ()V
  public abstract fun setDevSupportEnabled (Z)V

diff --git a/...ive/ReactAndroid/src/main/java/com/facebook/react/devsupport/BridgeDevSupportManager.java b/...ive/ReactAndroid/src/main/java/com/facebook/react/devsupport/BridgeDevSupportManager.java
@@ -62,8 +62,6 @@
  */
 public final class BridgeDevSupportManager extends DevSupportManagerBase {
  private boolean mIsSamplingProfilerEnabled = false;
- private ReactInstanceDevHelper mReactInstanceManagerHelper;
- private @Nullable DevLoadingViewManager mDevLoadingViewManager;
 
  public BridgeDevSupportManager(
  Context applicationContext,
@@ -211,7 +209,11 @@ public void handleReloadJS() {
  String bundleURL =
  getDevServerHelper()
  .getDevServerBundleURL(Assertions.assertNotNull(getJSAppBundleName()));
- reloadJSFromServer(bundleURL);
+ reloadJSFromServer(
+ bundleURL,
+ () ->
+ UiThreadUtil.runOnUiThread(
+ () -> getReactInstanceDevHelper().onJSBundleLoadedFromServer()));
  }
  }
 

diff --git a/...act-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/BundleDownloader.java b/...act-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/BundleDownloader.java
@@ -24,6 +24,7 @@
 import okhttp3.OkHttpClient;
 import okhttp3.Request;
 import okhttp3.Response;
+import okhttp3.ResponseBody;
 import okio.Buffer;
 import okio.BufferedSource;
 import okio.Okio;
@@ -146,14 +147,16 @@ public void onResponse(Call call, final Response response) throws IOException {
  } else {
  // In case the server doesn't support multipart/mixed responses, fallback to normal
  // download.
- processBundleResult(
- url,
- r.code(),
- r.headers(),
- Okio.buffer(r.body().source()),
- outputFile,
- bundleInfo,
- callback);
+ try (ResponseBody body = r.body()) {
+ processBundleResult(
+ url,
+ r.code(),
+ r.headers(),
+ r.body().source(),
+ outputFile,
+ bundleInfo,
+ callback);
+ }
  }
  }
  }

diff --git a/...ative/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerBase.java b/...ative/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerBase.java
@@ -910,12 +910,6 @@ private void updateLastErrorInfo(
  mLastErrorType = errorType;
  }
 
- public void reloadJSFromServer(final String bundleURL) {
- reloadJSFromServer(
- bundleURL,
- () -> UiThreadUtil.runOnUiThread(mReactInstanceDevHelper::onJSBundleLoadedFromServer));
- }
-
  public void reloadJSFromServer(final String bundleURL, final BundleLoadCallback callback) {
  ReactMarker.logMarker(ReactMarkerConstants.DOWNLOAD_START);
 
@@ -954,6 +948,7 @@ public void onFailure(final Exception cause) {
  }
  FLog.e(ReactConstants.TAG, "Unable to download JS bundle", cause);
  reportBundleLoadingFailure(cause);
+ callback.onError(cause);
  }
  },
  mJSBundleDownloadedFile,

diff --git a/...ve/ReactAndroid/src/main/java/com/facebook/react/devsupport/ReleaseDevSupportManager.java b/...ve/ReactAndroid/src/main/java/com/facebook/react/devsupport/ReleaseDevSupportManager.java
@@ -138,9 +138,6 @@ public void reloadSettings() {}
  @Override
  public void handleReloadJS() {}
 
- @Override
- public void reloadJSFromServer(String bundleURL) {}
-
  @Override
  public void reloadJSFromServer(final String bundleURL, final BundleLoadCallback callback) {}
 

diff --git a/...ReactAndroid/src/main/java/com/facebook/react/devsupport/interfaces/BundleLoadCallback.kt b/...ReactAndroid/src/main/java/com/facebook/react/devsupport/interfaces/BundleLoadCallback.kt
@@ -8,5 +8,7 @@
 package com.facebook.react.devsupport.interfaces
 
 public fun interface BundleLoadCallback {
- public fun onSuccess()
+ public fun onSuccess(): Unit
+
+ public fun onError(cause: Exception): Unit = Unit
 }
diff --git a/.../ReactAndroid/src/main/java/com/facebook/react/devsupport/interfaces/DevSupportManager.kt b/.../ReactAndroid/src/main/java/com/facebook/react/devsupport/interfaces/DevSupportManager.kt
@@ -58,18 +58,16 @@ public interface DevSupportManager : JSExceptionHandler {
 
  public fun stopInspector()
 
- public fun onNewReactContextCreated(reactContext: ReactContext?)
+ public fun onNewReactContextCreated(reactContext: ReactContext)
 
- public fun onReactInstanceDestroyed(reactContext: ReactContext?)
+ public fun onReactInstanceDestroyed(reactContext: ReactContext)
 
  public fun hasUpToDateJSBundleInCache(): Boolean
 
  public fun reloadSettings()
 
  public fun handleReloadJS()
 
- public fun reloadJSFromServer(bundleURL: String)
-
  public fun reloadJSFromServer(bundleURL: String, callback: BundleLoadCallback)
 
  public fun loadSplitBundleFromServer(bundlePath: String, callback: DevSplitBundleCallback)

diff --git a/...ve/ReactAndroid/src/main/java/com/facebook/react/runtime/BridgelessDevSupportManager.java b/...ve/ReactAndroid/src/main/java/com/facebook/react/runtime/BridgelessDevSupportManager.java
@@ -11,9 +11,7 @@
 import android.content.Context;
 import android.os.Bundle;
 import android.view.View;
-import com.facebook.debug.holder.PrinterHolder;
-import com.facebook.debug.tags.ReactDebugOverlayTags;
-import com.facebook.infer.annotation.Assertions;
+import androidx.annotation.Nullable;
 import com.facebook.infer.annotation.Nullsafe;
 import com.facebook.react.bridge.JSBundleLoader;
 import com.facebook.react.bridge.JavaJSExecutor;
@@ -27,7 +25,6 @@
 import com.facebook.react.modules.core.DeviceEventManagerModule;
 import com.facebook.react.runtime.internal.bolts.Continuation;
 import com.facebook.react.runtime.internal.bolts.Task;
-import javax.annotation.Nullable;
 
 /**
  * An implementation of {@link com.facebook.react.devsupport.interfaces.DevSupportManager} that
@@ -103,24 +100,18 @@ public void handleReloadJS() {
  // dismiss redbox if exists
  hideRedboxDialog();
  mReactHost.reload("BridgelessDevSupportManager.handleReloadJS()");
-
- PrinterHolder.getPrinter()
- .logMessage(ReactDebugOverlayTags.RN_CORE, "RNCore: load from Server");
- String bundleURL =
- getDevServerHelper().getDevServerBundleURL(Assertions.assertNotNull(getJSAppBundleName()));
- reloadJSFromServer(bundleURL);
  }
 
  private static ReactInstanceDevHelper createInstanceDevHelper(final ReactHostImpl reactHost) {
  return new ReactInstanceDevHelper() {
  @Override
  public void onReloadWithJSDebugger(JavaJSExecutor.Factory proxyExecutorFactory) {
- // Not implemented
+ // Not implemented, only used by BridgeDevSupportManager to reload with proxy executor
  }
 
  @Override
  public void onJSBundleLoadedFromServer() {
- // Not implemented
+ // Not implemented, only referenced by BridgeDevSupportManager
  }
 
  @Override
@@ -133,7 +124,7 @@ public void toggleElementInspector() {
  }
  }
 
- @androidx.annotation.Nullable
+ @Nullable
  @Override
  public Activity getCurrentActivity() {
  return reactHost.getLastUsedActivity();
@@ -144,7 +135,7 @@ public JavaScriptExecutorFactory getJavaScriptExecutorFactory() {
  throw new IllegalStateException("Not implemented for bridgeless mode");
  }
 
- @androidx.annotation.Nullable
+ @Nullable
  @Override
  public View createRootView(String appKey) {
  Activity currentActivity = getCurrentActivity();

diff --git a/...ges/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactHostImpl.java b/...ges/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactHostImpl.java
@@ -52,6 +52,7 @@
 import com.facebook.react.devsupport.DevSupportManagerBase;
 import com.facebook.react.devsupport.InspectorFlags;
 import com.facebook.react.devsupport.ReleaseDevSupportManager;
+import com.facebook.react.devsupport.interfaces.BundleLoadCallback;
 import com.facebook.react.devsupport.interfaces.DevSupportManager;
 import com.facebook.react.devsupport.interfaces.DevSupportManager.PausedInDebuggerOverlayCommandListener;
 import com.facebook.react.fabric.ComponentFactory;
@@ -1183,10 +1184,6 @@ private Task<Boolean> isMetroRunning() {
  return taskCompletionSource.getTask();
  }
 
- /**
- * TODO(T104078367): Ensure that if creating this JSBundleLoader fails, we route the errors
- * through ReactHost's error reporting pipeline
- */
  private Task<JSBundleLoader> loadJSBundleFromMetro() {
  final String method = "loadJSBundleFromMetro()";
  log(method);
@@ -1202,12 +1199,20 @@ private Task<JSBundleLoader> loadJSBundleFromMetro() {
 
  asyncDevSupportManager.reloadJSFromServer(
  bundleURL,
- () -> {
- log(method, "Creating BundleLoader");
- JSBundleLoader bundleLoader =
- JSBundleLoader.createCachedBundleFromNetworkLoader(
- bundleURL, asyncDevSupportManager.getDownloadedJSBundleFile());
- taskCompletionSource.setResult(bundleLoader);
+ new BundleLoadCallback() {
+ @Override
+ public void onSuccess() {
+ log(method, "Creating BundleLoader");
+ JSBundleLoader bundleLoader =
+ JSBundleLoader.createCachedBundleFromNetworkLoader(
+ bundleURL, asyncDevSupportManager.getDownloadedJSBundleFile());
+ taskCompletionSource.setResult(bundleLoader);
+ }
+
+ @Override
+ public void onError(Exception cause) {
+ taskCompletionSource.setError(cause);
+ }
  });
 
  return taskCompletionSource.getTask();