ConnectivityManager: uses service error codes and exceptions

This patch introduces between ConnectivityManager and
ConnectivityService a mechanism for propagating back to clients
informative errors in the form of error codes and associated custom
runtime exceptions.

Without error code, the service can only throw a limited number of
different exceptions over Binder. Furthermore the throw site stack
traces are always loss. Although for individual instances of a throw,
the error message can be inspected, aggregations of stack traces from
app crashes sanitize error messages and only leaves the stack traces.

This makes debugging dificult for some service calls such as
requestNetwork that can have a variety of failure modes.

In this patch only one failure mode is codified. More can be added later
at a light cost by: 1) defining an error code, 2) defining an
associated exception, 3) mapping the code to the exception. This patch
can serves as a template for doing so.

Test: $ runtest frameworks-net,
      #testNetworkRequestMaximum() detects the new exception type.
Bug:  36556809, 36701874
Change-Id: I611fd7915575c9e418f7149fcdc8a879d2a3716d

diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java
index efa1959..6320134 100644 (file)
--- a/core/java/android/net/ConnectivityManager.java
+++ b/core/java/android/net/ConnectivityManager.java
@@ -39,6 +39,7 @@ import android.os.Process;
 import android.os.RemoteException;
 import android.os.ResultReceiver;
 import android.os.ServiceManager;
+import android.os.ServiceSpecificException;
 import android.provider.Settings;
 import android.telephony.SubscriptionManager;
 import android.util.ArrayMap;
@@ -2701,6 +2702,28 @@ public class ConnectivityManager {
         }
     }
 
+    /**
+     * Constant error codes used by ConnectivityService to communicate about failures and errors
+     * across a Binder boundary.
+     * @hide
+     */
+    public interface Errors {
+        static int TOO_MANY_REQUESTS = 1;
+    }
+
+    /** @hide */
+    public static class TooManyRequestsException extends RuntimeException {}
+
+    private static RuntimeException convertServiceException(ServiceSpecificException e) {
+        switch (e.errorCode) {
+            case Errors.TOO_MANY_REQUESTS:
+                return new TooManyRequestsException();
+            default:
+                Log.w(TAG, "Unknown service error code " + e.errorCode);
+                return new RuntimeException(e);
+        }
+    }
+
     private static final int BASE = Protocol.BASE_CONNECTIVITY_MANAGER;
     /** @hide */
     public static final int CALLBACK_PRECHECK            = BASE + 1;
@@ -2892,6 +2915,8 @@ public class ConnectivityManager {
             }
         } catch (RemoteException e) {
             throw e.rethrowFromSystemServer();
+        } catch (ServiceSpecificException e) {
+            throw convertServiceException(e);
         }
         return request;
     }
@@ -3177,6 +3202,8 @@ public class ConnectivityManager {
             mService.pendingRequestForNetwork(request.networkCapabilities, operation);
         } catch (RemoteException e) {
             throw e.rethrowFromSystemServer();
+        } catch (ServiceSpecificException e) {
+            throw convertServiceException(e);
         }
     }
 
@@ -3279,6 +3306,8 @@ public class ConnectivityManager {
             mService.pendingListenForNetwork(request.networkCapabilities, operation);
         } catch (RemoteException e) {
             throw e.rethrowFromSystemServer();
+        } catch (ServiceSpecificException e) {
+            throw convertServiceException(e);
         }
     }
 

diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index 88bc54d..4f7b834 100644 (file)
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -98,6 +98,7 @@ import android.os.PowerManager;
 import android.os.Process;
 import android.os.RemoteException;
 import android.os.ResultReceiver;
+import android.os.ServiceSpecificException;
 import android.os.SystemClock;
 import android.os.UserHandle;
 import android.os.UserManager;
@@ -4064,7 +4065,8 @@ public class ConnectivityService extends IConnectivityManager.Stub
             synchronized (mUidToNetworkRequestCount) {
                 int networkRequests = mUidToNetworkRequestCount.get(mUid, 0) + 1;
                 if (networkRequests >= MAX_NETWORK_REQUESTS_PER_UID) {
-                    throw new IllegalArgumentException("Too many NetworkRequests filed");
+                    throw new ServiceSpecificException(
+                            ConnectivityManager.Errors.TOO_MANY_REQUESTS);
                 }
                 mUidToNetworkRequestCount.put(mUid, networkRequests);
             }

diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java
index 5173278..3c0b8aa 100644 (file)
--- a/tests/net/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java
@@ -43,6 +43,7 @@ import android.net.ConnectivityManager;
 import android.net.ConnectivityManager.NetworkCallback;
 import android.net.ConnectivityManager.PacketKeepalive;
 import android.net.ConnectivityManager.PacketKeepaliveCallback;
+import android.net.ConnectivityManager.TooManyRequestsException;
 import android.net.INetworkPolicyManager;
 import android.net.INetworkStatsService;
 import android.net.IpPrefix;
@@ -2981,7 +2982,7 @@ public class ConnectivityServiceTest extends AndroidTestCase {
                 networkCallbacks.add(networkCallback);
             }
             fail("Registering " + MAX_REQUESTS + " NetworkRequests did not throw exception");
-        } catch (IllegalArgumentException expected) {}
+        } catch (TooManyRequestsException expected) {}
         for (NetworkCallback networkCallback : networkCallbacks) {
             mCm.unregisterNetworkCallback(networkCallback);
         }
@@ -2994,7 +2995,7 @@ public class ConnectivityServiceTest extends AndroidTestCase {
                 networkCallbacks.add(networkCallback);
             }
             fail("Registering " + MAX_REQUESTS + " NetworkCallbacks did not throw exception");
-        } catch (IllegalArgumentException expected) {}
+        } catch (TooManyRequestsException expected) {}
         for (NetworkCallback networkCallback : networkCallbacks) {
             mCm.unregisterNetworkCallback(networkCallback);
         }
@@ -3010,7 +3011,7 @@ public class ConnectivityServiceTest extends AndroidTestCase {
             }
             fail("Registering " + MAX_REQUESTS +
                     " PendingIntent NetworkRequests did not throw exception");
-        } catch (IllegalArgumentException expected) {}
+        } catch (TooManyRequestsException expected) {}
         for (PendingIntent pendingIntent : pendingIntents) {
             mCm.unregisterNetworkCallback(pendingIntent);
         }
@@ -3025,7 +3026,7 @@ public class ConnectivityServiceTest extends AndroidTestCase {
             }
             fail("Registering " + MAX_REQUESTS +
                     " PendingIntent NetworkCallbacks did not throw exception");
-        } catch (IllegalArgumentException expected) {}
+        } catch (TooManyRequestsException expected) {}
         for (PendingIntent pendingIntent : pendingIntents) {
             mCm.unregisterNetworkCallback(pendingIntent);
         }