Don't use the value of AwsQueryError in json rpc/smithy-rpc-v2-cbor protocols

.changes/next-release/bugfix-AWSSDKforJavaV2-2300783.json

@@ -0,0 +1,6 @@
+{
+    "type": "bugfix",
+    "category": "AWS SDK for Java V2",
+    "contributor": "",
+    "description": "Ensure rpc 1.0/1.1 error code parsing matches smithy spec: use both __type and code fields and handle uris in body error codes."
+}

.changes/next-release/bugfix-AWSSDKforJavav2-bfa9190.json

@@ -0,0 +1,6 @@
+{
+    "type": "bugfix",
+    "category": "AWS SDK for Java v2",
+    "contributor": "",
+    "description": "Don't use the value of AwsQueryError in json rpc/smithy-rpc-v2-cbor protocols."
+}

codegen/src/main/java/software/amazon/awssdk/codegen/AddExceptionShapes.java

@@ -84,7 +84,14 @@ private String getErrorCode(String errorShapeName) {
     }
 
     private boolean isErrorCodeOverridden(ErrorTrait errorTrait) {
-        return errorTrait != null && !Utils.isNullOrEmpty(errorTrait.getCode());
+        return protocolSupportsErrorCodeOverride() && errorTrait != null && !Utils.isNullOrEmpty(errorTrait.getCode());
+    }
+
+    // JSON (ie non-rest) and smithy-rpc-v2-cbor should ignore the AwsQueryError trait
+    // error code on the deserialized exceptions is parsed based on the awsQueryCompatible trait in:
+    // AwsJsonProtocolErrorUnmarshaller#getEffectiveErrorCode
+    private boolean protocolSupportsErrorCodeOverride() {
+        return !getProtocol().equals("json") && !getProtocol().equals("smithy-rpc-v2-cbor");
     }
 
     private Integer getHttpStatusCode(String errorShapeName) {

codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/test-aws-query-compatible-json-async-client-class.java

@@ -117,8 +117,8 @@ public CompletableFuture<APostOperationResponse> aPostOperation(APostOperationRe
                     return Optional.empty();
                 }
                 switch (errorCode) {
-                    case "InvalidInput":
-                        return Optional.of(ExceptionMetadata.builder().errorCode("InvalidInput").httpStatusCode(400)
+                    case "InvalidInputException":
+                        return Optional.of(ExceptionMetadata.builder().errorCode("InvalidInputException").httpStatusCode(400)
                                                             .exceptionBuilderSupplier(InvalidInputException::builder).build());
                     default:
                         return Optional.empty();

codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/test-aws-query-compatible-json-sync-client-class.java

@@ -102,8 +102,8 @@ public APostOperationResponse aPostOperation(APostOperationRequest aPostOperatio
                 return Optional.empty();
             }
             switch (errorCode) {
-                case "InvalidInput":
-                    return Optional.of(ExceptionMetadata.builder().errorCode("InvalidInput").httpStatusCode(400)
+                case "InvalidInputException":
+                    return Optional.of(ExceptionMetadata.builder().errorCode("InvalidInputException").httpStatusCode(400)
                                                         .exceptionBuilderSupplier(InvalidInputException::builder).build());
                 default:
                     return Optional.empty();

core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/unmarshall/JsonErrorCodeParser.java

@@ -16,6 +16,7 @@
 package software.amazon.awssdk.protocols.json.internal.unmarshall;
 
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 import software.amazon.awssdk.annotations.SdkInternalApi;
@@ -37,15 +38,18 @@ public class JsonErrorCodeParser implements ErrorCodeParser {
 
     static final String EXCEPTION_TYPE_HEADER = ":exception-type";
 
+    static final List<String> ERROR_CODE_FIELDS = Collections.unmodifiableList(Arrays.asList("__type", "code"));
+
     /**
      * List of header keys that represent the error code sent by service.
      * Response should only contain one of these headers
      */
     private final List<String> errorCodeHeaders;
-    private final String errorCodeFieldName;
+    private final List<String> errorCodeFieldNames;
 
-    public JsonErrorCodeParser(String errorCodeFieldName) {
-        this.errorCodeFieldName = errorCodeFieldName == null ? "__type" : errorCodeFieldName;
+    public JsonErrorCodeParser(String customErrorCodeFieldName) {
+        this.errorCodeFieldNames = customErrorCodeFieldName == null ?
+                                   ERROR_CODE_FIELDS : Collections.singletonList(customErrorCodeFieldName);
         this.errorCodeHeaders = Arrays.asList(X_AMZN_ERROR_TYPE, ERROR_CODE_HEADER, EXCEPTION_TYPE_HEADER);
     }
 
@@ -88,10 +92,7 @@ private String parseErrorCodeFromHeader(SdkHttpFullResponse response) {
 
     private String parseErrorCodeFromXAmzErrorType(String headerValue) {
         if (headerValue != null) {
-            int separator = headerValue.indexOf(':');
-            if (separator != -1) {
-                headerValue = headerValue.substring(0, separator);
-            }
+            return parseErrorCode(headerValue);
         }
         return headerValue;
     }
@@ -106,12 +107,47 @@ private String parseErrorCodeFromContents(JsonNode jsonContents) {
         if (jsonContents == null) {
             return null;
         }
-        JsonNode errorCodeField = jsonContents.field(errorCodeFieldName).orElse(null);
+        JsonNode errorCodeField = errorCodeFieldNames.stream()
+                                                     .map(jsonContents::field)
+                                                     .filter(Optional::isPresent)
+                                                     .map(Optional::get)
+                                                     .findFirst()
+                                                     .orElse(null);
         if (errorCodeField == null) {
             return null;
         }
-        String code = errorCodeField.text();
-        int separator = code.lastIndexOf('#');
-        return code.substring(separator + 1);
+        return parseErrorCode(errorCodeField.text());
+    }
+
+    // Extract the error code from the error code contents following the smithy defined rules:
+    // 1) If a : character is present, then take only the contents before the first : character in the value.
+    // 2) If a # character is present, then take only the contents after the first # character in the value.
+    // see: https://smithy.io/2.0/aws/protocols/aws-json-1_1-protocol.html#operation-error-serialization
+    private static String parseErrorCode(String value) {
+        if (value == null || value.isEmpty()) {
+            return value;
+        }
+
+        int start = 0;
+        int end = value.length();
+
+        // 1 - everything before the first ':'
+        int colonIndex = value.indexOf(':');
+        if (colonIndex >= 0) {
+            end = colonIndex;
+        }
+
+        // 2 - everything after the first '#'
+        int hashIndex = value.indexOf('#');
+        if (hashIndex >= 0 && hashIndex + 1 < end) {
+            start = hashIndex + 1;
+        }
+
+        // return original string if unchanged
+        if (start == 0 && end == value.length()) {
+            return value;
+        }
+
+        return value.substring(start, end);
     }
 }

core/protocols/smithy-rpcv2-protocol/src/main/java/software/amazon/awssdk/protocols/rpcv2/internal/SdkStructuredRpcV2CborFactory.java

@@ -17,7 +17,9 @@
 
 import software.amazon.awssdk.annotations.SdkInternalApi;
 import software.amazon.awssdk.protocols.json.BaseAwsStructuredJsonFactory;
+import software.amazon.awssdk.protocols.json.ErrorCodeParser;
 import software.amazon.awssdk.protocols.json.StructuredJsonGenerator;
+import software.amazon.awssdk.protocols.json.internal.unmarshall.JsonErrorCodeParser;
 import software.amazon.awssdk.thirdparty.jackson.core.JsonFactory;
 import software.amazon.awssdk.thirdparty.jackson.dataformat.cbor.CBORFactory;
 import software.amazon.awssdk.thirdparty.jackson.dataformat.cbor.CBORFactoryBuilder;
@@ -48,6 +50,13 @@ protected StructuredJsonGenerator createWriter(JsonFactory jsonFactory,
             public CBORFactory getJsonFactory() {
                 return CBOR_FACTORY;
             }
+
+            @Override
+            public ErrorCodeParser getErrorCodeParser(String customErrorCodeFieldName) {
+                // smithy cbor ONLY supports the __type field and not code.  Set a customErrorCode to use only __type
+                String errorCodeField = customErrorCodeFieldName == null ? "__type" : customErrorCodeFieldName;
+                return new JsonErrorCodeParser(errorCodeField);
+            }
         };
 
     private SdkStructuredRpcV2CborFactory() {

services/cloudwatch/src/it/java/software/amazon/awssdk/services/cloudwatch/CloudWatchIntegrationTest.java

@@ -23,6 +23,7 @@
 import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.isIn;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
@@ -59,9 +60,11 @@
 import software.amazon.awssdk.services.cloudwatch.model.Metric;
 import software.amazon.awssdk.services.cloudwatch.model.MetricAlarm;
 import software.amazon.awssdk.services.cloudwatch.model.MetricDatum;
+import software.amazon.awssdk.services.cloudwatch.model.MissingRequiredParameterException;
 import software.amazon.awssdk.services.cloudwatch.model.PutMetricAlarmRequest;
 import software.amazon.awssdk.services.cloudwatch.model.PutMetricDataRequest;
 import software.amazon.awssdk.services.cloudwatch.model.StateValue;
+import software.amazon.awssdk.services.cloudwatch.model.Statistic;
 import software.amazon.awssdk.testutils.Waiter;
 import software.amazon.awssdk.testutils.service.AwsIntegrationTestBase;
 
@@ -374,6 +377,23 @@ public void testExceptionHandling() throws Exception {
         }
     }
 
+    @Test
+    public void testQueryCompatibleExceptionHandling() {
+        try {
+            cloudwatch.getMetricStatistics(GetMetricStatisticsRequest.builder().namespace("foo").statistics(Statistic.AVERAGE).build());
+            fail("Expected a MissingRequiredParameterException, but wasn't thrown");
+        } catch (MissingRequiredParameterException e) {
+            // There is a strong contract on the value of these fields, and they should never change unexpectedly
+            assertEquals("MissingParameter", e.awsErrorDetails().errorCode());
+            assertEquals(400, e.statusCode());
+
+            // There is no strong contract on the exact value of these fields, but they should always be at least present
+            assertFalse(e.requestId().isEmpty());
+            assertFalse(e.awsErrorDetails().serviceName().isEmpty());
+            assertFalse(e.getMessage().isEmpty());
+        }
+    }
+
     /**
      * In the following test, we purposely setting the time offset to trigger a clock skew error.
      * The time offset must be fixed and then we validate the global value for time offset has been

test/protocol-tests-core/src/main/java/software/amazon/awssdk/protocol/asserts/unmarshalling/UnmarshalledErrorAssertion.java

@@ -16,6 +16,7 @@
 package software.amazon.awssdk.protocol.asserts.unmarshalling;
 
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.fail;
 import static org.unitils.reflectionassert.ReflectionAssert.assertReflectionEquals;
 
@@ -39,6 +40,7 @@ protected void doAssert(UnmarshallingTestContext context, Object actual) throws
         }
         SdkServiceException actualException = (SdkServiceException) actual;
         SdkServiceException expectedException = createExpectedResult(context);
+        assertEquals(expectedException.getClass(), actualException.getClass());
         for (Field field : expectedException.getClass().getDeclaredFields()) {
             assertFieldEquals(field, actualException, expectedException);
         }