Skip to content

Commit 4964d5c

Browse files
committed
feat(gax): surface actionable error messages with upload session URL and stream requirements
Augments terminal failure exceptions with the active upload session URL to aid debugging and session recovery. Clarifies error messages when a server committed offset falls below the buffer base offset.
1 parent aad2e09 commit 4964d5c

9 files changed

Lines changed: 266 additions & 51 deletions

File tree

‎java-showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITResumableUpload.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,7 @@ void testChunkFatalError(@TempDir Path tempDir) throws Exception {
277277
assertThat(exception.getCause()).isInstanceOf(NotFoundException.class);
278278
NotFoundException notFoundException = (NotFoundException) exception.getCause();
279279
assertThat(notFoundException.getStatusCode().getCode()).isEqualTo(StatusCode.Code.NOT_FOUND);
280+
assertThat(notFoundException.getMessage()).contains(future.getUploadSessionUrl());
280281
}
281282
}
282283

‎sdk-platform-java/gax-java/gax-httpjson/src/test/java/com/google/api/gax/httpjson/HttpJsonResumableUploadClientTest.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -566,7 +566,9 @@ void queryStatus_serverReturnsFinalStatusOnNon200_marksExceptionNonRetryable() {
566566
UnavailableException ex =
567567
assertThrows(UnavailableException.class, () -> client.queryStatusCallable().call(request));
568568
assertThat(ex.isRetryable()).isFalse();
569-
assertThat(ex).hasMessageThat().contains("Server terminated upload session with HTTP status: 503");
569+
assertThat(ex)
570+
.hasMessageThat()
571+
.contains("Server terminated upload session with HTTP status: 503");
570572
assertThat(ex.getStatusCode()).isInstanceOf(ResumableUploadStatusCode.class);
571573
assertThat(((ResumableUploadStatusCode) ex.getStatusCode()).getUploadStatus())
572574
.isEqualTo(ResumableUploadStatus.FINAL);

‎sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/resumable/ResumableUploadStatusCode.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@
3939
import org.jspecify.annotations.Nullable;
4040

4141
/**
42-
* A {@link StatusCode} that preserves the transport status code of a failed resumable upload command
43-
* and carries the {@code X-Goog-Upload-Status} header value from the response.
42+
* A {@link StatusCode} that preserves the transport status code of a failed resumable upload
43+
* command and carries the {@code X-Goog-Upload-Status} header value from the response.
4444
*
4545
* <p>Used to propagate a server rejection (a non-2xx response with {@code X-Goog-Upload-Status:
4646
* final}) from the transport layer to the resumable upload retry logic, so that the rejection is

‎sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/rpc/ResumableUploadChunkCoordinator.java‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ final class ResumableUploadChunkCoordinator<ResponseT> {
9999
this.executor = checkNotNull(executor, "executor must not be null");
100100
this.recoverySettings = recoveryAlgorithm.createFirstAttempt();
101101
this.progressTracker = checkNotNull(progressTracker, "progressTracker must not be null");
102-
this.buffer = new RewindableStreamBuffer(payload, chunkSize, uploadUrl);
102+
this.buffer = new RewindableStreamBuffer(payload, chunkSize);
103103
}
104104

105105
ApiFuture<ResponseT> getFuture() {
@@ -149,8 +149,7 @@ public void onSuccess(ChunkUploadResponse<ResponseT> response) {
149149
if (response.getUploadStatus() == ResumableUploadStatus.UNKNOWN) {
150150
recover(
151151
new IllegalStateException(
152-
"Chunk upload response missing X-Goog-Upload-Status header for upload URL: "
153-
+ uploadUrl));
152+
"Chunk upload response missing X-Goog-Upload-Status header"));
154153
} else {
155154
try {
156155
handleChunkResponse(response);
@@ -262,8 +261,7 @@ private boolean tryRegisterInFlightFuture(Future<?> future) {
262261
private void handleQueryResponse(QueryStatusResponse<ResponseT> queryResponse)
263262
throws IOException {
264263
if (queryResponse.getUploadStatus() == ResumableUploadStatus.UNKNOWN) {
265-
throw new IllegalStateException(
266-
"Query status response missing X-Goog-Upload-Status header for upload URL: " + uploadUrl);
264+
throw new IllegalStateException("Query status response missing X-Goog-Upload-Status header");
267265
}
268266
if (queryResponse.getUploadStatus() == ResumableUploadStatus.FINAL) {
269267
progressTracker.onFinalized(buffer.getBufferBaseOffset() + buffer.getPayload().length);
@@ -273,8 +271,7 @@ private void handleQueryResponse(QueryStatusResponse<ResponseT> queryResponse)
273271
Long committedOffset = queryResponse.getCommittedOffset();
274272
if (committedOffset == null) {
275273
throw new IllegalStateException(
276-
"Incomplete query status response did not include a committed offset for upload URL: "
277-
+ uploadUrl);
274+
"Incomplete query status response did not include a committed offset");
278275
}
279276
progressTracker.onOffsetReceived(committedOffset);
280277
buffer.realignTo(committedOffset);
@@ -289,8 +286,7 @@ private void handleChunkResponse(ChunkUploadResponse<ResponseT> response) throws
289286
uploadResultFuture.setException(
290287
new IllegalStateException(
291288
"Upload stream ended and final chunk was transmitted, but server returned"
292-
+ " incomplete status for upload URL: "
293-
+ uploadUrl));
289+
+ " incomplete status"));
294290
} else {
295291
madeProgressSinceRecovery = true;
296292
buffer.fill();

‎sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/rpc/ResumableUploadFuture.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,12 @@
4141
* <p>The payload {@link java.io.InputStream} supplied when initiating the upload is managed by this
4242
* future and will be closed automatically upon completion, failure, or cancellation.
4343
*
44+
* <p>If the upload fails, {@link #get()} throws an {@link java.util.concurrent.ExecutionException}
45+
* whose cause is an {@link ApiException}. If the upload is cancelled, {@link #get()} throws a
46+
* {@link java.util.concurrent.CancellationException}. When the upload session URL is known, it is
47+
* included in the exception message. A rejection by the server is terminal and is not retried or
48+
* recovered.
49+
*
4450
* @param <ResponseT> the type of the final response message returned once the upload completes
4551
*/
4652
@BetaApi

‎sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/rpc/ResumableUploadFutureImpl.java‎

Lines changed: 66 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -210,10 +210,9 @@ public void onFailure(Throwable t) {
210210
}
211211

212212
private void onTimeout() {
213-
String sessionUrl = uploadSessionUrl;
214213
String message;
215-
if (sessionUrl != null) {
216-
message = "Resumable upload timed out for session: " + sessionUrl;
214+
if (uploadSessionUrl != null) {
215+
message = "Resumable upload timed out";
217216
} else {
218217
message = "Resumable upload timed out before session initiation completed";
219218
}
@@ -243,9 +242,45 @@ private void fail(Throwable t) {
243242
if (inFlight != null) {
244243
inFlight.cancel(true);
245244
}
245+
Throwable terminal = toTerminalException(t);
246246
progressTracker.onFailed();
247-
closePayload();
248-
resultFuture.setException(t);
247+
try {
248+
payload.close();
249+
} catch (Throwable closeException) {
250+
terminal.addSuppressed(closeException);
251+
}
252+
resultFuture.setException(terminal);
253+
}
254+
255+
private Throwable toTerminalException(Throwable t) {
256+
String url = uploadSessionUrl;
257+
if (t instanceof ApiException && url == null) {
258+
return t;
259+
}
260+
String message = firstNonNull(t.getMessage(), t.getClass().getSimpleName());
261+
if (url != null) {
262+
message = message + " (upload URL: " + url + ")";
263+
}
264+
ApiException terminal;
265+
if (t instanceof ApiException) {
266+
ApiException apiException = (ApiException) t;
267+
terminal =
268+
ApiExceptionFactory.createException(
269+
message,
270+
apiException,
271+
apiException.getStatusCode(),
272+
apiException.isRetryable(),
273+
apiException.getErrorDetails());
274+
} else if (t instanceof IllegalStateException) {
275+
terminal =
276+
new FailedPreconditionException(message, t, FAILED_PRECONDITION_STATUS_CODE, false);
277+
} else {
278+
terminal = ApiExceptionFactory.createException(message, t, UNKNOWN_STATUS_CODE, false);
279+
}
280+
for (Throwable suppressed : t.getSuppressed()) {
281+
terminal.addSuppressed(suppressed);
282+
}
283+
return terminal;
249284
}
250285

251286
private void closePayload() {
@@ -324,6 +359,32 @@ public StatusCode.Code getCode() {
324359
return StatusCode.Code.DEADLINE_EXCEEDED;
325360
}
326361

362+
@Override
363+
public @Nullable Object getTransportCode() {
364+
return null;
365+
}
366+
};
367+
368+
private static final StatusCode FAILED_PRECONDITION_STATUS_CODE =
369+
new StatusCode() {
370+
@Override
371+
public StatusCode.Code getCode() {
372+
return StatusCode.Code.FAILED_PRECONDITION;
373+
}
374+
375+
@Override
376+
public @Nullable Object getTransportCode() {
377+
return null;
378+
}
379+
};
380+
381+
private static final StatusCode UNKNOWN_STATUS_CODE =
382+
new StatusCode() {
383+
@Override
384+
public StatusCode.Code getCode() {
385+
return StatusCode.Code.UNKNOWN;
386+
}
387+
327388
@Override
328389
public @Nullable Object getTransportCode() {
329390
return null;

‎sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/rpc/RewindableStreamBuffer.java‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -46,19 +46,17 @@ final class RewindableStreamBuffer {
4646

4747
private final InputStream inputStream;
4848
private final int chunkSize;
49-
private final String uploadUrl;
5049
private final byte[] buffer;
5150

5251
private long bufferBaseOffset;
5352
private int payloadLength;
5453
private boolean isFinal;
5554
private boolean streamExhausted;
5655

57-
RewindableStreamBuffer(InputStream inputStream, int chunkSize, String uploadUrl) {
56+
RewindableStreamBuffer(InputStream inputStream, int chunkSize) {
5857
this.inputStream = checkNotNull(inputStream, "inputStream must not be null");
5958
checkArgument(chunkSize > 0, "chunkSize must be > 0");
6059
this.chunkSize = chunkSize;
61-
this.uploadUrl = checkNotNull(uploadUrl, "uploadUrl must not be null");
6260
this.buffer = new byte[chunkSize];
6361
this.bufferBaseOffset = 0L;
6462
this.payloadLength = 0;
@@ -95,17 +93,16 @@ void realignTo(long committedOffset) throws IOException {
9593
if (committedOffset < bufferBaseOffset) {
9694
throw new IllegalStateException(
9795
String.format(
98-
"Server committed offset %d is below buffer base offset %d for upload URL %s; cannot"
99-
+ " rewind stream before buffer base",
100-
committedOffset, bufferBaseOffset, uploadUrl));
96+
"Server committed offset %d is below buffer base offset %d, which the server already"
97+
+ " acknowledged; the upload cannot continue and must be restarted.",
98+
committedOffset, bufferBaseOffset));
10199
}
102100

103101
if (committedOffset > bufferBaseOffset + payloadLength) {
104102
throw new IllegalStateException(
105103
String.format(
106-
"Server committed offset %d is beyond current buffer window [%d, %d] for upload URL"
107-
+ " %s",
108-
committedOffset, bufferBaseOffset, bufferBaseOffset + payloadLength, uploadUrl));
104+
"Server committed offset %d is beyond current buffer window [%d, %d]",
105+
committedOffset, bufferBaseOffset, bufferBaseOffset + payloadLength));
109106
}
110107

111108
int committedWithinBuffer = (int) (committedOffset - bufferBaseOffset);

0 commit comments

Comments
 (0)