From cabab0f7eb4b0a9525e1117708fdd4bfa9e57bf5 Mon Sep 17 00:00:00 2001 From: Sakthivel Subramanian Date: Mon, 1 Jun 2026 14:55:32 +0000 Subject: [PATCH] chore(spanner): extract ErrorDetails from grpc-status-details-bin for raw gRPC exceptions --- .../cloud/spanner/SpannerException.java | 15 ++++- .../spanner/SpannerExceptionFactory.java | 32 ++++++++-- .../cloud/spanner/ITTransactionRetryTest.java | 59 ------------------- .../spanner/SpannerExceptionFactoryTest.java | 25 ++++++++ .../it/ITRetryDmlAsPartitionedDmlTest.java | 12 ---- 5 files changed, 66 insertions(+), 77 deletions(-) diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerException.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerException.java index 0829cc35d62a..d75fa6a735a8 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerException.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerException.java @@ -137,10 +137,21 @@ enum DoNotConstructDirectly { * retry delay. */ public long getRetryDelayInMillis() { - return extractRetryDelay(this.getCause()); + return extractRetryDelay(this, this.apiException); } static long extractRetryDelay(Throwable cause) { + return extractRetryDelay(cause, null); + } + + static long extractRetryDelay(Throwable cause, @Nullable ApiException apiException) { + ErrorDetails details = SpannerExceptionFactory.extractErrorDetails(cause, apiException); + if (details != null && details.getRetryInfo() != null) { + RetryInfo retryInfo = details.getRetryInfo(); + if (retryInfo.hasRetryDelay()) { + return Durations.toMillis(retryInfo.getRetryDelay()); + } + } if (cause != null) { Metadata trailers = Status.trailersFromThrowable(cause); if (trailers != null && trailers.containsKey(KEY_RETRY_INFO)) { @@ -227,7 +238,7 @@ public ErrorDetails getErrorDetails() { if (this.apiException != null) { return this.apiException.getErrorDetails(); } - return null; + return SpannerExceptionFactory.extractErrorDetails(getCause(), null); } void setStatement(String statement) { diff --git a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerExceptionFactory.java b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerExceptionFactory.java index 185f98b54332..fde4478a9502 100644 --- a/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerExceptionFactory.java +++ b/java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerExceptionFactory.java @@ -26,6 +26,7 @@ import com.google.cloud.spanner.SpannerException.DoNotConstructDirectly; import com.google.common.base.MoreObjects; import com.google.common.base.Predicate; +import com.google.protobuf.InvalidProtocolBufferException; import com.google.rpc.ErrorInfo; import com.google.rpc.ResourceInfo; import com.google.rpc.RetryInfo; @@ -56,6 +57,8 @@ public final class SpannerExceptionFactory { ProtoUtils.keyForProto(ResourceInfo.getDefaultInstance()); private static final Metadata.Key KEY_ERROR_INFO = ProtoUtils.keyForProto(ErrorInfo.getDefaultInstance()); + private static final Metadata.Key KEY_ERROR_DETAILS = + Metadata.Key.of("grpc-status-details-bin", Metadata.BINARY_BYTE_MARSHALLER); public static SpannerException newSpannerException(ErrorCode code, @Nullable String message) { return newSpannerException(code, message, null); @@ -247,6 +250,10 @@ private static String formatMessage(ErrorCode code, @Nullable String message) { } private static ResourceInfo extractResourceInfo(Throwable cause) { + ErrorDetails details = extractErrorDetails(cause, null); + if (details != null && details.getResourceInfo() != null) { + return details.getResourceInfo(); + } if (cause != null) { Metadata trailers = Status.trailersFromThrowable(cause); if (trailers != null) { @@ -257,8 +264,9 @@ private static ResourceInfo extractResourceInfo(Throwable cause) { } private static ErrorInfo extractErrorInfo(Throwable cause, ApiException apiException) { - if (apiException != null && apiException.getErrorDetails() != null) { - return apiException.getErrorDetails().getErrorInfo(); + ErrorDetails details = extractErrorDetails(cause, apiException); + if (details != null && details.getErrorInfo() != null) { + return details.getErrorInfo(); } if (cause != null) { Metadata trailers = Status.trailersFromThrowable(cause); @@ -277,10 +285,26 @@ static ErrorDetails extractErrorDetails(Throwable cause, ApiException apiExcepti Throwable prevCause = null; while (cause != null && cause != prevCause) { if (cause instanceof ApiException) { - return ((ApiException) cause).getErrorDetails(); + if (((ApiException) cause).getErrorDetails() != null) { + return ((ApiException) cause).getErrorDetails(); + } } if (cause instanceof SpannerException) { - return ((SpannerException) cause).getErrorDetails(); + if (((SpannerException) cause).getErrorDetails() != null) { + return ((SpannerException) cause).getErrorDetails(); + } + } + Metadata trailers = Status.trailersFromThrowable(cause); + if (trailers != null) { + byte[] bytes = trailers.get(KEY_ERROR_DETAILS); + if (bytes != null) { + try { + com.google.rpc.Status status = com.google.rpc.Status.parseFrom(bytes); + return ErrorDetails.builder().setRawErrorMessages(status.getDetailsList()).build(); + } catch (InvalidProtocolBufferException e) { + // ignore and continue + } + } } prevCause = cause; cause = cause.getCause(); diff --git a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/ITTransactionRetryTest.java b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/ITTransactionRetryTest.java index 0316b7ea8416..42042957d539 100644 --- a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/ITTransactionRetryTest.java +++ b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/ITTransactionRetryTest.java @@ -19,10 +19,8 @@ import static com.google.cloud.spanner.testing.EmulatorSpannerHelper.isUsingEmulator; import static com.google.cloud.spanner.testing.SpannerOmniHelper.isSpannerOmni; import static com.google.common.truth.Truth.assertThat; -import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.junit.Assume.assumeFalse; -import static org.junit.Assume.assumeTrue; import org.junit.ClassRule; import org.junit.Test; @@ -39,12 +37,7 @@ public class ITTransactionRetryTest { @Test public void TestRetryInfo() { assumeFalse("emulator does not support parallel transaction", isUsingEmulator()); - // TODO(sakthivelmani) - Re-enable once b/422916293 is resolved - assumeFalse( - "Skipping the test due to a known bug b/422916293", - env.getTestHelper().getOptions().isEnableDirectAccess()); assumeFalse("Skipping the test due to a known bug b/422916293", isSpannerOmni()); - // Creating a database with the table which contains INT64 columns Database db = env.getTestHelper() @@ -88,56 +81,4 @@ public void TestRetryInfo() { assertTrue("Transaction is not aborted with the trailers", isAbortedWithRetryInfo); } - - @Test - public void TestRetryInfoWithDirectPath() { - assumeFalse("emulator does not support parallel transaction", isUsingEmulator()); - // TODO(sakthivelmani) - Re-enable once b/422916293 is resolved - assumeTrue( - "Enabling this test due to bug b/422916293", - env.getTestHelper().getOptions().isEnableDirectAccess()); - - // Creating a database with the table which contains INT64 columns - Database db = - env.getTestHelper() - .createTestDatabase("CREATE TABLE Test(ID INT64, " + "EMPID INT64) PRIMARY KEY (ID)"); - DatabaseClient databaseClient = env.getTestHelper().getClient().getDatabaseClient(db.getId()); - - // Inserting one row - databaseClient - .readWriteTransaction() - .run( - transaction -> { - transaction.buffer( - Mutation.newInsertBuilder("Test").set("ID").to(1).set("EMPID").to(1).build()); - return null; - }); - - int numRetries = 10; - boolean isAbortedWithRetryInfo = false; - while (numRetries-- > 0) { - try (TransactionManager transactionManager1 = databaseClient.transactionManager()) { - try (TransactionManager transactionManager2 = databaseClient.transactionManager()) { - try { - TransactionContext transaction1 = transactionManager1.begin(); - TransactionContext transaction2 = transactionManager2.begin(); - transaction1.executeUpdate( - Statement.of("UPDATE Test SET EMPID = EMPID + 1 WHERE ID = 1")); - transaction2.executeUpdate( - Statement.of("UPDATE Test SET EMPID = EMPID + 1 WHERE ID = 1")); - transactionManager1.commit(); - transactionManager2.commit(); - } catch (AbortedException abortedException) { - assertThat(abortedException.getErrorCode()).isEqualTo(ErrorCode.ABORTED); - if (abortedException.getRetryDelayInMillis() > 0) { - isAbortedWithRetryInfo = true; - break; - } - } - } - } - } - - assertFalse("Transaction is aborted with the trailers", isAbortedWithRetryInfo); - } } diff --git a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerExceptionFactoryTest.java b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerExceptionFactoryTest.java index 55b9523d7db0..e7d45cee864c 100644 --- a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerExceptionFactoryTest.java +++ b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerExceptionFactoryTest.java @@ -251,4 +251,29 @@ private Metadata createResourceTypeMetadata(String resourceType, String resource return trailers; } + + @Test + public void resourceExhaustedWithGrpcStatusDetails() { + Status status = + Status.fromCodeValue(Status.Code.RESOURCE_EXHAUSTED.value()) + .withDescription("Memory pushback"); + Metadata trailers = new Metadata(); + Metadata.Key key = + Metadata.Key.of("grpc-status-details-bin", Metadata.BINARY_BYTE_MARSHALLER); + RetryInfo retryInfo = + RetryInfo.newBuilder() + .setRetryDelay(Duration.newBuilder().setNanos(1000000).setSeconds(1L)) + .build(); + com.google.rpc.Status rpcStatus = + com.google.rpc.Status.newBuilder() + .setCode(com.google.rpc.Code.RESOURCE_EXHAUSTED_VALUE) + .setMessage("Memory pushback") + .addDetails(com.google.protobuf.Any.pack(retryInfo)) + .build(); + trailers.put(key, rpcStatus.toByteArray()); + SpannerException e = + SpannerExceptionFactory.newSpannerException(new StatusRuntimeException(status, trailers)); + assertThat(e.isRetryable()).isTrue(); + assertThat(e.getRetryDelayInMillis()).isEqualTo(1001); + } } diff --git a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/it/ITRetryDmlAsPartitionedDmlTest.java b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/it/ITRetryDmlAsPartitionedDmlTest.java index 5437218a20cf..8f4441db11f5 100644 --- a/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/it/ITRetryDmlAsPartitionedDmlTest.java +++ b/java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/it/ITRetryDmlAsPartitionedDmlTest.java @@ -88,10 +88,6 @@ public static void setupTestData() { @Test public void testDmlFailsIfMutationLimitExceeded() { - // TODO(sakthivelmani) - Re-enable once b/422916293 is resolved - assumeFalse( - "Skipping the test due to a known bug b/422916293", - env.getTestHelper().getOptions().isEnableDirectAccess()); try (Connection connection = createConnection()) { connection.setAutocommit(true); assertThrows( @@ -104,10 +100,6 @@ public void testDmlFailsIfMutationLimitExceeded() { @Test public void testRetryDmlAsPartitionedDml() throws Exception { - // TODO(sakthivelmani) - Re-enable once b/422916293 is resolved - assumeFalse( - "Skipping the test due to a known bug b/422916293", - env.getTestHelper().getOptions().isEnableDirectAccess()); try (Connection connection = createConnection()) { connection.setAutocommit(true); connection.setAutocommitDmlMode( @@ -148,10 +140,6 @@ public void retryDmlAsPartitionedDmlFinished( @Test public void testRetryDmlAsPartitionedDml_failsForLargeInserts() throws Exception { - // TODO(sakthivelmani) - Re-enable once b/422916293 is resolved - assumeFalse( - "Skipping the test due to a known bug b/422916293", - env.getTestHelper().getOptions().isEnableDirectAccess()); try (Connection connection = createConnection()) { connection.setAutocommit(true); connection.setAutocommitDmlMode(