Skip to content
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.

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

service/s3: Derive error code, message from HTTP status code for no body/header responses. #818

Merged
merged 10 commits into from
Oct 16, 2020
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,6 @@ protected static GoDependency module(
}

private static final class Versions {
private static final String AWS_SDK = "v0.0.0-20201006075021-8b185f9d6dff";
private static final String AWS_SDK = "v0.26.1-0.20201015193332-24a88063b255";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -275,8 +275,13 @@ public static void writeXmlErrorMessageCodeDeserializer(ProtocolGenerator.Genera

if (requiresS3Customization(service)) {
writer.addUseImports(AwsCustomGoDependency.S3_SHARED_CUSTOMIZATION);
writer.write("errorComponents, err := s3shared.GetErrorResponseComponents(errorBody)");
String getErrorComponents = String.format("errorComponents, err := "
+ "s3shared.GetErrorResponseComponents(errorBody, response.StatusCode, %s)",
isS3Service(service));
writer.write(getErrorComponents);
skotambkar marked this conversation as resolved.
Show resolved Hide resolved

writer.write("if err != nil { return err }");

writer.insertTrailingNewline();
writer.openBlock("if hostID := errorComponents.HostID; len(hostID)!=0 {", "}", () -> {
writer.write("s3shared.SetHostIDMetadata(metadata, hostID)");
Expand Down Expand Up @@ -307,6 +312,11 @@ private static boolean requiresS3Customization(ServiceShape service) {
String serviceId= service.expectTrait(ServiceTrait.class).getSdkId();
return serviceId.equalsIgnoreCase("S3") || serviceId.equalsIgnoreCase("S3 Control");
}

private static boolean isS3Service(ServiceShape service) {
String serviceId= service.expectTrait(ServiceTrait.class).getSdkId();
return serviceId.equalsIgnoreCase("S3");
}
}


Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ private AwsCustomGoDependency() {
}

private static final class Versions {
private static final String INTERNAL_S3SHARED = "v0.1.0";
private static final String INTERNAL_S3SHARED = "v0.26.1-0.20201015193332-24a88063b255";
private static final String INTERNAL_ACCEPTENCODING = "v0.0.0-20200930084954-897dfb99530c";
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package software.amazon.smithy.aws.go.codegen.customization;

import software.amazon.smithy.aws.traits.ServiceTrait;
import software.amazon.smithy.go.codegen.GoSettings;
import software.amazon.smithy.go.codegen.integration.GoIntegration;
import software.amazon.smithy.model.Model;
import software.amazon.smithy.model.shapes.OperationShape;
import software.amazon.smithy.model.shapes.ServiceShape;
import software.amazon.smithy.model.shapes.ShapeId;

/**
* This integration removes incorrectly modeled S3 `NoSuchKey` error for HeadObject operation.
* Related to aws/aws-sdk-go#1208
*
*/
public class S3HeadObjectCustomizations implements GoIntegration {
@Override
public byte getOrder() {
return 127;
}

@Override
public Model preprocessModel(
Model model, GoSettings settings
) {
ServiceShape service = model.expectShape(settings.getService(), ServiceShape.class);
if (!isS3Service(model, service)) {
return model;
}

for (ShapeId operationId :service.getAllOperations()) {
if (operationId.getName().equalsIgnoreCase("HeadObject")) {
Model.Builder modelBuilder = model.toBuilder();
OperationShape operationShape = model.expectShape(operationId, OperationShape.class);
OperationShape.Builder builder = operationShape.toBuilder();
// clear all errors associated with 'HeadObject' operation aws/aws-sdk-go#1208
builder.clearErrors();

// remove old operation shape and add the updated shape to model
return modelBuilder.removeShape(operationId).addShape(builder.build()).build();
}
}
return model;
}

// returns true if service is s3
private static boolean isS3Service(Model model, ServiceShape service) {
String serviceId= service.expectTrait(ServiceTrait.class).getSdkId();
return serviceId.equalsIgnoreCase("S3");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,4 @@ software.amazon.smithy.aws.go.codegen.customization.S3AcceptEncodingGzip
software.amazon.smithy.aws.go.codegen.customization.KinesisCustomizations
software.amazon.smithy.aws.go.codegen.customization.S3ErrorWith200Status
software.amazon.smithy.aws.go.codegen.customization.Route53Customizations
software.amazon.smithy.aws.go.codegen.customization.S3HeadObjectCustomizations
skotambkar marked this conversation as resolved.
Show resolved Hide resolved
13 changes: 12 additions & 1 deletion service/internal/s3shared/xml_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"encoding/xml"
"fmt"
"io"
"net/http"
"strings"
)

// ErrorComponents represents the error response fields
Expand All @@ -16,10 +18,19 @@ type ErrorComponents struct {
}

// GetErrorResponseComponents returns the error fields from an xml error response body
func GetErrorResponseComponents(r io.Reader) (ErrorComponents, error) {
func GetErrorResponseComponents(r io.Reader, statusCode int, isS3service bool) (ErrorComponents, error) {
skotambkar marked this conversation as resolved.
Show resolved Hide resolved
var errComponents ErrorComponents
if err := xml.NewDecoder(r).Decode(&errComponents); err != nil && err != io.EOF {
return ErrorComponents{}, fmt.Errorf("error while deserializing xml error response : %w", err)
}

// for S3 service, we derive err code and message, if none is found
if isS3service && len(errComponents.Code) == 0 && len(errComponents.Message) == 0 {
// derive code and message from status code
statusText := http.StatusText(statusCode)
errComponents.Code = strings.Replace(statusText, " ", "", -1)
errComponents.Message = statusText
}

return errComponents, nil
}
70 changes: 70 additions & 0 deletions service/internal/s3shared/xml_utils_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
package s3shared

import (
"bytes"
"io"
"strings"
"testing"
)

func TestGetResponseErrorCode(t *testing.T) {
cases := map[string]struct {
isS3Service bool
status int
errorResponse io.Reader
expectedErrorCode string
expectedErrorMessage string
expectedErrorRequestID string
expectedErrorHostID string
}{
"standard xml error": {
isS3Service: true,
status: 400,
errorResponse: bytes.NewReader([]byte(`<Error>
<Type>Sender</Type>
<Code>InvalidGreeting</Code>
<Message>Hi</Message>
<HostId>bar-id</HostId>
<RequestId>foo-id</RequestId>
</Error>`)),
expectedErrorCode: "InvalidGreeting",
expectedErrorMessage: "Hi",
expectedErrorRequestID: "foo-id",
expectedErrorHostID: "bar-id",
},
"s3 no response body": {
isS3Service: true,
status: 400,
errorResponse: bytes.NewReader([]byte(``)),
expectedErrorCode: "BadRequest",
expectedErrorMessage: "Bad Request",
},
"s3control no response body": {
isS3Service: false,
status: 400,
errorResponse: bytes.NewReader([]byte(``)),
},
}

for name, c := range cases {
t.Run(name, func(t *testing.T) {
ec, err := GetErrorResponseComponents(c.errorResponse, c.status, c.isS3Service)
if err != nil {
t.Fatalf("expected no error, got %v", err)
}

if e, a := c.expectedErrorCode, ec.Code; !strings.EqualFold(e, a) {
t.Fatalf("expected %v, got %v", e, a)
}
if e, a := c.expectedErrorMessage, ec.Message; !strings.EqualFold(e, a) {
t.Fatalf("expected %v, got %v", e, a)
}
if e, a := c.expectedErrorRequestID, ec.RequestID; !strings.EqualFold(e, a) {
t.Fatalf("expected %v, got %v", e, a)
}
if e, a := c.expectedErrorHostID, ec.HostID; !strings.EqualFold(e, a) {
t.Fatalf("expected %v, got %v", e, a)
}
})
}
}
Loading