From 750f26eaa337e8cd3805730efbef927729bc7796 Mon Sep 17 00:00:00 2001
From: Michel Weststrate <mweststrate@fb.com>
Date: Tue, 9 Nov 2021 06:51:50 -0800
Subject: [PATCH] Fix potential NPE in Layout Inspector

Summary: Fixed NPE linked by task. From the code, that stuff should never be null, unless there is some threading / concurrency issue, which there apparently is. In that case, failing with a warning is imho a bit more elegant than raising exceptions from a debugging tool

Reviewed By: lblasa

Differential Revision: D32278044

fbshipit-source-id: 710fcdcfe458f33bbb806d9f2f1b9352252eedec
---
 .../plugins/inspector/InspectorFlipperPlugin.java      |  6 +++++-
 .../plugins/inspector/InspectorFlipperPluginTest.java  | 10 ++--------
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/android/src/main/java/com/facebook/flipper/plugins/inspector/InspectorFlipperPlugin.java b/android/src/main/java/com/facebook/flipper/plugins/inspector/InspectorFlipperPlugin.java
index b5101df4c..01fe6f840 100644
--- a/android/src/main/java/com/facebook/flipper/plugins/inspector/InspectorFlipperPlugin.java
+++ b/android/src/main/java/com/facebook/flipper/plugins/inspector/InspectorFlipperPlugin.java
@@ -592,7 +592,11 @@ public class InspectorFlipperPlugin implements FlipperPlugin {
       @Override
       protected void runOrThrow() throws Exception {
         for (int i = 0, count = descriptor.getChildCount(obj); i < count; i++) {
-          final Object child = assertNotNull(descriptor.getChildAt(obj, i));
+          final Object child = descriptor.getChildAt(obj, i);
+          if (child == null) {
+            Log.w(TAG, "Failed to get child at index: " + i);
+            break;
+          }
           children.put(trackObject(child));
         }
       }
diff --git a/android/src/test/java/com/facebook/flipper/plugins/inspector/InspectorFlipperPluginTest.java b/android/src/test/java/com/facebook/flipper/plugins/inspector/InspectorFlipperPluginTest.java
index 52140b095..a3f707c84 100644
--- a/android/src/test/java/com/facebook/flipper/plugins/inspector/InspectorFlipperPluginTest.java
+++ b/android/src/test/java/com/facebook/flipper/plugins/inspector/InspectorFlipperPluginTest.java
@@ -7,7 +7,6 @@
 
 package com.facebook.flipper.plugins.inspector;
 
-import static com.facebook.flipper.plugins.inspector.ThrowableMessageMatcher.hasThrowableWithMessage;
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.CoreMatchers.hasItem;
 import static org.hamcrest.MatcherAssert.assertThat;
@@ -31,7 +30,6 @@ import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 import javax.annotation.Nullable;
-import org.hamcrest.CoreMatchers;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -325,7 +323,7 @@ public class InspectorFlipperPluginTest {
   }
 
   @Test
-  public void testNullChildThrows() throws Exception {
+  public void testNullChildNotThrows() throws Exception {
     final InspectorFlipperPlugin plugin =
         new InspectorFlipperPlugin(mApp, mDescriptorMapping, null);
     final FlipperResponderMock responder = new FlipperResponderMock();
@@ -344,11 +342,7 @@ public class InspectorFlipperPluginTest {
         new FlipperObject.Builder().put("ids", new FlipperArray.Builder().put("test")).build(),
         responder);
 
-    assertThat(connection.errors.size(), equalTo(1));
-    assertThat(
-        connection.errors,
-        CoreMatchers.hasItem(
-            hasThrowableWithMessage("java.lang.RuntimeException: Unexpected null value")));
+    assertThat(connection.errors.size(), equalTo(0));
   }
 
   private class TestNode {