From ae29724ac1fbedeba1d264d47d62cb876f68ecb6 Mon Sep 17 00:00:00 2001 From: Emmanuel T Odeke Date: Wed, 9 Apr 2025 18:59:07 +0300 Subject: [PATCH 1/2] chore: add x-goog-request-id insertion into *Exception Allows users to examine and report the requestId in any thrown exceptions. Updates #3537 --- .../cloud/spanner/AbortedException.java | 7 +- ...minRequestsPerMinuteExceededException.java | 7 +- .../spanner/DatabaseNotFoundException.java | 7 +- .../spanner/InstanceNotFoundException.java | 7 +- .../MissingDefaultSequenceKindException.java | 5 +- .../com/google/cloud/spanner/Operation.java | 2 +- .../spanner/ResumableStreamIterator.java | 4 +- .../spanner/SessionNotFoundException.java | 7 +- .../cloud/spanner/SpannerApiFutures.java | 5 +- .../spanner/SpannerBatchUpdateException.java | 5 +- .../cloud/spanner/SpannerException.java | 26 ++++- .../spanner/SpannerExceptionFactory.java | 98 +++++++++++++------ .../cloud/spanner/SpannerRetryHelper.java | 3 +- ...sactionMutationLimitExceededException.java | 5 +- .../cloud/spanner/TransactionRunnerImpl.java | 9 +- .../cloud/spanner/connection/DdlBatch.java | 2 +- .../cloud/spanner/spi/v1/GapicSpannerRpc.java | 2 +- .../spanner/SpannerExceptionFactoryTest.java | 13 +++ 18 files changed, 152 insertions(+), 62 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbortedException.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbortedException.java index 03dff9f7609..3e5227888d9 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbortedException.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbortedException.java @@ -35,7 +35,7 @@ public class AbortedException extends SpannerException { /** Private constructor. Use {@link SpannerExceptionFactory} to create instances. */ AbortedException( DoNotConstructDirectly token, @Nullable String message, @Nullable Throwable cause) { - this(token, message, cause, null); + this(token, message, cause, null, null); } /** Private constructor. Use {@link SpannerExceptionFactory} to create instances. */ @@ -43,8 +43,9 @@ public class AbortedException extends SpannerException { DoNotConstructDirectly token, @Nullable String message, @Nullable Throwable cause, - @Nullable ApiException apiException) { - super(token, ErrorCode.ABORTED, IS_RETRYABLE, message, cause, apiException); + @Nullable ApiException apiException, + @Nullable XGoogSpannerRequestId reqId) { + super(token, ErrorCode.ABORTED, IS_RETRYABLE, message, cause, apiException, reqId); } /** diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AdminRequestsPerMinuteExceededException.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AdminRequestsPerMinuteExceededException.java index 72d8b0ab15d..b7d76ab9e82 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AdminRequestsPerMinuteExceededException.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AdminRequestsPerMinuteExceededException.java @@ -32,7 +32,7 @@ public class AdminRequestsPerMinuteExceededException extends SpannerException { /** Private constructor. Use {@link SpannerExceptionFactory} to create instances. */ AdminRequestsPerMinuteExceededException( DoNotConstructDirectly token, @Nullable String message, @Nullable Throwable cause) { - this(token, message, cause, null); + this(token, message, cause, null, null); } /** Private constructor. Use {@link SpannerExceptionFactory} to create instances. */ @@ -40,7 +40,8 @@ public class AdminRequestsPerMinuteExceededException extends SpannerException { DoNotConstructDirectly token, @Nullable String message, @Nullable Throwable cause, - @Nullable ApiException apiException) { - super(token, ErrorCode.RESOURCE_EXHAUSTED, true, message, cause, apiException); + @Nullable ApiException apiException, + @Nullable XGoogSpannerRequestId reqId) { + super(token, ErrorCode.RESOURCE_EXHAUSTED, true, message, cause, apiException, reqId); } } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseNotFoundException.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseNotFoundException.java index cc4a2e32f0b..474acb04890 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseNotFoundException.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseNotFoundException.java @@ -35,7 +35,7 @@ public class DatabaseNotFoundException extends ResourceNotFoundException { @Nullable String message, ResourceInfo resourceInfo, @Nullable Throwable cause) { - this(token, message, resourceInfo, cause, null); + this(token, message, resourceInfo, cause, null, null); } /** Private constructor. Use {@link SpannerExceptionFactory} to create instances. */ @@ -44,7 +44,8 @@ public class DatabaseNotFoundException extends ResourceNotFoundException { @Nullable String message, ResourceInfo resourceInfo, @Nullable Throwable cause, - @Nullable ApiException apiException) { - super(token, message, resourceInfo, cause, apiException); + @Nullable ApiException apiException, + @Nullable XGoogSpannerRequestId reqId) { + super(token, message, resourceInfo, cause, apiException, reqId); } } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceNotFoundException.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceNotFoundException.java index 82c451f9475..18961569bbf 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceNotFoundException.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceNotFoundException.java @@ -35,7 +35,7 @@ public class InstanceNotFoundException extends ResourceNotFoundException { @Nullable String message, ResourceInfo resourceInfo, @Nullable Throwable cause) { - this(token, message, resourceInfo, cause, null); + this(token, message, resourceInfo, cause, null, null); } /** Private constructor. Use {@link SpannerExceptionFactory} to create instances. */ InstanceNotFoundException( @@ -43,7 +43,8 @@ public class InstanceNotFoundException extends ResourceNotFoundException { @Nullable String message, ResourceInfo resourceInfo, @Nullable Throwable cause, - @Nullable ApiException apiException) { - super(token, message, resourceInfo, cause, apiException); + @Nullable ApiException apiException, + @Nullable XGoogSpannerRequestId reqId) { + super(token, message, resourceInfo, cause, apiException, reqId); } } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/MissingDefaultSequenceKindException.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/MissingDefaultSequenceKindException.java index 7b61dbbd881..5ed594e10cd 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/MissingDefaultSequenceKindException.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/MissingDefaultSequenceKindException.java @@ -37,8 +37,9 @@ public class MissingDefaultSequenceKindException extends SpannerException { ErrorCode errorCode, String message, Throwable cause, - @Nullable ApiException apiException) { - super(token, errorCode, /*retryable = */ false, message, cause, apiException); + @Nullable ApiException apiException, + @Nullable XGoogSpannerRequestId reqId) { + super(token, errorCode, /*retryable = */ false, message, cause, apiException, reqId); } static boolean isMissingDefaultSequenceKindException(Throwable cause) { diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Operation.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Operation.java index 66b1165f4a9..8cfbcedc175 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Operation.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Operation.java @@ -89,7 +89,7 @@ private static Operation failed( SpannerRpc rpc, String name, Status status, M metadata, Parser parser, ApiClock clock) { SpannerException e = SpannerExceptionFactory.newSpannerException( - ErrorCode.fromRpcStatus(status), status.getMessage(), null); + ErrorCode.fromRpcStatus(status), status.getMessage(), (Throwable) (null)); return new Operation<>(rpc, name, metadata, null, e, true, parser, clock); } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/ResumableStreamIterator.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/ResumableStreamIterator.java index 793f3bcbe32..25cc30a2814 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/ResumableStreamIterator.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/ResumableStreamIterator.java @@ -180,10 +180,10 @@ private void backoffSleep(Context context, long backoffMillis) throws SpannerExc } if (latch.await(backoffMillis, TimeUnit.MILLISECONDS)) { // Woken by context cancellation. - throw newSpannerExceptionForCancellation(context, null); + throw newSpannerExceptionForCancellation(context, null, null /*TODO: requestId*/); } } catch (InterruptedException interruptExcept) { - throw newSpannerExceptionForCancellation(context, interruptExcept); + throw newSpannerExceptionForCancellation(context, interruptExcept, null /*TODO: requestId*/); } finally { context.removeListener(listener); } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionNotFoundException.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionNotFoundException.java index f4a62b1954a..c17384db3ec 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionNotFoundException.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionNotFoundException.java @@ -35,7 +35,7 @@ public class SessionNotFoundException extends ResourceNotFoundException { @Nullable String message, ResourceInfo resourceInfo, @Nullable Throwable cause) { - this(token, message, resourceInfo, cause, null); + this(token, message, resourceInfo, cause, null, null); } /** Private constructor. Use {@link SpannerExceptionFactory} to create instances. */ @@ -44,7 +44,8 @@ public class SessionNotFoundException extends ResourceNotFoundException { @Nullable String message, ResourceInfo resourceInfo, @Nullable Throwable cause, - @Nullable ApiException apiException) { - super(token, message, resourceInfo, cause, apiException); + @Nullable ApiException apiException, + @Nullable XGoogSpannerRequestId reqId) { + super(token, message, resourceInfo, cause, apiException, reqId); } } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerApiFutures.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerApiFutures.java index 39afc1b81a4..fb55378aa56 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerApiFutures.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerApiFutures.java @@ -35,9 +35,10 @@ public static T getOrNull(ApiFuture future) throws SpannerException { } throw SpannerExceptionFactory.newSpannerException(e.getCause()); } catch (InterruptedException e) { - throw SpannerExceptionFactory.propagateInterrupt(e); + throw SpannerExceptionFactory.propagateInterrupt(e, null /*TODO: requestId*/); } catch (CancellationException e) { - throw SpannerExceptionFactory.newSpannerExceptionForCancellation(null, e); + throw SpannerExceptionFactory.newSpannerExceptionForCancellation( + null, e, null /*TODO: requestId*/); } } } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerBatchUpdateException.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerBatchUpdateException.java index b5d0d2a6871..0d841d24463 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerBatchUpdateException.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerBatchUpdateException.java @@ -25,8 +25,9 @@ public class SpannerBatchUpdateException extends SpannerException { ErrorCode code, String message, long[] counts, - Throwable cause) { - super(token, code, false, message, cause); + Throwable cause, + XGoogSpannerRequestId reqId) { + super(token, code, false, message, cause, null, reqId); updateCounts = counts; } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerException.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerException.java index 58076570c20..22a5270cef7 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerException.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerException.java @@ -40,8 +40,9 @@ public abstract static class ResourceNotFoundException extends SpannerException @Nullable String message, ResourceInfo resourceInfo, @Nullable Throwable cause, - @Nullable ApiException apiException) { - super(token, ErrorCode.NOT_FOUND, /* retryable */ false, message, cause, apiException); + @Nullable ApiException apiException, + @Nullable XGoogSpannerRequestId reqId) { + super(token, ErrorCode.NOT_FOUND, /* retryable */ false, message, cause, apiException, reqId); this.resourceInfo = resourceInfo; } @@ -56,6 +57,7 @@ public String getResourceName() { private final ErrorCode code; private final ApiException apiException; + private final XGoogSpannerRequestId requestId; /** Private constructor. Use {@link SpannerExceptionFactory} to create instances. */ SpannerException( @@ -75,12 +77,25 @@ public String getResourceName() { @Nullable String message, @Nullable Throwable cause, @Nullable ApiException apiException) { + this(token, code, retryable, message, cause, apiException, null); + } + + /** Private constructor. Use {@link SpannerExceptionFactory} to create instances. */ + SpannerException( + DoNotConstructDirectly token, + ErrorCode code, + boolean retryable, + @Nullable String message, + @Nullable Throwable cause, + @Nullable ApiException apiException, + @Nullable XGoogSpannerRequestId requestId) { super(message, cause, code.getCode(), retryable); if (token != DoNotConstructDirectly.ALLOWED) { throw new AssertionError("Do not construct directly: use SpannerExceptionFactory"); } this.code = Preconditions.checkNotNull(code); this.apiException = apiException; + this.requestId = requestId; } /** Returns the error code associated with this exception. */ @@ -88,6 +103,13 @@ public ErrorCode getErrorCode() { return code; } + public String getRequestId() { + if (requestId == null) { + return ""; + } + return requestId.toString(); + } + enum DoNotConstructDirectly { ALLOWED } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerExceptionFactory.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerExceptionFactory.java index 3e88f06543c..e87e2927452 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerExceptionFactory.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerExceptionFactory.java @@ -58,17 +58,37 @@ public final class SpannerExceptionFactory { ProtoUtils.keyForProto(ErrorInfo.getDefaultInstance()); public static SpannerException newSpannerException(ErrorCode code, @Nullable String message) { - return newSpannerException(code, message, null); + return newSpannerException(code, message, (XGoogSpannerRequestId) (null)); + } + + public static SpannerException newSpannerException( + ErrorCode code, + @Nullable String message, + @Nullable Throwable cause, + @Nullable XGoogSpannerRequestId reqId) { + return newSpannerExceptionPreformatted( + code, formatMessage(code, message), cause, (ApiException) (null), reqId); + } + + public static SpannerException newSpannerException( + ErrorCode code, @Nullable String message, @Nullable XGoogSpannerRequestId reqId) { + return newSpannerException(code, message, (Throwable) (null), reqId); } public static SpannerException newSpannerException( ErrorCode code, @Nullable String message, @Nullable Throwable cause) { - return newSpannerExceptionPreformatted(code, formatMessage(code, message), cause); + return newSpannerException(code, message, cause, null); } public static SpannerException propagateInterrupt(InterruptedException e) { + return propagateInterrupt(e, null); + } + + public static SpannerException propagateInterrupt( + InterruptedException e, XGoogSpannerRequestId reqId) { Thread.currentThread().interrupt(); - return SpannerExceptionFactory.newSpannerException(ErrorCode.CANCELLED, "Interrupted", e); + return SpannerExceptionFactory.newSpannerException( + ErrorCode.CANCELLED, "Interrupted", e, reqId); } /** @@ -112,17 +132,22 @@ public static SpannerException asSpannerException(Throwable t) { * #newSpannerException(ErrorCode, String)} instead of this method. */ public static SpannerException newSpannerException(Throwable cause) { - return newSpannerException(null, cause); + return newSpannerException(null, cause, null); + } + + public static SpannerException newSpannerException(Throwable cause, XGoogSpannerRequestId reqId) { + return newSpannerException(null, cause, reqId); } public static SpannerBatchUpdateException newSpannerBatchUpdateException( - ErrorCode code, String message, long[] updateCounts) { + ErrorCode code, String message, long[] updateCounts, @Nullable XGoogSpannerRequestId reqId) { DoNotConstructDirectly token = DoNotConstructDirectly.ALLOWED; SpannerException cause = null; if (isTransactionMutationLimitException(code, message)) { - cause = new TransactionMutationLimitExceededException(token, code, message, null, null); + cause = + new TransactionMutationLimitExceededException(token, code, message, null, null, reqId); } - return new SpannerBatchUpdateException(token, code, message, updateCounts, cause); + return new SpannerBatchUpdateException(token, code, message, updateCounts, cause, reqId); } /** Constructs a specific error that */ @@ -173,6 +198,10 @@ public static SpannerBatchUpdateException newSpannerBatchUpdateException( cause); } + public static SpannerException newSpannerException(@Nullable Context context, Throwable cause) { + return newSpannerException(context, cause, null); + } + /** * Creates a new exception based on {@code cause}. If {@code cause} indicates cancellation, {@code * context} will be inspected to establish the type of cancellation. @@ -180,21 +209,22 @@ public static SpannerBatchUpdateException newSpannerBatchUpdateException( *

