Skip to content

Commit bce83a3

Browse files
chore: update StorageException ErrorDetails attachment logic to avoid duplication (googleapis#3025)
Co-authored-by: cojenco <[email protected]>
1 parent 62b6248 commit bce83a3

File tree

2 files changed

+51
-2
lines changed

2 files changed

+51
-2
lines changed

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

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,8 +177,8 @@ static StorageException asStorageException(ApiException apiEx) {
177177
}
178178

179179
private static void attachErrorDetails(ApiException ae) {
180-
if (ae != null && ae.getErrorDetails() != null) {
181-
final StringBuilder sb = new StringBuilder();
180+
if (ae != null && ae.getErrorDetails() != null && !errorDetailsAttached(ae)) {
181+
StringBuilder sb = new StringBuilder();
182182
ErrorDetails ed = ae.getErrorDetails();
183183
sb.append("ErrorDetails {\n");
184184
Stream.of(
@@ -202,6 +202,16 @@ private static void attachErrorDetails(ApiException ae) {
202202
}
203203
}
204204

205+
private static boolean errorDetailsAttached(ApiException ae) {
206+
Throwable[] suppressed = ae.getSuppressed();
207+
for (Throwable throwable : suppressed) {
208+
if (throwable instanceof ApiExceptionErrorDetailsComment) {
209+
return true;
210+
}
211+
}
212+
return false;
213+
}
214+
205215
/**
206216
* Translate IOException to a StorageException representing the cause of the error. This method
207217
* defaults to idempotent always being {@code true}. Additionally, this method translates

google-cloud-storage/src/test/java/com/google/cloud/storage/StorageExceptionGrpcCompatibilityTest.java

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,45 @@ public void apiExceptionErrorDetails() throws Exception {
207207
() -> assertThat(message).contains("\t}"));
208208
}
209209

210+
@SuppressWarnings("ThrowableNotThrown")
211+
@Test
212+
public void apiExceptionErrorDetails_onlyAttachedOnce() throws Exception {
213+
Help help =
214+
Help.newBuilder()
215+
.addLinks(
216+
Link.newBuilder().setDescription("link1").setUrl("https://google.com").build())
217+
.build();
218+
List<Any> errors = ImmutableList.of(Any.pack(help));
219+
ErrorDetails errorDetails = ErrorDetails.builder().setRawErrorMessages(errors).build();
220+
221+
ApiException ex =
222+
ApiExceptionFactory.createException(
223+
Code.OUT_OF_RANGE.toStatus().asRuntimeException(),
224+
GrpcStatusCode.of(Code.OUT_OF_RANGE),
225+
false,
226+
errorDetails);
227+
228+
// apply a coalesce to the exception -- similar to what a retry algorithm might do to determine
229+
// retryability. This is not ideal, as it is unpure but it is the way things are today with the
230+
// structure of storage exception and ApiException.
231+
BaseServiceException ignore1 = StorageException.coalesce(ex);
232+
BaseServiceException se = StorageException.coalesce(ex);
233+
234+
String message = TestUtils.messagesToText(se);
235+
Printer printer = TextFormat.printer();
236+
assertAll(
237+
() -> assertThat(message).contains("ErrorDetails {"),
238+
() -> assertThat(message).contains(printer.shortDebugString(help)),
239+
() -> assertThat(message).contains("\t}"),
240+
() -> {
241+
// make sure the error details are only attached to the exception once
242+
String str = "ErrorDetails {";
243+
int indexOf1 = message.indexOf(str);
244+
int indexOf2 = message.indexOf(str, indexOf1 + str.length());
245+
assertThat(indexOf2).isEqualTo(-1);
246+
});
247+
}
248+
210249
private void doTestCoalesce(int expectedCode, Code code) {
211250
Status status = code.toStatus();
212251
GrpcStatusCode statusCode = GrpcStatusCode.of(code);

0 commit comments

Comments
 (0)