Bigtable: non-functional improvements relating to error handling (#3809)

* Use gax's async error handling now that it has been upstreamed

* Bigtable: non-functional improvements

* Add error handling examples to the javadocs
* Use gax for error handling now that the code has been upstreamed
* Fix outdated bigtable-admin readme

* spacing

* more docs

* more docs

* address feedback

Author: igorbernstein2

google-cloud-clients/google-cloud-bigtable-admin/README.md

@@ -72,22 +72,23 @@ at the top of your file:
 
 
 ```java
-import com.google.bigtable.admin.v2.ColumnFamily;
+import static com.google.cloud.bigtable.admin.v2.models.GCRules.GCRULES;
 import com.google.bigtable.admin.v2.InstanceName;
 import com.google.cloud.bigtable.admin.v2.BigtableTableAdminClient;
+import com.google.cloud.bigtable.admin.v2.models.CreateTableRequest;
+import com.google.cloud.bigtable.admin.v2.models.Table;
 ```
 
 Then, to create a table, use the following code:
 ```java
 InstanceName instanceName = InstanceName.of("my-project", "my-instance");
 
-TableAdminClient tableAdminClient = TableAdminClient.create(instanceName);
+BigtableTableAdminClient tableAdminClient = BigtableTableAdminClient.create(instanceName);
 
 try {
-    CreateTable createTableReq = TableAdminRequests.createTable("tableId")
+  Table createdTable = tableAdminClient.createTable(
+    CreateTableRequest.of("my-table")
       .addFamily("cf2", GCRULES.maxVersions(10));
-    client.createTable(createTableReq);
-
 } finally {
   tableAdminClient.close();
 }

google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableInstanceAdminClient.java

@@ -19,6 +19,7 @@
 import com.google.api.core.ApiFunction;
 import com.google.api.core.ApiFuture;
 import com.google.api.core.ApiFutures;
+import com.google.api.gax.rpc.ApiExceptions;
 import com.google.api.resourcenames.ResourceName;
 import com.google.bigtable.admin.v2.AppProfileName;
 import com.google.bigtable.admin.v2.ClusterName;
@@ -46,9 +47,7 @@
 import com.google.common.base.Verify;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
-import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.MoreExecutors;
-import com.google.common.util.concurrent.UncheckedExecutionException;
 import com.google.iam.v1.GetIamPolicyRequest;
 import com.google.iam.v1.SetIamPolicyRequest;
 import com.google.iam.v1.TestIamPermissionsRequest;
@@ -160,7 +159,7 @@ public void close() {
    */
   @SuppressWarnings("WeakerAccess")
   public Instance createInstance(CreateInstanceRequest request) {
-    return awaitFuture(createInstanceAsync(request));
+    return ApiExceptions.callAndTranslateApiException(createInstanceAsync(request));
   }
 
   /**
@@ -208,7 +207,7 @@ public Instance apply(com.google.bigtable.admin.v2.Instance proto) {
    */
   @SuppressWarnings("WeakerAccess")
   public Instance updateInstance(UpdateInstanceRequest request) {
-    return awaitFuture(updateInstanceAsync(request));
+    return ApiExceptions.callAndTranslateApiException(updateInstanceAsync(request));
   }
 
   /**
@@ -251,7 +250,7 @@ public Instance apply(com.google.bigtable.admin.v2.Instance proto) {
    */
   @SuppressWarnings("WeakerAccess")
   public Instance getInstance(String id) {
-    return awaitFuture(getInstanceAsync(id));
+    return ApiExceptions.callAndTranslateApiException(getInstanceAsync(id));
   }
 
   /**
@@ -302,7 +301,7 @@ public Instance apply(com.google.bigtable.admin.v2.Instance proto) {
    */
   @SuppressWarnings("WeakerAccess")
   public List<Instance> listInstances() {
-    return awaitFuture(listInstancesAsync());
+    return ApiExceptions.callAndTranslateApiException(listInstancesAsync());
   }
 
   /**
@@ -385,7 +384,7 @@ public List<Instance> apply(com.google.bigtable.admin.v2.ListInstancesResponse p
    */
   @SuppressWarnings("WeakerAccess")
   public void deleteInstance(String instanceId) {
-    awaitFuture(deleteInstanceAsync(instanceId));
+    ApiExceptions.callAndTranslateApiException(deleteInstanceAsync(instanceId));
   }
 
   /**
@@ -434,7 +433,7 @@ public Void apply(Empty input) {
    */
   @SuppressWarnings("WeakerAccess")
   public Cluster createCluster(CreateClusterRequest request) {
-    return awaitFuture(createClusterAsync(request));
+    return ApiExceptions.callAndTranslateApiException(createClusterAsync(request));
   }
 
   /**
@@ -478,7 +477,7 @@ public Cluster apply(com.google.bigtable.admin.v2.Cluster proto) {
    */
   @SuppressWarnings("WeakerAccess")
   public Cluster getCluster(String instanceId, String clusterId) {
-    return awaitFuture(getClusterAsync(instanceId, clusterId));
+    return ApiExceptions.callAndTranslateApiException(getClusterAsync(instanceId, clusterId));
   }
 
   /**
@@ -531,7 +530,7 @@ public Cluster apply(com.google.bigtable.admin.v2.Cluster proto) {
    */
   @SuppressWarnings("WeakerAccess")
   public List<Cluster> listClusters(String instanceId) {
-    return awaitFuture(listClustersAsync(instanceId));
+    return ApiExceptions.callAndTranslateApiException(listClustersAsync(instanceId));
   }
 
   /**
@@ -615,7 +614,8 @@ public List<Cluster> apply(com.google.bigtable.admin.v2.ListClustersResponse pro
    */
   @SuppressWarnings("WeakerAccess")
   public Cluster resizeCluster(String instanceId, String clusterId, int numServeNodes) {
-    return awaitFuture(resizeClusterAsync(instanceId, clusterId, numServeNodes));
+    return ApiExceptions.callAndTranslateApiException(
+        resizeClusterAsync(instanceId, clusterId, numServeNodes));
   }
 
   /**
@@ -662,7 +662,7 @@ public Cluster apply(com.google.bigtable.admin.v2.Cluster proto) {
    */
   @SuppressWarnings("WeakerAccess")
   public void deleteCluster(String instanceId, String clusterId) {
-    awaitFuture(deleteClusterAsync(instanceId, clusterId));
+    ApiExceptions.callAndTranslateApiException(deleteClusterAsync(instanceId, clusterId));
   }
 
   /**
@@ -713,7 +713,7 @@ public Void apply(Empty input) {
    */
   @SuppressWarnings("WeakerAccess")
   public AppProfile createAppProfile(CreateAppProfileRequest request) {
-    return awaitFuture(createAppProfileAsync(request));
+    return ApiExceptions.callAndTranslateApiException(createAppProfileAsync(request));
   }
 
   /**
@@ -759,7 +759,7 @@ public AppProfile apply(com.google.bigtable.admin.v2.AppProfile proto) {
    */
   @SuppressWarnings("WeakerAccess")
   public AppProfile getAppProfile(String instanceId, String appProfileId) {
-    return awaitFuture(getAppProfileAsync(instanceId, appProfileId));
+    return ApiExceptions.callAndTranslateApiException(getAppProfileAsync(instanceId, appProfileId));
   }
 
   /**
@@ -808,7 +808,7 @@ public AppProfile apply(com.google.bigtable.admin.v2.AppProfile proto) {
    */
   @SuppressWarnings("WeakerAccess")
   public List<AppProfile> listAppProfiles(String instanceId) {
-    return awaitFuture(listAppProfilesAsync(instanceId));
+    return ApiExceptions.callAndTranslateApiException(listAppProfilesAsync(instanceId));
   }
 
   /**
@@ -911,7 +911,7 @@ public List<AppProfile> apply(List<com.google.bigtable.admin.v2.AppProfile> inpu
    */
   @SuppressWarnings("WeakerAccess")
   public AppProfile updateAppProfile(UpdateAppProfileRequest request) {
-    return awaitFuture(updateAppProfileAsync(request));
+    return ApiExceptions.callAndTranslateApiException(updateAppProfileAsync(request));
   }
 
   /**
@@ -966,7 +966,7 @@ public AppProfile apply(com.google.bigtable.admin.v2.AppProfile proto) {
    */
   @SuppressWarnings("WeakerAccess")
   public void deleteAppProfile(String instanceId, String appProfileId) {
-    awaitFuture(deleteAppProfileAsync(instanceId, appProfileId));
+    ApiExceptions.callAndTranslateApiException(deleteAppProfileAsync(instanceId, appProfileId));
   }
 
   /**
@@ -1016,7 +1016,7 @@ public Void apply(Empty input) {
    */
   @SuppressWarnings("WeakerAccess")
   public Policy getIamPolicy(String instanceId) {
-    return awaitFuture(getIamPolicyAsync(instanceId));
+    return ApiExceptions.callAndTranslateApiException(getIamPolicyAsync(instanceId));
   }
 
   /**
@@ -1083,7 +1083,7 @@ public Policy apply(com.google.iam.v1.Policy proto) {
    */
   @SuppressWarnings("WeakerAccess")
   public Policy setIamPolicy(String instanceId, Policy policy) {
-    return awaitFuture(setIamPolicyAsync(instanceId, policy));
+    return ApiExceptions.callAndTranslateApiException(setIamPolicyAsync(instanceId, policy));
   }
 
   /**
@@ -1210,7 +1210,8 @@ public ApiFuture<List<String>> testIamPermissionAsync(String instanceId, String.
    */
   @SuppressWarnings("WeakerAccess")
   public List<String> testIamPermission(ResourceName resourceName, String... permissions) {
-    return awaitFuture(testIamPermissionAsync(resourceName, permissions));
+    return ApiExceptions.callAndTranslateApiException(
+        testIamPermissionAsync(resourceName, permissions));
   }
 
 
@@ -1276,29 +1277,4 @@ public com.google.iam.v1.Policy toPb(Policy policy) {
       return super.toPb(policy);
     }
   }
-
-  /**
-   * Awaits the result of a future, taking care to propagate errors while maintaining the call site
-   * in a suppressed exception. This allows semantic errors to be caught across threads, while
-   * preserving the call site in the error. The caller's stacktrace will be made available as a
-   * suppressed exception.
-   */
-  // TODO(igorbernstein2): try to move this into gax
-  private <T> T awaitFuture(ApiFuture<T> future) {
-    RuntimeException error;
-    try {
-      return Futures.getUnchecked(future);
-    } catch (UncheckedExecutionException e) {
-      if (e.getCause() instanceof RuntimeException) {
-        error = (RuntimeException) e.getCause();
-      } else {
-        error = e;
-      }
-    } catch (RuntimeException e) {
-      error = e;
-    }
-    // Add the caller's stack as a suppressed exception
-    error.addSuppressed(new RuntimeException("Encountered error while awaiting future"));
-    throw error;
-  }
 }

google-cloud-clients/google-cloud-bigtable-admin/src/main/java/com/google/cloud/bigtable/admin/v2/BigtableTableAdminClient.java

@@ -19,6 +19,7 @@
 import com.google.api.core.ApiFunction;
 import com.google.api.core.ApiFuture;
 import com.google.api.core.ApiFutures;
+import com.google.api.gax.rpc.ApiExceptions;
 import com.google.bigtable.admin.v2.DeleteTableRequest;
 import com.google.bigtable.admin.v2.DropRowRangeRequest;
 import com.google.bigtable.admin.v2.GetTableRequest;
@@ -33,9 +34,7 @@
 import com.google.cloud.bigtable.admin.v2.stub.EnhancedBigtableTableAdminStub;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.Lists;
-import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.MoreExecutors;
-import com.google.common.util.concurrent.UncheckedExecutionException;
 import com.google.protobuf.ByteString;
 import com.google.protobuf.Empty;
 import java.io.IOException;
@@ -149,7 +148,7 @@ public void close() {
    */
   @SuppressWarnings("WeakerAccess")
   public Table createTable(CreateTableRequest request) {
-    return awaitFuture(createTableAsync(request));
+    return ApiExceptions.callAndTranslateApiException(createTableAsync(request));
   }
 
   /**
@@ -222,7 +221,7 @@ public ApiFuture<Table> createTableAsync(CreateTableRequest request) {
    */
   @SuppressWarnings("WeakerAccess")
   public Table modifyFamilies(ModifyColumnFamiliesRequest request) {
-    return awaitFuture(modifyFamiliesAsync(request));
+    return ApiExceptions.callAndTranslateApiException(modifyFamiliesAsync(request));
   }
 
   /**
@@ -289,7 +288,7 @@ public ApiFuture<Table> modifyFamiliesAsync(ModifyColumnFamiliesRequest request)
    */
   @SuppressWarnings("WeakerAccess")
   public void deleteTable(String tableId) {
-    awaitFuture(deleteTableAsync(tableId));
+    ApiExceptions.callAndTranslateApiException(deleteTableAsync(tableId));
   }
 
   /**
@@ -342,7 +341,7 @@ public ApiFuture<Void> deleteTableAsync(String tableId) {
    */
   @SuppressWarnings("WeakerAccess")
   public Table getTable(String tableId) {
-    return awaitFuture(getTableAsync(tableId));
+    return ApiExceptions.callAndTranslateApiException(getTableAsync(tableId));
   }
 
   /**
@@ -397,7 +396,7 @@ public ApiFuture<Table> getTableAsync(String tableId) {
   // TODO(igorbernstein2): consider changing this method to use relative table ids.
   @SuppressWarnings("WeakerAccess")
   public List<TableName> listTables() {
-    return awaitFuture(listTablesAsync());
+    return ApiExceptions.callAndTranslateApiException(listTablesAsync());
   }
 
   /**
@@ -506,7 +505,7 @@ public List<TableName> apply(List<com.google.bigtable.admin.v2.Table> protos) {
    */
   @SuppressWarnings("WeakerAccess")
   public void dropRowRange(String tableId, String rowKeyPrefix) {
-    awaitFuture(dropRowRangeAsync(tableId, rowKeyPrefix));
+    ApiExceptions.callAndTranslateApiException(dropRowRangeAsync(tableId, rowKeyPrefix));
   }
 
   /**
@@ -552,7 +551,7 @@ public ApiFuture<Void> dropRowRangeAsync(String tableId, String rowKeyPrefix) {
    */
   @SuppressWarnings("WeakerAccess")
   public void dropRowRange(String tableId, ByteString rowKeyPrefix) {
-    awaitFuture(dropRowRangeAsync(tableId, rowKeyPrefix));
+    ApiExceptions.callAndTranslateApiException(dropRowRangeAsync(tableId, rowKeyPrefix));
   }
 
   /**
@@ -604,7 +603,7 @@ public ApiFuture<Void> dropRowRangeAsync(String tableId, ByteString rowKeyPrefix
    */
   @SuppressWarnings("WeakerAccess")
   public void dropAllRows(String tableId) {
-    awaitFuture(dropAllRowsAsync(tableId));
+    ApiExceptions.callAndTranslateApiException(dropAllRowsAsync(tableId));
   }
 
   /**
@@ -659,7 +658,8 @@ public ApiFuture<Void> dropAllRowsAsync(String tableId) {
   public void awaitReplication(String tableId) {
     TableName tableName = TableName
         .of(instanceName.getProject(), instanceName.getInstance(), tableId);
-    awaitFuture(stub.awaitReplicationCallable().futureCall(tableName));
+    ApiExceptions
+        .callAndTranslateApiException(stub.awaitReplicationCallable().futureCall(tableName));
   }
 
   /**
@@ -734,29 +734,4 @@ public Void apply(Empty empty) {
         },
         MoreExecutors.directExecutor());
   }
-
-  /**
-   * Awaits the result of a future, taking care to propagate errors while maintaining the call site
-   * in a suppressed exception. This allows semantic errors to be caught across threads, while
-   * preserving the call site in the error. The caller's stacktrace will be made available as a
-   * suppressed exception.
-   */
-  // TODO(igorbernstein2): try to move this into gax
-  private <T> T awaitFuture(ApiFuture<T> future) {
-    RuntimeException error;
-    try {
-      return Futures.getUnchecked(future);
-    } catch (UncheckedExecutionException e) {
-      if (e.getCause() instanceof RuntimeException) {
-        error = (RuntimeException) e.getCause();
-      } else {
-        error = e;
-      }
-    } catch (RuntimeException e) {
-      error = e;
-    }
-    // Add the caller's stack as a suppressed exception
-    error.addSuppressed(new RuntimeException("Encountered error while awaiting future"));
-    throw error;
-  }
 }