Merge "Handle NPE for query in ConnectivityPublisher." into tm-qpr-dev

2 files changed, 54 insertions, 9 deletions

diff --git a/service/src/com/android/car/telemetry/publisher/ConnectivityPublisher.java b/service/src/com/android/car/telemetry/publisher/ConnectivityPublisher.java
index ca4ef84280..2bc7a8c3ee 100644
--- a/service/src/com/android/car/telemetry/publisher/ConnectivityPublisher.java
+++ b/service/src/com/android/car/telemetry/publisher/ConnectivityPublisher.java
@@ -173,7 +173,11 @@ public class ConnectivityPublisher extends AbstractPublisher {
         }
         try {
             for (QueryParam param : mSubscribers.keySet()) {
-                mTransportPreviousNetstats.put(param, getSummaryForAllUid(param));
+                RefinedStats summary = getSummaryForAllUid(param);
+                if (summary == null) {
+                    continue;
+                }
+                mTransportPreviousNetstats.put(param, summary);
             }
         } catch (RemoteException e) {
             // Can't do much if the NetworkStatsService is not available. Next netstats pull
@@ -233,13 +237,17 @@ public class ConnectivityPublisher extends AbstractPublisher {
         RefinedStats current;
         try {
             current = getSummaryForAllUid(param);
-        } catch (RemoteException | NullPointerException e) {
+        } catch (RemoteException e) {
             // If the NetworkStatsService is not available, it retries in the next pull.
             Slogf.w(CarLog.TAG_TELEMETRY, e);
             mTraceLog.traceEnd();
             return null;
         }
 
+        if (current == null) {
+            mTraceLog.traceEnd();
+            return null;
+        }
 
         // By subtracting, it calculates network usage since the last pull.
         RefinedStats diff = RefinedStats.subtract(current, previous);
@@ -260,7 +268,7 @@ public class ConnectivityPublisher extends AbstractPublisher {
      *
      * <p>TODO(b/218529196): run this method on a separate thread for better performance.
      */
-    @NonNull
+    @Nullable
     private RefinedStats getSummaryForAllUid(@NonNull QueryParam param) throws RemoteException {
         if (DEBUG) {
             Slogf.d(CarLog.TAG_TELEMETRY, "getSummaryForAllUid " + param);
@@ -273,12 +281,22 @@ public class ConnectivityPublisher extends AbstractPublisher {
                 - elapsedMillisSinceBoot
                 - NETSTATS_UID_DEFAULT_BUCKET_DURATION_MILLIS;
 
-        NetworkStatsWrapper nonTaggedStats =
-                mNetworkStatsManager.querySummary(
-                        param.buildNetworkTemplate(), startMillis, currentTimeInMillis);
-        NetworkStatsWrapper taggedStats =
-                mNetworkStatsManager.queryTaggedSummary(
-                        param.buildNetworkTemplate(), startMillis, currentTimeInMillis);
+        NetworkStatsWrapper nonTaggedStats;
+        NetworkStatsWrapper taggedStats;
+        // querySummary and queryTaggedSummary may throw NPE propagated from NetworkStatsService
+        // when its NetworkStatsRecorder failed to initialize and
+        // NetworkStatsRecorder.getOrLoadCompleteLocked() is called.
+        try {
+            nonTaggedStats =
+                    mNetworkStatsManager.querySummary(
+                            param.buildNetworkTemplate(), startMillis, currentTimeInMillis);
+            taggedStats =
+                    mNetworkStatsManager.queryTaggedSummary(
+                            param.buildNetworkTemplate(), startMillis, currentTimeInMillis);
+        } catch (NullPointerException e) {
+            Slogf.w(CarLog.TAG_TELEMETRY, e);
+            return null;
+        }
 
         RefinedStats result = new RefinedStats(startMillis, currentTimeInMillis);
         result.addNetworkStats(nonTaggedStats);
diff --git a/tests/carservice_unit_test/src/com/android/car/telemetry/publisher/ConnectivityPublisherTest.java b/tests/carservice_unit_test/src/com/android/car/telemetry/publisher/ConnectivityPublisherTest.java
index 93ebc907cb..e10dd1a876 100644
--- a/tests/carservice_unit_test/src/com/android/car/telemetry/publisher/ConnectivityPublisherTest.java
+++ b/tests/carservice_unit_test/src/com/android/car/telemetry/publisher/ConnectivityPublisherTest.java
@@ -527,6 +527,21 @@ public class ConnectivityPublisherTest {
                 .asList().containsExactly(1000L);
     }
 
+    @Test
+    public void testWhenQueryThrowsNullPointerExceptionIsCaught() {
+        mFakeManager.setSimulateFailedQuery(true);
+        mSessionControllerCallbackArgumentCaptor.getValue().onSessionStateChanged(
+                SESSION_ANNOTATION_BEGIN_1);
+
+        // querySummary gets called for each QueryParam combination, but throws
+        // NullPointerException each time, which is caught
+        assertThat(mFakeManager.getMethodCallCount("querySummary"))
+                .isEqualTo(BASELINE_PULL_COUNT);
+        // queryTaggedSummary not reached because of previous NullPointerException in querySummary
+        assertThat(mFakeManager.getMethodCallCount("queryTaggedSummary"))
+                .isEqualTo(0);
+    }
+
     private static class FakeDataSubscriber extends DataSubscriber {
         private final ArrayList<PushedData> mPushedData = new ArrayList<>();
 
@@ -569,6 +584,8 @@ public class ConnectivityPublisherTest {
         private final ArrayList<FakeNetworkStats.CustomBucket> mBuckets = new ArrayList<>();
         private final HashMap<String, Integer> mMethodCallCount = new HashMap<>();
 
+        private boolean mSimulateFailedQuery = false;
+
         private FakeNetworkStatsManager() {
             super(/* networkStatsManager= */ null);
         }
@@ -623,10 +640,17 @@ public class ConnectivityPublisherTest {
             return mMethodCallCount.getOrDefault(name, 0);
         }
 
+        public void setSimulateFailedQuery(boolean simulateFailedQuery) {
+            mSimulateFailedQuery = simulateFailedQuery;
+        }
+
         @Override
         @NonNull
         public NetworkStatsWrapper querySummary(NetworkTemplate template, long start, long end) {
             increaseMethodCall("querySummary", 1);
+            if (mSimulateFailedQuery) {
+                throw new NullPointerException();
+            }
             return commonQuerySummary(false, template, start, end);
         }
 
@@ -635,6 +659,9 @@ public class ConnectivityPublisherTest {
         public NetworkStatsWrapper queryTaggedSummary(
                 NetworkTemplate template, long start, long end) {
             increaseMethodCall("queryTaggedSummary", 1);
+            if (mSimulateFailedQuery) {
+                throw new NullPointerException();
+            }
             return commonQuerySummary(true, template, start, end);
         }