Skip to content

Commit d0f2184

Browse files
authored
fix: improve error detection and reporting for BlobWriteChannel retry state (#846)
Add new checks to ensure a more informative error than NullPointerException is thrown if the StorageObject or it's size are unable to be resolved on the last chunk. Fixes #839
1 parent df98af2 commit d0f2184

File tree

1 file changed

+82
-10
lines changed

1 file changed

+82
-10
lines changed

google-cloud-storage/src/main/java/com/google/cloud/storage/BlobWriteChannel.java

+82-10
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import com.google.cloud.WriteChannel;
2727
import com.google.cloud.storage.spi.v1.StorageRpc;
2828
import com.google.common.collect.Maps;
29+
import java.math.BigInteger;
2930
import java.net.URL;
3031
import java.util.Map;
3132
import java.util.concurrent.Callable;
@@ -83,14 +84,52 @@ private StorageObject getRemoteStorageObject() {
8384
.get(getEntity().toPb(), Maps.newEnumMap(StorageRpc.Option.class));
8485
}
8586

86-
private StorageException unrecoverableState(
87-
int chunkOffset, int chunkLength, long localPosition, long remotePosition, boolean last) {
87+
private static StorageException unrecoverableState(
88+
String uploadId,
89+
int chunkOffset,
90+
int chunkLength,
91+
long localPosition,
92+
long remotePosition,
93+
boolean last) {
94+
return unrecoverableState(
95+
uploadId,
96+
chunkOffset,
97+
chunkLength,
98+
localPosition,
99+
remotePosition,
100+
last,
101+
"Unable to recover in upload.\nThis may be a symptom of multiple clients uploading to the same upload session.");
102+
}
103+
104+
private static StorageException errorResolvingMetadataLastChunk(
105+
String uploadId,
106+
int chunkOffset,
107+
int chunkLength,
108+
long localPosition,
109+
long remotePosition,
110+
boolean last) {
111+
return unrecoverableState(
112+
uploadId,
113+
chunkOffset,
114+
chunkLength,
115+
localPosition,
116+
remotePosition,
117+
last,
118+
"Unable to load object metadata to determine if last chunk was successfully written");
119+
}
120+
121+
private static StorageException unrecoverableState(
122+
String uploadId,
123+
int chunkOffset,
124+
int chunkLength,
125+
long localPosition,
126+
long remotePosition,
127+
boolean last,
128+
String message) {
88129
StringBuilder sb = new StringBuilder();
89-
sb.append("Unable to recover in upload.\n");
90-
sb.append(
91-
"This may be a symptom of multiple clients uploading to the same upload session.\n\n");
130+
sb.append(message).append("\n\n");
92131
sb.append("For debugging purposes:\n");
93-
sb.append("uploadId: ").append(getUploadId()).append('\n');
132+
sb.append("uploadId: ").append(uploadId).append('\n');
94133
sb.append("chunkOffset: ").append(chunkOffset).append('\n');
95134
sb.append("chunkLength: ").append(chunkLength).append('\n');
96135
sb.append("localOffset: ").append(localPosition).append('\n');
@@ -162,7 +201,7 @@ public void run() {
162201
// Get remote offset from API
163202
final long localPosition = getPosition();
164203
// For each request it should be possible to retry from its location in this code
165-
final long remotePosition = isRetrying() ? getRemotePosition() : getPosition();
204+
final long remotePosition = isRetrying() ? getRemotePosition() : localPosition;
166205
final int chunkOffset = (int) (remotePosition - localPosition);
167206
final int chunkLength = length - chunkOffset;
168207
final boolean uploadAlreadyComplete = remotePosition == -1;
@@ -176,10 +215,38 @@ public void run() {
176215
if (storageObject == null) {
177216
storageObject = getRemoteStorageObject();
178217
}
218+
// the following checks are defined here explicitly to provide a more
219+
// informative if either storageObject is unable to be resolved or it's size is
220+
// unable to be determined. This scenario is a very rare case of failure that
221+
// can arise when packets are lost.
222+
if (storageObject == null) {
223+
throw errorResolvingMetadataLastChunk(
224+
getUploadId(),
225+
chunkOffset,
226+
chunkLength,
227+
localPosition,
228+
remotePosition,
229+
lastChunk);
230+
}
179231
// Verify that with the final chunk we match the blob length
180-
if (storageObject.getSize().longValue() != getPosition() + length) {
232+
BigInteger size = storageObject.getSize();
233+
if (size == null) {
234+
throw errorResolvingMetadataLastChunk(
235+
getUploadId(),
236+
chunkOffset,
237+
chunkLength,
238+
localPosition,
239+
remotePosition,
240+
lastChunk);
241+
}
242+
if (size.longValue() != getPosition() + length) {
181243
throw unrecoverableState(
182-
chunkOffset, chunkLength, localPosition, remotePosition, lastChunk);
244+
getUploadId(),
245+
chunkOffset,
246+
chunkLength,
247+
localPosition,
248+
remotePosition,
249+
lastChunk);
183250
}
184251
retrying = false;
185252
} else if (uploadAlreadyComplete && !lastChunk && !checkingForLastChunk) {
@@ -201,7 +268,12 @@ public void run() {
201268
} else {
202269
// Case 4 && Case 8 && Case 9
203270
throw unrecoverableState(
204-
chunkOffset, chunkLength, localPosition, remotePosition, lastChunk);
271+
getUploadId(),
272+
chunkOffset,
273+
chunkLength,
274+
localPosition,
275+
remotePosition,
276+
lastChunk);
205277
}
206278
}
207279
}),

0 commit comments

Comments
 (0)