Skip to content

Support modeled exceptions in event streams #655

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 11, 2018
Merged

Support modeled exceptions in event streams #655

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 13 additions & 22 deletions codegen/src/main/java/software/amazon/awssdk/codegen/AddExceptionShapes.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,13 @@
package software.amazon.awssdk.codegen;

import java.util.HashMap;
import java.util.List;
import java.util.Map;
import software.amazon.awssdk.codegen.internal.Utils;
import software.amazon.awssdk.codegen.model.intermediate.OperationModel;
import software.amazon.awssdk.codegen.model.intermediate.ShapeModel;
import software.amazon.awssdk.codegen.model.intermediate.ShapeType;
import software.amazon.awssdk.codegen.model.service.ErrorMap;
import software.amazon.awssdk.codegen.model.service.ErrorTrait;
import software.amazon.awssdk.codegen.model.service.Operation;
import software.amazon.awssdk.codegen.model.service.Shape;

/**
* Constructs the exception shapes for the intermediate model. Analyzes the operations in the
Expand All @@ -46,28 +44,21 @@ private Map<String, ShapeModel> constructExceptionShapes() {
// Java shape models, to be constructed
final Map<String, ShapeModel> javaShapes = new HashMap<String, ShapeModel>();

for (Map.Entry<String, Operation> entry : getServiceModel().getOperations().entrySet()) {
for (Map.Entry<String, Shape> shape : getServiceModel().getShapes().entrySet()) {
if (shape.getValue().isException()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will be okay to do but you may want to validate this by importing the Dev preview package to see if any unexpected source changes occur in the generated client.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I validated the shape names but makes sense to review the entire generated code

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I verified that there is no diff in the generated code.

String errorShapeName = shape.getKey();
String javaClassName = getNamingStrategy().getExceptionName(errorShapeName);

Operation operation = entry.getValue();
List<ErrorMap> operationErrors = operation.getErrors();
ShapeModel exceptionShapeModel = generateShapeModel(javaClassName,
errorShapeName);

if (operationErrors != null) {
for (ErrorMap error : operationErrors) {

String errorShapeName = error.getShape();
String javaClassName = getNamingStrategy().getExceptionName(errorShapeName);

ShapeModel exceptionShapeModel = generateShapeModel(javaClassName,
errorShapeName);

exceptionShapeModel.setType(ShapeType.Exception.getValue());
exceptionShapeModel.setErrorCode(getErrorCode(errorShapeName));
if (exceptionShapeModel.getDocumentation() == null) {
exceptionShapeModel.setDocumentation(error.getDocumentation());
}

javaShapes.put(javaClassName, exceptionShapeModel);
exceptionShapeModel.setType(ShapeType.Exception.getValue());
exceptionShapeModel.setErrorCode(getErrorCode(errorShapeName));
if (exceptionShapeModel.getDocumentation() == null) {
exceptionShapeModel.setDocumentation(shape.getValue().getDocumentation());
}

javaShapes.put(javaClassName, exceptionShapeModel);
}
}

Expand Down
17 changes: 1 addition & 16 deletions codegen/src/main/java/software/amazon/awssdk/codegen/poet/client/specs/JsonProtocolSpec.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,7 @@
import java.util.Optional;
import java.util.stream.Collectors;
import javax.lang.model.element.Modifier;

import software.amazon.awssdk.awscore.eventstream.EventStreamAsyncResponseTransformer;
import software.amazon.awssdk.awscore.eventstream.EventStreamExceptionJsonUnmarshaller;
import software.amazon.awssdk.awscore.eventstream.EventStreamTaggedUnionJsonUnmarshaller;
import software.amazon.awssdk.awscore.exception.AwsServiceException;
import software.amazon.awssdk.awscore.internal.protocol.json.AwsJsonProtocol;
Expand Down Expand Up @@ -159,19 +157,6 @@ public CodeBlock responseHandler(IntermediateModel model, OperationModel opModel
});
builder.add(".defaultUnmarshaller((in) -> $T.UNKNOWN)\n"
+ ".build());\n", eventStreamUtils.eventStreamBaseClass());

builder.add("\n\n$T<$T> exceptionHandler = $L.createResponseHandler(\n" +
" new JsonOperationMetadata().withPayloadJson(true).withHasStreamingSuccessResponse(false),\n" +
" $T.builder()\n" +
" .defaultUnmarshaller(x -> $T.populateDefaultException($T::builder, x))\n" +
" .build());",
HttpResponseHandler.class,
WildcardTypeName.subtypeOf(Throwable.class),
protocolFactory,
EventStreamExceptionJsonUnmarshaller.class,
EventStreamExceptionJsonUnmarshaller.class,
baseExceptionClassName(model));

}
return builder.build();
}
Expand Down Expand Up @@ -229,7 +214,7 @@ public CodeBlock asyncExecutionHandler(OperationModel opModel) {
CodeBlock.Builder builder = CodeBlock.builder();
if (opModel.hasEventStreamOutput()) {
builder.add("$T<$T, $T> asyncResponseTransformer = new $T<>(\n"
+ "asyncResponseHandler, responseHandler, eventResponseHandler, exceptionHandler);\n",
+ "asyncResponseHandler, responseHandler, eventResponseHandler, errorResponseHandler, serviceName());\n",
ClassName.get(AsyncResponseTransformer.class),
ClassName.get(SdkResponse.class),
ClassName.get(Void.class),
Expand Down
13 changes: 1 addition & 12 deletions ...rc/test/resources/software/amazon/awssdk/codegen/poet/client/test-async-client-class.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import software.amazon.awssdk.awscore.AwsRequestOverrideConfiguration;
import software.amazon.awssdk.awscore.client.handler.AwsAsyncClientHandler;
import software.amazon.awssdk.awscore.eventstream.EventStreamAsyncResponseTransformer;
import software.amazon.awssdk.awscore.eventstream.EventStreamExceptionJsonUnmarshaller;
import software.amazon.awssdk.awscore.eventstream.EventStreamTaggedUnionJsonUnmarshaller;
import software.amazon.awssdk.awscore.exception.AwsServiceException;
import software.amazon.awssdk.awscore.internal.protocol.json.AwsJsonProtocol;
Expand Down Expand Up @@ -42,7 +41,6 @@
import software.amazon.awssdk.services.json.model.GetWithoutRequiredMembersRequest;
import software.amazon.awssdk.services.json.model.GetWithoutRequiredMembersResponse;
import software.amazon.awssdk.services.json.model.InvalidInputException;
import software.amazon.awssdk.services.json.model.JsonException;
import software.amazon.awssdk.services.json.model.JsonRequest;
import software.amazon.awssdk.services.json.model.PaginatedOperationWithResultKeyRequest;
import software.amazon.awssdk.services.json.model.PaginatedOperationWithResultKeyResponse;
Expand Down Expand Up @@ -222,18 +220,9 @@ public CompletableFuture<Void> eventStreamOperation(EventStreamOperationRequest
.addUnmarshaller("EventTwo", EventTwoUnmarshaller.getInstance())
.defaultUnmarshaller((in) -> EventStream.UNKNOWN).build());

HttpResponseHandler<? extends Throwable> exceptionHandler = jsonProtocolFactory
.createResponseHandler(
new JsonOperationMetadata().withPayloadJson(true).withHasStreamingSuccessResponse(false),
EventStreamExceptionJsonUnmarshaller
.builder()
.defaultUnmarshaller(
x -> EventStreamExceptionJsonUnmarshaller.populateDefaultException(
JsonException::builder, x)).build());

HttpResponseHandler<AwsServiceException> errorResponseHandler = createErrorResponseHandler(jsonProtocolFactory);
AsyncResponseTransformer<SdkResponse, Void> asyncResponseTransformer = new EventStreamAsyncResponseTransformer<>(
asyncResponseHandler, responseHandler, eventResponseHandler, exceptionHandler);
asyncResponseHandler, responseHandler, eventResponseHandler, errorResponseHandler, serviceName());

return clientHandler.execute(
new ClientExecutionParams<EventStreamOperationRequest, SdkResponse>()
Expand Down
36 changes: 33 additions & 3 deletions .../java/software/amazon/awssdk/awscore/eventstream/EventStreamAsyncResponseTransformer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package software.amazon.awssdk.awscore.eventstream;

import static java.util.Collections.singletonList;
import static software.amazon.awssdk.core.http.HttpResponseHandler.X_AMZN_REQUEST_ID_HEADER;
import static software.amazon.awssdk.utils.FunctionalUtils.runAndLogError;

import java.io.ByteArrayInputStream;
Expand All @@ -37,6 +38,7 @@
import software.amazon.awssdk.core.exception.SdkClientException;
import software.amazon.awssdk.core.http.HttpResponseHandler;
import software.amazon.awssdk.core.interceptor.ExecutionAttributes;
import software.amazon.awssdk.core.interceptor.SdkExecutionAttribute;
import software.amazon.awssdk.core.internal.util.ThrowableUtils;
import software.amazon.awssdk.http.AbortableInputStream;
import software.amazon.awssdk.http.SdkHttpFullResponse;
Expand Down Expand Up @@ -106,6 +108,18 @@ public class EventStreamAsyncResponseTransformer<ResponseT, EventT>
*/
private final AtomicReference<Throwable> error = new AtomicReference<>();