Intended for internal library use; user code should use {@link * #newSpannerException(ErrorCode, String)} instead of this method. */ - public static SpannerException newSpannerException(@Nullable Context context, Throwable cause) { + public static SpannerException newSpannerException( + @Nullable Context context, Throwable cause, @Nullable XGoogSpannerRequestId reqId) { if (cause instanceof SpannerException) { SpannerException e = (SpannerException) cause; - return newSpannerExceptionPreformatted(e.getErrorCode(), e.getMessage(), e); + return newSpannerExceptionPreformatted(e.getErrorCode(), e.getMessage(), e, null, reqId); } else if (cause instanceof CancellationException) { - return newSpannerExceptionForCancellation(context, cause); + return newSpannerExceptionForCancellation(context, cause, reqId); } else if (cause instanceof ApiException) { - return fromApiException((ApiException) cause); + return fromApiException((ApiException) cause, reqId); } // Extract gRPC status. This will produce "UNKNOWN" for non-gRPC exceptions. Status status = Status.fromThrowable(cause); if (status.getCode() == Status.Code.CANCELLED) { - return newSpannerExceptionForCancellation(context, cause); + return newSpannerExceptionForCancellation(context, cause, reqId); } - return newSpannerException(ErrorCode.fromGrpcStatus(status), cause.getMessage(), cause); + return newSpannerException(ErrorCode.fromGrpcStatus(status), cause.getMessage(), cause, reqId); } public static RuntimeException causeAsRunTimeException(ExecutionException executionException) { @@ -219,6 +249,11 @@ static SpannerException newRetryOnDifferentGrpcChannelException( static SpannerException newSpannerExceptionForCancellation( @Nullable Context context, @Nullable Throwable cause) { + return newSpannerExceptionForCancellation(context, cause, null); + } + + static SpannerException newSpannerExceptionForCancellation( + @Nullable Context context, @Nullable Throwable cause, @Nullable XGoogSpannerRequestId reqId) { if (context != null && context.isCancelled()) { Throwable cancellationCause = context.cancellationCause(); Throwable throwable = @@ -227,13 +262,14 @@ static SpannerException newSpannerExceptionForCancellation( : MoreObjects.firstNonNull(cause, cancellationCause); if (cancellationCause instanceof TimeoutException) { return newSpannerException( - ErrorCode.DEADLINE_EXCEEDED, "Current context exceeded deadline", throwable); + ErrorCode.DEADLINE_EXCEEDED, "Current context exceeded deadline", throwable, reqId); } else { - return newSpannerException(ErrorCode.CANCELLED, "Current context was cancelled", throwable); + return newSpannerException( + ErrorCode.CANCELLED, "Current context was cancelled", throwable, reqId); } } return newSpannerException( - ErrorCode.CANCELLED, cause == null ? "Cancelled" : cause.getMessage(), cause); + ErrorCode.CANCELLED, cause == null ? "Cancelled" : cause.getMessage(), cause, reqId); } private static String formatMessage(ErrorCode code, @Nullable String message) { @@ -305,12 +341,13 @@ static SpannerException newSpannerExceptionPreformatted( ErrorCode code, @Nullable String message, @Nullable Throwable cause, - @Nullable ApiException apiException) { + @Nullable ApiException apiException, + @Nullable XGoogSpannerRequestId reqId) { // This is the one place in the codebase that is allowed to call constructors directly. DoNotConstructDirectly token = DoNotConstructDirectly.ALLOWED; switch (code) { case ABORTED: - return new AbortedException(token, message, cause, apiException); + return new AbortedException(token, message, cause, apiException, reqId); case RESOURCE_EXHAUSTED: ErrorInfo info = extractErrorInfo(cause); if (info != null @@ -319,7 +356,8 @@ static SpannerException newSpannerExceptionPreformatted( && AdminRequestsPerMinuteExceededException.ADMIN_REQUESTS_LIMIT_VALUE.equals( info.getMetadataMap() .get(AdminRequestsPerMinuteExceededException.ADMIN_REQUESTS_LIMIT_KEY))) { - return new AdminRequestsPerMinuteExceededException(token, message, cause, apiException); + return new AdminRequestsPerMinuteExceededException( + token, message, cause, apiException, reqId); } case NOT_FOUND: ResourceInfo resourceInfo = extractResourceInfo(cause); @@ -327,36 +365,39 @@ static SpannerException newSpannerExceptionPreformatted( switch (resourceInfo.getResourceType()) { case SESSION_RESOURCE_TYPE: return new SessionNotFoundException( - token, message, resourceInfo, cause, apiException); + token, message, resourceInfo, cause, apiException, reqId); case DATABASE_RESOURCE_TYPE: return new DatabaseNotFoundException( - token, message, resourceInfo, cause, apiException); + token, message, resourceInfo, cause, apiException, reqId); case INSTANCE_RESOURCE_TYPE: return new InstanceNotFoundException( - token, message, resourceInfo, cause, apiException); + token, message, resourceInfo, cause, apiException, reqId); } } case INVALID_ARGUMENT: if (isTransactionMutationLimitException(cause)) { return new TransactionMutationLimitExceededException( - token, code, message, cause, apiException); + token, code, message, cause, apiException, reqId); } if (isMissingDefaultSequenceKindException(apiException)) { - return new MissingDefaultSequenceKindException(token, code, message, cause, apiException); + return new MissingDefaultSequenceKindException( + token, code, message, cause, apiException, reqId); } // Fall through to the default. default: return new SpannerException( - token, code, isRetryable(code, cause), message, cause, apiException); + token, code, isRetryable(code, cause), message, cause, apiException, reqId); } } static SpannerException newSpannerExceptionPreformatted( ErrorCode code, @Nullable String message, @Nullable Throwable cause) { - return newSpannerExceptionPreformatted(code, message, cause, null); + return newSpannerExceptionPreformatted( + code, message, cause, null, (XGoogSpannerRequestId) (null)); } - private static SpannerException fromApiException(ApiException exception) { + private static SpannerException fromApiException( + ApiException exception, @Nullable XGoogSpannerRequestId reqId) { Status.Code code; if (exception.getStatusCode() instanceof GrpcStatusCode) { code = ((GrpcStatusCode) exception.getStatusCode()).getTransportCode(); @@ -371,7 +412,8 @@ private static SpannerException fromApiException(ApiException exception) { errorCode, formatMessage(errorCode, exception.getMessage()), exception.getCause(), - exception); + exception, + reqId); } private static boolean isRetryable(ErrorCode code, @Nullable Throwable cause) { diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerRetryHelper.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerRetryHelper.java index 5fb35513222..48aff4cbee7 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerRetryHelper.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerRetryHelper.java @@ -120,7 +120,8 @@ public TimedAttemptSettings createNextAttempt( public boolean shouldRetry(Throwable prevThrowable, T prevResponse) throws CancellationException { if (Context.current().isCancelled()) { - throw SpannerExceptionFactory.newSpannerExceptionForCancellation(Context.current(), null); + throw SpannerExceptionFactory.newSpannerExceptionForCancellation( + Context.current(), null, null); } return prevThrowable instanceof AbortedException || prevThrowable instanceof com.google.api.gax.rpc.AbortedException; diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionMutationLimitExceededException.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionMutationLimitExceededException.java index f04af2cae93..1fab7346ca7 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionMutationLimitExceededException.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionMutationLimitExceededException.java @@ -34,8 +34,9 @@ public class TransactionMutationLimitExceededException extends SpannerException ErrorCode errorCode, String message, Throwable cause, - @Nullable ApiException apiException) { - super(token, errorCode, /*retryable = */ false, message, cause, apiException); + @Nullable ApiException apiException, + @Nullable XGoogSpannerRequestId reqId) { + super(token, errorCode, /*retryable = */ false, message, cause, apiException, reqId); } static boolean isTransactionMutationLimitException(ErrorCode code, String message) { diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionRunnerImpl.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionRunnerImpl.java index fefbb383604..a715fae0fad 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionRunnerImpl.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionRunnerImpl.java @@ -685,7 +685,8 @@ options, getPreviousTransactionId()))) } throw se; } catch (InterruptedException e) { - throw SpannerExceptionFactory.newSpannerExceptionForCancellation(null, e); + throw SpannerExceptionFactory.newSpannerExceptionForCancellation( + null, e, null /*TODO: requestId*/); } } // There is already a transactionId available. Include that id as the transaction to use. @@ -1074,7 +1075,8 @@ public long[] batchUpdate(Iterable statements, UpdateOption... update throw newSpannerBatchUpdateException( ErrorCode.fromRpcStatus(response.getStatus()), response.getStatus().getMessage(), - results); + results, + null /*TODO: requestId*/); } return results; } catch (Throwable e) { @@ -1141,7 +1143,8 @@ public ApiFuture batchUpdateAsync( throw newSpannerBatchUpdateException( ErrorCode.fromRpcStatus(batchDmlResponse.getStatus()), batchDmlResponse.getStatus().getMessage(), - results); + results, + null /*TODO: requestId*/); } return results; }, diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/DdlBatch.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/DdlBatch.java index 813f5d6e45b..4cec1d5677b 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/DdlBatch.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/DdlBatch.java @@ -268,7 +268,7 @@ public ApiFuture runBatchAsync(CallType callType) { } catch (SpannerException e) { long[] updateCounts = extractUpdateCounts(operationReference.get()); throw SpannerExceptionFactory.newSpannerBatchUpdateException( - e.getErrorCode(), e.getMessage(), updateCounts); + e.getErrorCode(), e.getMessage(), updateCounts, null /* TODO: requestId */); } } catch (Throwable t) { span.setStatus(StatusCode.ERROR); diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java index 0e540ea7926..47026f0dc86 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java @@ -1974,7 +1974,7 @@ private static T get(final Future future) throws SpannerException { future.cancel(true); throw SpannerExceptionFactory.propagateInterrupt(e); } catch (Exception e) { - throw newSpannerException(context, e); + throw newSpannerException(context, e, null); } } diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerExceptionFactoryTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerExceptionFactoryTest.java index 2c4801bd0d7..9f0ee04e858 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerExceptionFactoryTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerExceptionFactoryTest.java @@ -250,4 +250,17 @@ private Metadata createResourceTypeMetadata(String resourceType, String resource return trailers; } + + @Test + public void withRequestId() { + XGoogSpannerRequestId reqIdIn = XGoogSpannerRequestId.of(1, 2, 3, 4); + Status status = Status.fromCodeValue(Status.Code.ABORTED.value()); + Exception exc = new StatusRuntimeException(status); + SpannerException spannerExceptionWithReqId = + SpannerExceptionFactory.newSpannerException(exc, reqIdIn); + assertThat(spannerExceptionWithReqId.getRequestId()).isEqualTo(reqIdIn.toString()); + SpannerException spannerExceptionWithoutReqId = + SpannerExceptionFactory.newSpannerException(exc); + assertThat(spannerExceptionWithoutReqId.getRequestId()).isEqualTo(""); + } } From d0827e110da44be960426bd49ea817166ff03d9f Mon Sep 17 00:00:00 2001 From: Emmanuel T Odeke Date: Fri, 11 Apr 2025 12:16:26 +0300 Subject: [PATCH 2/2] Overload newSpannerBatchUpdateException to avoid breakages --- .../com/google/cloud/spanner/SpannerExceptionFactory.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerExceptionFactory.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerExceptionFactory.java index e87e2927452..941f3b405b6 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerExceptionFactory.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerExceptionFactory.java @@ -139,6 +139,11 @@ public static SpannerException newSpannerException(Throwable cause, XGoogSpanner return newSpannerException(null, cause, reqId); } + public static SpannerBatchUpdateException newSpannerBatchUpdateException( + ErrorCode code, String message, long[] updateCounts) { + return newSpannerBatchUpdateException(code, message, updateCounts, null); + } + public static SpannerBatchUpdateException newSpannerBatchUpdateException( ErrorCode code, String message, long[] updateCounts, @Nullable XGoogSpannerRequestId reqId) { DoNotConstructDirectly token = DoNotConstructDirectly.ALLOWED;