Enhance/fix error reporting in reload and destroy

Summary:
The new reload/create/destroy methods work by chaining tasks together.

This task chain has the type Task<ReactInstance>.

**The problem:** If any step in the chain fails, task.getResult() actually returns null - not the ReactInstance. Many steps in the existing reload() and destroy() task chains don't account for this case. So:
- The reload() and destroy() task chains sometimes swallow errors.
- Sometimes steps in the reload() and destroy() task chains don't execute: they use .successTask

This diff makes two changes:
1. Ensure each step **always** executes (i.e: use .continueWith vs .success)
2. Ensure each step first checks if the Task<ReactInstance> isn't faulted/cancelled. If the task is faulted/cancelled, a soft exception gets reported, and the current ReactInstance gets returned.

Changelog: [Internal

Reviewed By: mdvacca

Differential Revision: D48080779

fbshipit-source-id: 22f03ef1a54b538d01eeb5ecde6d82a84d32f1f8

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridgeless/ReactHostImpl.java

@@ -1180,6 +1180,61 @@ private void startAttachedSurfaces(String method, ReactInstance reactInstance) {
   @ThreadConfined("ReactHost")
   private @Nullable Task<ReactInstance> mReloadTask = null;
 
+  private interface ReactInstanceTaskUnwrapper {
+    @Nullable
+    ReactInstance unwrap(Task<ReactInstance> task, String stage);
+  }
+
+  private ReactInstanceTaskUnwrapper createReactInstanceUnwraper(
+      String tag, String method, String reason) {
+
+    return (task, stage) -> {
+      final ReactInstance reactInstance = task.getResult();
+      final ReactInstance currentReactInstance = mReactInstanceTaskRef.get().getResult();
+
+      final String stageLabel = "Stage: " + stage;
+      final String reasonLabel = tag + " reason: " + reason;
+      if (task.isFaulted()) {
+        final Exception ex = task.getError();
+        final String faultLabel = "Fault reason: " + ex.getMessage();
+        raiseSoftException(
+            method,
+            tag
+                + ": ReactInstance task faulted. "
+                + stageLabel
+                + ". "
+                + faultLabel
+                + ". "
+                + reasonLabel);
+        return currentReactInstance;
+      }
+
+      if (task.isCancelled()) {
+        raiseSoftException(
+            method, tag + ": ReactInstance task cancelled. " + stageLabel + ". " + reasonLabel);
+        return currentReactInstance;
+      }
+
+      if (reactInstance == null) {
+        raiseSoftException(
+            method, tag + ": ReactInstance task returned null. " + stageLabel + ". " + reasonLabel);
+        return currentReactInstance;
+      }
+
+      if (currentReactInstance != null && reactInstance != currentReactInstance) {
+        raiseSoftException(
+            method,
+            tag
+                + ": Detected two different ReactInstances. Returning old. "
+                + stageLabel
+                + ". "
+                + reasonLabel);
+      }
+
+      return reactInstance;
+    };
+  }
+
   /**
    * The ReactInstance is loaded. Tear it down, and re-create it.
    *
@@ -1197,30 +1252,18 @@ private Task<ReactInstance> newGetOrCreateReloadTask(String reason) {
     // TODO(T136397487): Remove after Venice is shipped to 100%
     raiseSoftException(method, reason);
 
+    ReactInstanceTaskUnwrapper reactInstanceTaskUnwrapper =
+        createReactInstanceUnwraper("Reload", method, reason);
+
     if (mReloadTask == null) {
       mReloadTask =
           mReactInstanceTaskRef
               .get()
               .continueWithTask(
                   (task) -> {
-                    log(method, "Starting on UI thread");
-
-                    if (task.isFaulted()) {
-                      raiseSoftException(
-                          method,
-                          "ReactInstance task faulted. Reload reason: " + reason,
-                          task.getError());
-                    }
-
-                    if (task.isCancelled()) {
-                      raiseSoftException(
-                          method, "ReactInstance task cancelled. Reload reason: " + reason);
-                    }
-
-                    final ReactInstance reactInstance = task.getResult();
-                    if (reactInstance == null) {
-                      raiseSoftException(method, "ReactInstance is null. Reload reason: " + reason);
-                    }
+                    log(method, "Starting React Native reload");
+                    final ReactInstance reactInstance =
+                        reactInstanceTaskUnwrapper.unwrap(task, "1: Starting reload");
 
                     final ReactContext reactContext = mBridgelessReactContextRef.getNullable();
                     if (reactContext == null) {
@@ -1234,13 +1277,16 @@ private Task<ReactInstance> newGetOrCreateReloadTask(String reason) {
                       reactContext.onHostPause();
                     }
 
-                    return task;
+                    return Task.forResult(reactInstance);
                   },
                   mUIExecutor)
               .continueWithTask(
                   task -> {
-                    final ReactInstance reactInstance = task.getResult();
+                    final ReactInstance reactInstance =
+                        reactInstanceTaskUnwrapper.unwrap(task, "2: Surface shutdown");
+
                     if (reactInstance == null) {
+                      raiseSoftException(method, "Skipping surface shutdown: ReactInstance null");
                       return task;
                     }
 
@@ -1250,6 +1296,8 @@ private Task<ReactInstance> newGetOrCreateReloadTask(String reason) {
                   mBGExecutor)
               .continueWithTask(
                   task -> {
+                    reactInstanceTaskUnwrapper.unwrap(task, "3: Destroying ReactContext");
+
                     log(method, "Removing memory pressure listener");
                     mMemoryPressureRouter.removeMemoryPressureListener(mMemoryPressureListener);
 
@@ -1271,10 +1319,14 @@ private Task<ReactInstance> newGetOrCreateReloadTask(String reason) {
                   mUIExecutor)
               .continueWithTask(
                   task -> {
-                    final ReactInstance reactInstance = task.getResult();
+                    final ReactInstance reactInstance =
+                        reactInstanceTaskUnwrapper.unwrap(task, "4: Destroying ReactInstance");
 
-                    log(method, "Destroying ReactInstance");
-                    if (reactInstance != null) {
+                    if (reactInstance == null) {
+                      raiseSoftException(
+                          method, "Skipping ReactInstance.destroy(): ReactInstance null");
+                    } else {
+                      log(method, "Destroying ReactInstance");
                       reactInstance.destroy();
                     }
 
@@ -1291,31 +1343,38 @@ private Task<ReactInstance> newGetOrCreateReloadTask(String reason) {
                     return newGetOrCreateReactInstanceTask();
                   },
                   mBGExecutor)
-              .onSuccess(
+              .continueWithTask(
                   task -> {
-                    final ReactInstance reactInstance = task.getResult();
+                    final ReactInstance reactInstance =
+                        reactInstanceTaskUnwrapper.unwrap(task, "5: Restarting surfaces");
+
                     if (reactInstance == null) {
-                      return null;
+                      raiseSoftException(method, "Skipping surface restart: ReactInstance null");
+                      return task;
                     }
 
                     startAttachedSurfaces(method, reactInstance);
-                    return reactInstance;
+
+                    return task;
                   },
                   mBGExecutor)
               .continueWithTask(
                   task -> {
                     if (task.isFaulted()) {
+                      Exception fault = task.getError();
                       raiseSoftException(
                           method,
-                          "Failed to re-created ReactInstance. Task faulted. Reload reason: "
+                          "Error during reload. ReactInstance task faulted. Fault reason: "
+                              + fault.getMessage()
+                              + ". Reload reason: "
                               + reason,
                           task.getError());
                     }
 
                     if (task.isCancelled()) {
                       raiseSoftException(
                           method,
-                          "Failed to re-created ReactInstance. Task cancelled. Reload reason: "
+                          "Error during reload. ReactInstance task cancelled. Reload reason: "
                               + reason);
                     }
 
@@ -1349,31 +1408,19 @@ private Task<Void> newGetOrCreateDestroyTask(final String reason, @Nullable Exce
     // TODO(T136397487): Remove after Venice is shipped to 100%
     raiseSoftException(method, reason, ex);
 
+    ReactInstanceTaskUnwrapper reactInstanceTaskUnwrapper =
+        createReactInstanceUnwraper("Destroy", method, reason);
+
     if (mDestroyTask == null) {
       mDestroyTask =
           mReactInstanceTaskRef
               .get()
               .continueWithTask(
                   task -> {
-                    log(method, "Destroying ReactInstance on UI Thread");
+                    log(method, "Starting React Native destruction");
 
-                    if (task.isFaulted()) {
-                      raiseSoftException(
-                          method,
-                          "ReactInstance task faulted. Destroy reason: " + reason,
-                          task.getError());
-                    }
-
-                    if (task.isCancelled()) {
-                      raiseSoftException(
-                          method, "ReactInstance task cancelled. Destroy reason: " + reason);
-                    }
-
-                    final ReactInstance reactInstance = task.getResult();
-                    if (reactInstance == null) {
-                      raiseSoftException(
-                          method, "ReactInstance is null. Destroy reason: " + reason);
-                    }
+                    final ReactInstance reactInstance =
+                        reactInstanceTaskUnwrapper.unwrap(task, "1: Starting destroy");
 
                     // Step 1: Destroy DevSupportManager
                     if (mUseDevSupport) {
@@ -1392,13 +1439,16 @@ private Task<Void> newGetOrCreateDestroyTask(final String reason, @Nullable Exce
                     log(method, "Move ReactHost to onHostDestroy()");
                     mReactLifecycleStateManager.moveToOnHostDestroy(reactContext);
 
-                    return task;
+                    return Task.forResult(reactInstance);
                   },
                   mUIExecutor)
               .continueWithTask(
                   task -> {
-                    final ReactInstance reactInstance = task.getResult();
+                    final ReactInstance reactInstance =
+                        reactInstanceTaskUnwrapper.unwrap(task, "2: Stopping surfaces");
+
                     if (reactInstance == null) {
+                      raiseSoftException(method, "Skipping surface shutdown: ReactInstance null");
                       return task;
                     }
 
@@ -1413,6 +1463,8 @@ private Task<Void> newGetOrCreateDestroyTask(final String reason, @Nullable Exce
                   mBGExecutor)
               .continueWithTask(
                   task -> {
+                    reactInstanceTaskUnwrapper.unwrap(task, "3: Destroying ReactContext");
+
                     final ReactContext reactContext = mBridgelessReactContextRef.getNullable();
 
                     if (reactContext == null) {
@@ -1437,10 +1489,15 @@ private Task<Void> newGetOrCreateDestroyTask(final String reason, @Nullable Exce
                     return task;
                   },
                   mUIExecutor)
-              .continueWith(
+              .continueWithTask(
                   task -> {
-                    final ReactInstance reactInstance = task.getResult();
-                    if (reactInstance != null) {
+                    final ReactInstance reactInstance =
+                        reactInstanceTaskUnwrapper.unwrap(task, "3: Destroying ReactInstance");
+
+                    if (reactInstance == null) {
+                      raiseSoftException(
+                          method, "Skipping ReactInstance.destroy(): ReactInstance null");
+                    } else {
                       log(method, "Destroying ReactInstance");
                       reactInstance.destroy();
                     }
@@ -1456,9 +1513,30 @@ private Task<Void> newGetOrCreateDestroyTask(final String reason, @Nullable Exce
 
                     log(method, "Resetting destroy task ref");
                     mDestroyTask = null;
-                    return null;
+                    return task;
                   },
-                  mBGExecutor);
+                  mBGExecutor)
+              .continueWith(
+                  task -> {
+                    if (task.isFaulted()) {
+                      Exception fault = task.getError();
+                      raiseSoftException(
+                          method,
+                          "React destruction failed. ReactInstance task faulted. Fault reason: "
+                              + fault.getMessage()
+                              + ". Destroy reason: "
+                              + reason,
+                          task.getError());
+                    }
+
+                    if (task.isCancelled()) {
+                      raiseSoftException(
+                          method,
+                          "React destruction failed. ReactInstance task cancelled. Destroy reason: "
+                              + reason);
+                    }
+                    return null;
+                  });
     }
 
     return mDestroyTask;

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridgeless/internal/bolts/Task.java

@@ -7,6 +7,7 @@
 
 package com.facebook.react.bridgeless.internal.bolts;
 
+import androidx.annotation.Nullable;
 import com.facebook.react.interfaces.TaskInterface;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -187,7 +188,7 @@ public boolean waitForCompletion(long duration, TimeUnit timeUnit) throws Interr
 
   /** Creates a completed task with the given value. */
   @SuppressWarnings("unchecked")
-  public static <TResult> Task<TResult> forResult(TResult value) {
+  public static <TResult> Task<TResult> forResult(@Nullable TResult value) {
     if (value == null) {
       return (Task<TResult>) TASK_NULL;
     }