/**
* The name of the aws service
*/
private final String serviceName;

/**
* Request Id for the streaming request. The value is populated when the initial response is received from the service.
* As request id is not sent in event messages (including exceptions), this can be returned by the SDK along with
* received exception details.
*/
private String requestId = null;

/**
* @param eventStreamResponseTransformer Response transformer provided by customer.
* @param initialResponseUnmarshaller Unmarshaller for the initial-response event stream message.
Expand All @@ -115,19 +129,26 @@ public EventStreamAsyncResponseTransformer(
EventStreamResponseHandler<ResponseT, EventT> eventStreamResponseTransformer,
HttpResponseHandler<? extends ResponseT> initialResponseUnmarshaller,
HttpResponseHandler<? extends EventT> eventUnmarshaller,
HttpResponseHandler<? extends Throwable> exceptionUnmarshaller) {
HttpResponseHandler<? extends Throwable> exceptionUnmarshaller,
String serviceName) {

this.eventStreamResponseTransformer = eventStreamResponseTransformer;
this.initialResponseUnmarshaller = initialResponseUnmarshaller;
this.eventUnmarshaller = eventUnmarshaller;
this.exceptionUnmarshaller = exceptionUnmarshaller;
this.serviceName = serviceName;
}

@Override
public void responseReceived(SdkResponse response) {
// We use a void unmarshaller and unmarshall the actual response in the message
// decoder when we receive the initial-response frame. TODO not clear
// how we would handle REST protocol which would unmarshall the response from the HTTP headers
if (response != null && response.sdkHttpResponse() != null) {
this.requestId = response.sdkHttpResponse()
.firstMatchingHeader(X_AMZN_REQUEST_ID_HEADER)
.orElse(null);
}
}

@Override
Expand Down Expand Up @@ -205,8 +226,17 @@ private MessageDecoder createDecoder() {
EMPTY_EXECUTION_ATTRIBUTES));
}
} else if (isError(m) || isException(m)) {
Throwable exception = exceptionUnmarshaller.handle(adaptMessageToResponse(m, true),
EMPTY_EXECUTION_ATTRIBUTES);
SdkHttpFullResponse errorResponse = adaptMessageToResponse(m, true);
if (requestId != null) {
errorResponse = errorResponse.toBuilder()
.putHeader(X_AMZN_REQUEST_ID_HEADER, requestId)
.build();
}

Throwable exception = exceptionUnmarshaller.handle(errorResponse,
new ExecutionAttributes()
.putAttribute(SdkExecutionAttribute.SERVICE_NAME,
serviceName));
runAndLogError(log, "Error thrown from exceptionOccurred, ignoring.", () -> exceptionOccurred(exception));
}
} catch (Exception e) {
Expand Down
2 changes: 2 additions & 0 deletions ...java/software/amazon/awssdk/awscore/eventstream/EventStreamExceptionJsonUnmarshaller.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,15 @@
import java.util.function.Supplier;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import software.amazon.awssdk.annotations.ReviewBeforeRelease;
import software.amazon.awssdk.annotations.SdkProtectedApi;
import software.amazon.awssdk.awscore.exception.AwsErrorDetails;
import software.amazon.awssdk.awscore.exception.AwsServiceException;
import software.amazon.awssdk.core.SdkBytes;
import software.amazon.awssdk.core.runtime.transform.JsonUnmarshallerContext;
import software.amazon.awssdk.core.runtime.transform.Unmarshaller;

@ReviewBeforeRelease("Not currently used, keeping for backwards compatibility. Remove at GA.")
@SdkProtectedApi
public class EventStreamExceptionJsonUnmarshaller<T extends AwsServiceException>
implements Unmarshaller<T, JsonUnmarshallerContext> {
Expand Down
42 changes: 41 additions & 1 deletion .../main/java/software/amazon/awssdk/awscore/internal/protocol/json/JsonErrorCodeParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@
package software.amazon.awssdk.awscore.internal.protocol.json;

import com.fasterxml.jackson.databind.JsonNode;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import software.amazon.awssdk.annotations.SdkInternalApi;
import software.amazon.awssdk.core.internal.protocol.json.JsonContent;
import software.amazon.awssdk.http.SdkHttpFullResponse;
Expand All @@ -29,6 +35,17 @@ public class JsonErrorCodeParser implements ErrorCodeParser {
*/
public static final String X_AMZN_ERROR_TYPE = "x-amzn-ErrorType";

