diff --git a/.changes/next-release/bugfix-AWSSDKforJavaV2-2300783.json b/.changes/next-release/bugfix-AWSSDKforJavaV2-2300783.json new file mode 100644 index 000000000000..e43c2e88a150 --- /dev/null +++ b/.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." +} diff --git a/.changes/next-release/bugfix-AWSSDKforJavav2-bfa9190.json b/.changes/next-release/bugfix-AWSSDKforJavav2-bfa9190.json new file mode 100644 index 000000000000..513ffc4c0b57 --- /dev/null +++ b/.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." +} diff --git a/codegen/src/main/java/software/amazon/awssdk/codegen/AddExceptionShapes.java b/codegen/src/main/java/software/amazon/awssdk/codegen/AddExceptionShapes.java index be9f2c7ad6b9..0b8380a07c61 100644 --- a/codegen/src/main/java/software/amazon/awssdk/codegen/AddExceptionShapes.java +++ b/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) { diff --git a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/test-aws-query-compatible-json-async-client-class.java b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/test-aws-query-compatible-json-async-client-class.java index 4e1b37c04cf5..96f890a789a1 100644 --- a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/test-aws-query-compatible-json-async-client-class.java +++ b/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 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(); diff --git a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/test-aws-query-compatible-json-sync-client-class.java b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/test-aws-query-compatible-json-sync-client-class.java index 48a3b32ee4cf..d4fb640000aa 100644 --- a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/test-aws-query-compatible-json-sync-client-class.java +++ b/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(); diff --git a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/unmarshall/JsonErrorCodeParser.java b/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/unmarshall/JsonErrorCodeParser.java index 7c9b8a041841..76318af406e9 100644 --- a/core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/unmarshall/JsonErrorCodeParser.java +++ b/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 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 errorCodeHeaders; - private final String errorCodeFieldName; + private final List 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); } } diff --git a/core/protocols/smithy-rpcv2-protocol/src/main/java/software/amazon/awssdk/protocols/rpcv2/internal/SdkStructuredRpcV2CborFactory.java b/core/protocols/smithy-rpcv2-protocol/src/main/java/software/amazon/awssdk/protocols/rpcv2/internal/SdkStructuredRpcV2CborFactory.java index d595715bdd9a..368127e19727 100644 --- a/core/protocols/smithy-rpcv2-protocol/src/main/java/software/amazon/awssdk/protocols/rpcv2/internal/SdkStructuredRpcV2CborFactory.java +++ b/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() { diff --git a/services/cloudwatch/src/it/java/software/amazon/awssdk/services/cloudwatch/CloudWatchIntegrationTest.java b/services/cloudwatch/src/it/java/software/amazon/awssdk/services/cloudwatch/CloudWatchIntegrationTest.java index 8245d82a7ef5..763d820f3f17 100644 --- a/services/cloudwatch/src/it/java/software/amazon/awssdk/services/cloudwatch/CloudWatchIntegrationTest.java +++ b/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; @@ -369,8 +372,15 @@ public void testExceptionHandling() throws Exception { cloudwatch.getMetricStatistics(GetMetricStatisticsRequest.builder() .namespace("fake-namespace").build()); fail("Expected an SdkServiceException, but wasn't thrown"); - } catch (SdkServiceException e) { - assertThat(e, isValidSdkServiceException()); + } 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()); } } diff --git a/test/protocol-tests-core/src/main/java/software/amazon/awssdk/protocol/asserts/unmarshalling/UnmarshalledErrorAssertion.java b/test/protocol-tests-core/src/main/java/software/amazon/awssdk/protocol/asserts/unmarshalling/UnmarshalledErrorAssertion.java index 763e774a930f..e388f38bb42b 100644 --- a/test/protocol-tests-core/src/main/java/software/amazon/awssdk/protocol/asserts/unmarshalling/UnmarshalledErrorAssertion.java +++ b/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); }