Skip to content

Commit 5ddfcf9

Browse files
chore(spanner): extract ErrorDetails from grpc-status-details-bin for raw gRPC exceptions
1 parent 5959c02 commit 5ddfcf9

5 files changed

Lines changed: 66 additions & 75 deletions

File tree

java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerException.java

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,10 +137,21 @@ enum DoNotConstructDirectly {
137137
* retry delay.
138138
*/
139139
public long getRetryDelayInMillis() {
140-
return extractRetryDelay(this.getCause());
140+
return extractRetryDelay(this, this.apiException);
141141
}
142142

143143
static long extractRetryDelay(Throwable cause) {
144+
return extractRetryDelay(cause, null);
145+
}
146+
147+
static long extractRetryDelay(Throwable cause, @Nullable ApiException apiException) {
148+
ErrorDetails details = SpannerExceptionFactory.extractErrorDetails(cause, apiException);
149+
if (details != null && details.getRetryInfo() != null) {
150+
RetryInfo retryInfo = details.getRetryInfo();
151+
if (retryInfo.hasRetryDelay()) {
152+
return Durations.toMillis(retryInfo.getRetryDelay());
153+
}
154+
}
144155
if (cause != null) {
145156
Metadata trailers = Status.trailersFromThrowable(cause);
146157
if (trailers != null && trailers.containsKey(KEY_RETRY_INFO)) {
@@ -227,7 +238,7 @@ public ErrorDetails getErrorDetails() {
227238
if (this.apiException != null) {
228239
return this.apiException.getErrorDetails();
229240
}
230-
return null;
241+
return SpannerExceptionFactory.extractErrorDetails(getCause(), null);
231242
}
232243

233244
void setStatement(String statement) {

java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerExceptionFactory.java

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import com.google.cloud.spanner.SpannerException.DoNotConstructDirectly;
2727
import com.google.common.base.MoreObjects;
2828
import com.google.common.base.Predicate;
29+
import com.google.protobuf.InvalidProtocolBufferException;
2930
import com.google.rpc.ErrorInfo;
3031
import com.google.rpc.ResourceInfo;
3132
import com.google.rpc.RetryInfo;
@@ -56,6 +57,8 @@ public final class SpannerExceptionFactory {
5657
ProtoUtils.keyForProto(ResourceInfo.getDefaultInstance());
5758
private static final Metadata.Key<ErrorInfo> KEY_ERROR_INFO =
5859
ProtoUtils.keyForProto(ErrorInfo.getDefaultInstance());
60+
private static final Metadata.Key<byte[]> KEY_ERROR_DETAILS =
61+
Metadata.Key.of("grpc-status-details-bin", Metadata.BINARY_BYTE_MARSHALLER);
5962

6063
public static SpannerException newSpannerException(ErrorCode code, @Nullable String message) {
6164
return newSpannerException(code, message, null);
@@ -247,6 +250,10 @@ private static String formatMessage(ErrorCode code, @Nullable String message) {
247250
}
248251

249252
private static ResourceInfo extractResourceInfo(Throwable cause) {
253+
ErrorDetails details = extractErrorDetails(cause, null);
254+
if (details != null && details.getResourceInfo() != null) {
255+
return details.getResourceInfo();
256+
}
250257
if (cause != null) {
251258
Metadata trailers = Status.trailersFromThrowable(cause);
252259
if (trailers != null) {
@@ -257,8 +264,9 @@ private static ResourceInfo extractResourceInfo(Throwable cause) {
257264
}
258265

259266
private static ErrorInfo extractErrorInfo(Throwable cause, ApiException apiException) {
260-
if (apiException != null && apiException.getErrorDetails() != null) {
261-
return apiException.getErrorDetails().getErrorInfo();
267+
ErrorDetails details = extractErrorDetails(cause, apiException);
268+
if (details != null && details.getErrorInfo() != null) {
269+
return details.getErrorInfo();
262270
}
263271
if (cause != null) {
264272
Metadata trailers = Status.trailersFromThrowable(cause);
@@ -277,10 +285,26 @@ static ErrorDetails extractErrorDetails(Throwable cause, ApiException apiExcepti
277285
Throwable prevCause = null;
278286
while (cause != null && cause != prevCause) {
279287
if (cause instanceof ApiException) {
280-
return ((ApiException) cause).getErrorDetails();
288+
if (((ApiException) cause).getErrorDetails() != null) {
289+
return ((ApiException) cause).getErrorDetails();
290+
}
281291
}
282292
if (cause instanceof SpannerException) {
283-
return ((SpannerException) cause).getErrorDetails();
293+
if (((SpannerException) cause).getErrorDetails() != null) {
294+
return ((SpannerException) cause).getErrorDetails();
295+
}
296+
}
297+
Metadata trailers = Status.trailersFromThrowable(cause);
298+
if (trailers != null) {
299+
byte[] bytes = trailers.get(KEY_ERROR_DETAILS);
300+
if (bytes != null) {
301+
try {
302+
com.google.rpc.Status status = com.google.rpc.Status.parseFrom(bytes);
303+
return ErrorDetails.builder().setRawErrorMessages(status.getDetailsList()).build();
304+
} catch (InvalidProtocolBufferException e) {
305+
// ignore and continue
306+
}
307+
}
284308
}
285309
prevCause = cause;
286310
cause = cause.getCause();

java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/ITTransactionRetryTest.java

Lines changed: 0 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,7 @@ public class ITTransactionRetryTest {
3939
@Test
4040
public void TestRetryInfo() {
4141
assumeFalse("emulator does not support parallel transaction", isUsingEmulator());
42-
// TODO(sakthivelmani) - Re-enable once b/422916293 is resolved
43-
assumeFalse(
44-
"Skipping the test due to a known bug b/422916293",
45-
env.getTestHelper().getOptions().isEnableDirectAccess());
4642
assumeFalse("Skipping the test due to a known bug b/422916293", isSpannerOmni());
47-
4843
// Creating a database with the table which contains INT64 columns
4944
Database db =
5045
env.getTestHelper()
@@ -88,56 +83,4 @@ public void TestRetryInfo() {
8883

8984
assertTrue("Transaction is not aborted with the trailers", isAbortedWithRetryInfo);
9085
}
91-
92-
@Test
93-
public void TestRetryInfoWithDirectPath() {
94-
assumeFalse("emulator does not support parallel transaction", isUsingEmulator());
95-
// TODO(sakthivelmani) - Re-enable once b/422916293 is resolved
96-
assumeTrue(
97-
"Enabling this test due to bug b/422916293",
98-
env.getTestHelper().getOptions().isEnableDirectAccess());
99-
100-
// Creating a database with the table which contains INT64 columns
101-
Database db =
102-
env.getTestHelper()
103-
.createTestDatabase("CREATE TABLE Test(ID INT64, " + "EMPID INT64) PRIMARY KEY (ID)");
104-
DatabaseClient databaseClient = env.getTestHelper().getClient().getDatabaseClient(db.getId());
105-
106-
// Inserting one row
107-
databaseClient
108-
.readWriteTransaction()
109-
.run(
110-
transaction -> {
111-
transaction.buffer(
112-
Mutation.newInsertBuilder("Test").set("ID").to(1).set("EMPID").to(1).build());
113-
return null;
114-
});
115-
116-
int numRetries = 10;
117-
boolean isAbortedWithRetryInfo = false;
118-
while (numRetries-- > 0) {
119-
try (TransactionManager transactionManager1 = databaseClient.transactionManager()) {
120-
try (TransactionManager transactionManager2 = databaseClient.transactionManager()) {
121-
try {
122-
TransactionContext transaction1 = transactionManager1.begin();
123-
TransactionContext transaction2 = transactionManager2.begin();
124-
transaction1.executeUpdate(
125-
Statement.of("UPDATE Test SET EMPID = EMPID + 1 WHERE ID = 1"));
126-
transaction2.executeUpdate(
127-
Statement.of("UPDATE Test SET EMPID = EMPID + 1 WHERE ID = 1"));
128-
transactionManager1.commit();
129-
transactionManager2.commit();
130-
} catch (AbortedException abortedException) {
131-
assertThat(abortedException.getErrorCode()).isEqualTo(ErrorCode.ABORTED);
132-
if (abortedException.getRetryDelayInMillis() > 0) {
133-
isAbortedWithRetryInfo = true;
134-
break;
135-
}
136-
}
137-
}
138-
}
139-
}
140-
141-
assertFalse("Transaction is aborted with the trailers", isAbortedWithRetryInfo);
142-
}
14386
}

java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerExceptionFactoryTest.java

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,4 +251,29 @@ private Metadata createResourceTypeMetadata(String resourceType, String resource
251251

252252
return trailers;
253253
}
254+
255+
@Test
256+
public void resourceExhaustedWithGrpcStatusDetails() {
257+
Status status =
258+
Status.fromCodeValue(Status.Code.RESOURCE_EXHAUSTED.value())
259+
.withDescription("Memory pushback");
260+
Metadata trailers = new Metadata();
261+
Metadata.Key<byte[]> key =
262+
Metadata.Key.of("grpc-status-details-bin", Metadata.BINARY_BYTE_MARSHALLER);
263+
RetryInfo retryInfo =
264+
RetryInfo.newBuilder()
265+
.setRetryDelay(Duration.newBuilder().setNanos(1000000).setSeconds(1L))
266+
.build();
267+
com.google.rpc.Status rpcStatus =
268+
com.google.rpc.Status.newBuilder()
269+
.setCode(com.google.rpc.Code.RESOURCE_EXHAUSTED_VALUE)
270+
.setMessage("Memory pushback")
271+
.addDetails(com.google.protobuf.Any.pack(retryInfo))
272+
.build();
273+
trailers.put(key, rpcStatus.toByteArray());
274+
SpannerException e =
275+
SpannerExceptionFactory.newSpannerException(new StatusRuntimeException(status, trailers));
276+
assertThat(e.isRetryable()).isTrue();
277+
assertThat(e.getRetryDelayInMillis()).isEqualTo(1001);
278+
}
254279
}

java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/it/ITRetryDmlAsPartitionedDmlTest.java

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -88,10 +88,6 @@ public static void setupTestData() {
8888

8989
@Test
9090
public void testDmlFailsIfMutationLimitExceeded() {
91-
// TODO(sakthivelmani) - Re-enable once b/422916293 is resolved
92-
assumeFalse(
93-
"Skipping the test due to a known bug b/422916293",
94-
env.getTestHelper().getOptions().isEnableDirectAccess());
9591
try (Connection connection = createConnection()) {
9692
connection.setAutocommit(true);
9793
assertThrows(
@@ -104,10 +100,6 @@ public void testDmlFailsIfMutationLimitExceeded() {
104100

105101
@Test
106102
public void testRetryDmlAsPartitionedDml() throws Exception {
107-
// TODO(sakthivelmani) - Re-enable once b/422916293 is resolved
108-
assumeFalse(
109-
"Skipping the test due to a known bug b/422916293",
110-
env.getTestHelper().getOptions().isEnableDirectAccess());
111103
try (Connection connection = createConnection()) {
112104
connection.setAutocommit(true);
113105
connection.setAutocommitDmlMode(
@@ -148,10 +140,6 @@ public void retryDmlAsPartitionedDmlFinished(
148140

149141
@Test
150142
public void testRetryDmlAsPartitionedDml_failsForLargeInserts() throws Exception {
151-
// TODO(sakthivelmani) - Re-enable once b/422916293 is resolved
152-
assumeFalse(
153-
"Skipping the test due to a known bug b/422916293",
154-
env.getTestHelper().getOptions().isEnableDirectAccess());
155143
try (Connection connection = createConnection()) {
156144
connection.setAutocommit(true);
157145
connection.setAutocommitDmlMode(

0 commit comments

Comments
 (0)