static final String ERROR_CODE_HEADER = ":error-code";

static final String EXCEPTION_TYPE_HEADER = ":exception-type";

private static final Logger log = LoggerFactory.getLogger(JsonErrorCodeParser.class);

/**
* 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;

public JsonErrorCodeParser() {
Expand All @@ -37,6 +54,7 @@ public JsonErrorCodeParser() {

public JsonErrorCodeParser(String errorCodeFieldName) {
this.errorCodeFieldName = errorCodeFieldName == null ? "__type" : errorCodeFieldName;
this.errorCodeHeaders = Arrays.asList(X_AMZN_ERROR_TYPE, ERROR_CODE_HEADER, EXCEPTION_TYPE_HEADER);
}

/**
Expand All @@ -60,7 +78,29 @@ public String parseErrorCode(SdkHttpFullResponse response, JsonContent jsonConte
* present in the header.
*/
private String parseErrorCodeFromHeader(SdkHttpFullResponse response) {
String headerValue = response.firstMatchingHeader(X_AMZN_ERROR_TYPE).orElse(null);
Map<String, List<String>> filteredHeaders = response.headers().entrySet().stream()
.filter(e -> errorCodeHeaders.contains(e.getKey()))
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));

if (filteredHeaders.isEmpty()) {
return null;
}

