Include retried failure messages as "suppressed" exceptions. (#3560)

Additionally, always include a useful exception message even when the service returns no error message. Stack trace example before this change: ``` software.amazon.awssdk.services.protocolrestjson.model.ProtocolRestJsonException: null (Service: ProtocolRestJson, Status Code: 400, Request ID: null) at software.amazon.awssdk.core.internal.http.CombinedResponseHandler.handleErrorResponse(CombinedResponseHandler.java:125) ... at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:54) ``` Stack trace example after this change: ``` software.amazon.awssdk.services.protocolrestjson.model.ProtocolRestJsonException: Service returned HTTP status code 400 (Service: ProtocolRestJson, Status Code: 400, Request ID: null) at software.amazon.awssdk.core.internal.http.CombinedResponseHandler.handleErrorResponse(CombinedResponseHandler.java:125) ... at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:54) Suppressed: software.amazon.awssdk.core.exception.SdkClientException: Request attempt 1 failure: Service returned HTTP status code 500 (Service: ProtocolRestJson, Status Code: 500, Request ID: null) Suppressed: software.amazon.awssdk.core.exception.SdkClientException: Request attempt 2 failure: Service returned HTTP status code 500 (Service: ProtocolRestJson, Status Code: 500, Request ID: null) ```
aws · Nov 23, 2022 · f14f4a2 · f14f4a2
1 parent de3f73d
commit f14f4a2
Show file tree

Hide file tree

Showing 25 changed files with 385 additions and 20 deletions.
diff --git a/.changes/next-release/feature-AWSSDKforJavav2-05b771b.json b/.changes/next-release/feature-AWSSDKforJavav2-05b771b.json
@@ -0,0 +1,6 @@
+{
+    "category": "AWS SDK for Java v2", 
+    "contributor": "", 
+    "type": "feature", 
+    "description": "When raising an exception as a result of a service response, if service does not return a error message, include the error code or HTTP status code in exception messages instead of the string \"null\"."
+}
diff --git a/.changes/next-release/feature-AWSSDKforJavav2-2636db0.json b/.changes/next-release/feature-AWSSDKforJavav2-2636db0.json
@@ -0,0 +1,6 @@
+{
+    "category": "AWS SDK for Java v2", 
+    "contributor": "", 
+    "type": "feature", 
+    "description": "When a request fails after SDK retries, include the retried failure messages as \"suppressed\" exceptions. Stack traces for these suppressed exceptions are not preserved."
+}
diff --git a/.idea/inspectionProfiles/AWS_Java_SDK_2_0.xml b/.idea/inspectionProfiles/AWS_Java_SDK_2_0.xml
diff --git a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/ExceptionProperties.java b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/ExceptionProperties.java
@@ -33,7 +33,8 @@ public static List<MethodSpec> builderInterfaceMethods(ClassName className) {
                 builderMethod(className, "message", String.class),
                 builderMethod(className, "requestId", String.class),
                 builderMethod(className, "statusCode", int.class),
-                builderMethod(className, "cause", Throwable.class));
+                builderMethod(className, "cause", Throwable.class),
+                builderMethod(className, "writableStackTrace", Boolean.class));
     }
 
     public static List<MethodSpec> builderImplMethods(ClassName className) {
@@ -42,7 +43,8 @@ public static List<MethodSpec> builderImplMethods(ClassName className) {
                 builderImplMethods(className, "message", String.class),
                 builderImplMethods(className, "requestId", String.class),
                 builderImplMethods(className, "statusCode", int.class),
-                builderImplMethods(className, "cause", Throwable.class));
+                builderImplMethods(className, "cause", Throwable.class),
+                builderImplMethods(className, "writableStackTrace", Boolean.class));
     }
 
     private static MethodSpec builderMethod(ClassName className, String name, Class clazz) {

diff --git a/...gen/src/main/resources/software/amazon/awssdk/codegen/rules/SourceException.java.resource b/...gen/src/main/resources/software/amazon/awssdk/codegen/rules/SourceException.java.resource
@@ -15,6 +15,9 @@ public class SourceException extends SdkException {
         @Override
         Builder cause(Throwable cause);
 
+        @Override
+        Builder writableStackTrace(Boolean writableStackTrace);
+
         @Override
         Builder message(String message);
 
@@ -35,6 +38,12 @@ public class SourceException extends SdkException {
             return this;
         }
 
+        @Override
+        public Builder writableStackTrace(Boolean writableStackTrace) {
+            super.writableStackTrace(writableStackTrace);
+            return this;
+        }
+
         @Override
         public SourceException build() {
             return new SourceException(this);

diff --git a/...en/src/test/resources/software/amazon/awssdk/codegen/poet/model/baseserviceexception.java b/...en/src/test/resources/software/amazon/awssdk/codegen/poet/model/baseserviceexception.java
@@ -37,6 +37,9 @@ public interface Builder extends AwsServiceException.Builder {
 
         @Override
         Builder cause(Throwable cause);
+
+        @Override
+        Builder writableStackTrace(Boolean writableStackTrace);
     }
 
     protected static class BuilderImpl extends AwsServiceException.BuilderImpl implements Builder {
@@ -77,6 +80,12 @@ public BuilderImpl cause(Throwable cause) {
             return this;
         }
 
+        @Override
+        public BuilderImpl writableStackTrace(Boolean writableStackTrace) {
+            this.writableStackTrace = writableStackTrace;
+            return this;
+        }
+
         @Override
         public JsonProtocolTestsException build() {
             return new JsonProtocolTestsException(this);

diff --git a/...n/src/test/resources/software/amazon/awssdk/codegen/poet/model/emptymodeledexception.java b/...n/src/test/resources/software/amazon/awssdk/codegen/poet/model/emptymodeledexception.java
@@ -56,6 +56,9 @@ public interface Builder extends SdkPojo, CopyableBuilder<Builder, EmptyModeledE
 
         @Override
         Builder cause(Throwable cause);
+
+        @Override
+        Builder writableStackTrace(Boolean writableStackTrace);
     }
 
     static final class BuilderImpl extends JsonProtocolTestsException.BuilderImpl implements Builder {
@@ -96,6 +99,12 @@ public BuilderImpl cause(Throwable cause) {
             return this;
         }
 
+        @Override
+        public BuilderImpl writableStackTrace(Boolean writableStackTrace) {
+            this.writableStackTrace = writableStackTrace;
+            return this;
+        }
+
         @Override
         public EmptyModeledException build() {
             return new EmptyModeledException(this);

diff --git a/.../aws-core/src/main/java/software/amazon/awssdk/awscore/exception/AwsServiceException.java b/.../aws-core/src/main/java/software/amazon/awssdk/awscore/exception/AwsServiceException.java
@@ -70,7 +70,11 @@ public String getMessage() {
             if (extendedRequestId() != null) {
                 details.add("Extended Request ID: " + extendedRequestId());
             }
-            return awsErrorDetails().errorMessage() + " " + details;
+            String message = super.getMessage();
+            if (message == null) {
+                message = awsErrorDetails().errorMessage();
+            }
+            return message + " " + details;
         }
 
         return super.getMessage();
@@ -234,6 +238,12 @@ public Builder cause(Throwable cause) {
             return this;
         }
 
+        @Override
+        public Builder writableStackTrace(Boolean writableStackTrace) {
+            this.writableStackTrace = writableStackTrace;
+            return this;
+        }
+
         @Override
         public Builder requestId(String requestId) {
             this.requestId = requestId;

diff --git a/...re/amazon/awssdk/protocols/json/internal/unmarshall/AwsJsonProtocolErrorUnmarshaller.java b/...re/amazon/awssdk/protocols/json/internal/unmarshall/AwsJsonProtocolErrorUnmarshaller.java
@@ -32,6 +32,7 @@
 import software.amazon.awssdk.protocols.json.ErrorCodeParser;
 import software.amazon.awssdk.protocols.json.JsonContent;
 import software.amazon.awssdk.thirdparty.jackson.core.JsonFactory;
+import software.amazon.awssdk.utils.StringUtils;
 
 /**
  * Unmarshaller for AWS specific error responses. All errors are unmarshalled into a subtype of
@@ -80,13 +81,25 @@ private AwsServiceException unmarshall(SdkHttpFullResponse response, ExecutionAt
                                                          errorCode, errorMessage));
         exception.clockSkew(getClockSkew(executionAttributes));
         // Status code and request id are sdk level fields
-        exception.message(errorMessage);
+        exception.message(errorMessageForException(errorMessage, errorCode, response.statusCode()));
         exception.statusCode(statusCode(response, modeledExceptionMetadata));
         exception.requestId(response.firstMatchingHeader(X_AMZN_REQUEST_ID_HEADERS).orElse(null));
         exception.extendedRequestId(response.firstMatchingHeader(X_AMZ_ID_2_HEADER).orElse(null));
         return exception.build();
     }
 
+    private String errorMessageForException(String errorMessage, String errorCode, int statusCode) {
+        if (StringUtils.isNotBlank(errorMessage)) {
+            return errorMessage;
+        }
+
+        if (StringUtils.isNotBlank(errorCode)) {
+            return "Service returned error code " + errorCode;
+        }
+
+        return "Service returned HTTP status code " + statusCode;
+    }
+
     private Duration getClockSkew(ExecutionAttributes executionAttributes) {
         Integer timeOffset = executionAttributes.getAttribute(SdkExecutionAttribute.TIME_OFFSET);
         return timeOffset == null ? null : Duration.ofSeconds(timeOffset);

diff --git a/.../java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryFailedException.java b/.../java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryFailedException.java
@@ -55,6 +55,9 @@ public interface Builder extends SdkClientException.Builder {
         @Override
         Builder cause(Throwable cause);
 
+        @Override
+        Builder writableStackTrace(Boolean writableStackTrace);
+
         @Override
         EndpointDiscoveryFailedException build();
     }
@@ -80,6 +83,12 @@ public Builder cause(Throwable cause) {
             return this;
         }
 
+        @Override
+        public Builder writableStackTrace(Boolean writableStackTrace) {
+            this.writableStackTrace = writableStackTrace;
+            return this;
+        }
+
         @Override
         public EndpointDiscoveryFailedException build() {
             return new EndpointDiscoveryFailedException(this);

diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/exception/AbortedException.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/exception/AbortedException.java
@@ -54,6 +54,9 @@ public interface Builder extends SdkClientException.Builder {
         @Override
         Builder cause(Throwable cause);
 
+        @Override
+        Builder writableStackTrace(Boolean writableStackTrace);
+
         @Override
         AbortedException build();
     }
@@ -79,6 +82,12 @@ public Builder cause(Throwable cause) {
             return this;
         }
 
+        @Override
+        public Builder writableStackTrace(Boolean writableStackTrace) {
+            this.writableStackTrace = writableStackTrace;
+            return this;
+        }
+
         @Override
         public AbortedException build() {
             return new AbortedException(this);

diff --git a/...e/src/main/java/software/amazon/awssdk/core/exception/ApiCallAttemptTimeoutException.java b/...e/src/main/java/software/amazon/awssdk/core/exception/ApiCallAttemptTimeoutException.java
@@ -58,6 +58,9 @@ public interface Builder extends SdkClientException.Builder {
         @Override
         Builder cause(Throwable cause);
 
+        @Override
+        Builder writableStackTrace(Boolean writableStackTrace);
+
         @Override
         ApiCallAttemptTimeoutException build();
     }
@@ -83,6 +86,12 @@ public Builder cause(Throwable cause) {
             return this;
         }
 
+        @Override
+        public Builder writableStackTrace(Boolean writableStackTrace) {
+            this.writableStackTrace = writableStackTrace;
+            return this;
+        }
+
         @Override
         public ApiCallAttemptTimeoutException build() {
             return new ApiCallAttemptTimeoutException(this);

diff --git a/...sdk-core/src/main/java/software/amazon/awssdk/core/exception/ApiCallTimeoutException.java b/...sdk-core/src/main/java/software/amazon/awssdk/core/exception/ApiCallTimeoutException.java
@@ -58,6 +58,9 @@ public interface Builder extends SdkClientException.Builder {
         @Override
         Builder cause(Throwable cause);
 
+        @Override
+        Builder writableStackTrace(Boolean writableStackTrace);
+
         @Override
         ApiCallTimeoutException build();
     }
@@ -83,6 +86,12 @@ public Builder cause(Throwable cause) {
             return this;
         }
 
+        @Override
+        public Builder writableStackTrace(Boolean writableStackTrace) {
+            this.writableStackTrace = writableStackTrace;
+            return this;
+        }
+
         @Override
         public ApiCallTimeoutException build() {
             return new ApiCallTimeoutException(this);

diff --git a/.../sdk-core/src/main/java/software/amazon/awssdk/core/exception/Crc32MismatchException.java b/.../sdk-core/src/main/java/software/amazon/awssdk/core/exception/Crc32MismatchException.java
@@ -58,6 +58,9 @@ public interface Builder extends SdkClientException.Builder {
         @Override
         Builder cause(Throwable cause);
 
+        @Override
+        Builder writableStackTrace(Boolean writableStackTrace);
+
         @Override
         Crc32MismatchException build();
     }
@@ -83,6 +86,12 @@ public Builder cause(Throwable cause) {
             return this;
         }
 
+        @Override
+        public Builder writableStackTrace(Boolean writableStackTrace) {
+            this.writableStackTrace = writableStackTrace;
+            return this;
+        }
+
         @Override
         public Crc32MismatchException build() {
             return new Crc32MismatchException(this);

diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/exception/NonRetryableException.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/exception/NonRetryableException.java
@@ -52,6 +52,9 @@ public interface Builder extends SdkClientException.Builder {
         @Override
         Builder cause(Throwable cause);
 
+        @Override
+        Builder writableStackTrace(Boolean writableStackTrace);
+
         @Override
         NonRetryableException build();
     }
@@ -87,6 +90,12 @@ public Builder cause(Throwable cause) {
             return this;
         }
 
+        @Override
+        public Builder writableStackTrace(Boolean writableStackTrace) {
+            this.writableStackTrace = writableStackTrace;
+            return this;
+        }
+
         @Override
         public NonRetryableException build() {
             return new NonRetryableException(this);

diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/exception/RetryableException.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/exception/RetryableException.java
@@ -60,6 +60,9 @@ public interface Builder extends SdkClientException.Builder {
         @Override
         Builder cause(Throwable cause);
 
+        @Override
+        Builder writableStackTrace(Boolean writableStackTrace);
+
         @Override
         RetryableException build();
     }
@@ -85,6 +88,12 @@ public Builder cause(Throwable cause) {
             return this;
         }
 
+        @Override
+        public Builder writableStackTrace(Boolean writableStackTrace) {
+            this.writableStackTrace = writableStackTrace;
+            return this;
+        }
+
         @Override
         public RetryableException build() {
             return new RetryableException(this);