From ca3c524b49457616e2a3779bff69573223637e84 Mon Sep 17 00:00:00 2001
From: Remi NGUYEN VAN <reminv@google.com>
Date: Fri, 1 Dec 2023 09:07:24 +0000
Subject: [PATCH] Change throw to Log.wtf

Log.wtf should avoid crashes, while making sure that one can still
notice if there is a problem.

In particular, it may be possible that calling ensureIpServerStopped
and ensureIpServerStarted could start forwarding before the previous
IpServer has stopped it.

Bug: 314260975
Change-Id: I047a673d2fe6f218192d3144fa0bd60ce8739a15
Test: TH
---
 .../connectivity/RoutingCoordinatorService.java   |  3 ++-
 .../connectivity/RoutingCoordinatorServiceTest.kt | 15 ++++++++++++---
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/service/src/com/android/server/connectivity/RoutingCoordinatorService.java b/service/src/com/android/server/connectivity/RoutingCoordinatorService.java
index 3350d2d8b2..742a2ccd60 100644
--- a/service/src/com/android/server/connectivity/RoutingCoordinatorService.java
+++ b/service/src/com/android/server/connectivity/RoutingCoordinatorService.java
@@ -171,7 +171,8 @@ public class RoutingCoordinatorService extends IRoutingCoordinator.Stub {
             }
             final ForwardingPair fwp = new ForwardingPair(fromIface, toIface);
             if (mForwardedInterfaces.contains(fwp)) {
-                throw new IllegalStateException("Forward already exists between ifaces "
+                // TODO: remove if no reports are observed from the below log
+                Log.wtf(TAG, "Forward already exists between ifaces "
                         + fromIface + " → " + toIface);
             }
             mForwardedInterfaces.add(fwp);
diff --git a/tests/unit/java/com/android/server/connectivity/RoutingCoordinatorServiceTest.kt b/tests/unit/java/com/android/server/connectivity/RoutingCoordinatorServiceTest.kt
index 12758c6081..4e15d5fa87 100644
--- a/tests/unit/java/com/android/server/connectivity/RoutingCoordinatorServiceTest.kt
+++ b/tests/unit/java/com/android/server/connectivity/RoutingCoordinatorServiceTest.kt
@@ -18,14 +18,17 @@ package com.android.server.connectivity
 
 import android.net.INetd
 import android.os.Build
+import android.util.Log
 import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo
 import com.android.testutils.DevSdkIgnoreRunner
+import com.android.testutils.tryTest
+import java.util.concurrent.atomic.AtomicBoolean
+import kotlin.test.assertTrue
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.mockito.ArgumentMatchers.any
 import org.mockito.Mockito.inOrder
 import org.mockito.Mockito.mock
-import kotlin.test.assertFailsWith
 
 @RunWith(DevSdkIgnoreRunner::class)
 @IgnoreUpTo(Build.VERSION_CODES.TIRAMISU)
@@ -46,9 +49,15 @@ class RoutingCoordinatorServiceTest {
         inOrder.verify(mNetd).tetherAddForward("from2", "to1")
         inOrder.verify(mNetd).ipfwdAddInterfaceForward("from2", "to1")
 
-        assertFailsWith<IllegalStateException> {
-            // Can't add the same pair again
+        val hasFailed = AtomicBoolean(false)
+        val prevHandler = Log.setWtfHandler { tag, what, system ->
+            hasFailed.set(true)
+        }
+        tryTest {
             mService.addInterfaceForward("from2", "to1")
+            assertTrue(hasFailed.get())
+        } cleanup {
+            Log.setWtfHandler(prevHandler)
         }
 
         mService.removeInterfaceForward("from1", "to1")