if (filteredHeaders.size() > 1) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the customer experience in this situation? We generally try to be as resilient as possible when unmarshlaling the exceptions so we can preserve as much info as possible about the error.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you saying we should combine all the error codes? I think its confusing if service multiple headers representing the error code with different values. If they do, maybe we can select the first option and return it? what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whatever we do we should try to fail as gracefully a possible so that we still generate a service exception rather than a runtime one. I'm thinking selecting the first one we find and log a warning.

log.warn("Response contains multiple headers representing the error code: " + filteredHeaders.keySet());
}

String headerKey = filteredHeaders.keySet().stream().findFirst().get();
String headerValue = filteredHeaders.get(headerKey).get(0);

if (X_AMZN_ERROR_TYPE.equals(headerKey)) {
return parseErrorCodeFromXAmzErrorType(headerValue);
}

return headerValue;
}

private String parseErrorCodeFromXAmzErrorType(String headerValue) {
if (headerValue != null) {
int separator = headerValue.indexOf(':');
if (separator != -1) {
Expand Down
12 changes: 11 additions & 1 deletion ...src/main/java/software/amazon/awssdk/awscore/protocol/json/AwsJsonErrorMessageParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ public final class AwsJsonErrorMessageParser implements ErrorMessageParser {
*/
private static final String X_AMZN_ERROR_MESSAGE = "x-amzn-error-message";

/**
* Error message header returned by event stream errors
*/
private static final String EVENT_ERROR_MESSAGE = ":error-message";

private SdkJsonErrorMessageParser errorMessageParser;

/**
Expand All @@ -50,11 +55,16 @@ public AwsJsonErrorMessageParser(SdkJsonErrorMessageParser errorMessageJsonLocat
*/
@Override
public String parseErrorMessage(SdkHttpFullResponse httpResponse, JsonNode jsonNode) {
// If X_AMZN_ERROR_MESSAGE is present, prefer that. Otherwise check the JSON body.
final String headerMessage = httpResponse.firstMatchingHeader(X_AMZN_ERROR_MESSAGE).orElse(null);
if (headerMessage != null) {
return headerMessage;
}

final String eventHeaderMessage = httpResponse.firstMatchingHeader(EVENT_ERROR_MESSAGE).orElse(null);
if (eventHeaderMessage != null) {
return eventHeaderMessage;
}

return errorMessageParser.parseErrorMessage(httpResponse, jsonNode);
}

Expand Down
2 changes: 1 addition & 1 deletion ...a/software/amazon/awssdk/awscore/eventstream/EventStreamAsyncResponseTransformerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ private void verifyExceptionThrown(Map<String, HeaderValue> headers) {

AsyncResponseTransformer<SdkResponse, Void> transformer =
new EventStreamAsyncResponseTransformer<>(new SubscribingResponseHandler(), null, null,
(response, executionAttributes) -> exception);
(response, executionAttributes) -> exception, null);
transformer.responseReceived(null);
transformer.onStream(SdkPublisher.adapt(bytePublisher));

Expand Down
Loading