Skip to content

Commit bdabc57

Browse files
authored
Merge pull request #818 from aws/fix-unknown-error
`s3`: Derive error code, message from HTTP status code for no body/header responses.
2 parents 4754c2f + 977bf97 commit bdabc57

File tree

10 files changed

+246
-93
lines changed

10 files changed

+246
-93
lines changed

codegen/smithy-aws-go-codegen/src/main/java/software/amazon/smithy/aws/go/codegen/AwsGoDependency.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,6 @@ protected static GoDependency module(
8181
}
8282

8383
private static final class Versions {
84-
private static final String AWS_SDK = "v0.0.0-20201006075021-8b185f9d6dff";
84+
private static final String AWS_SDK = "v0.26.1-0.20201016111247-66b2791dafc4";
8585
}
8686
}

codegen/smithy-aws-go-codegen/src/main/java/software/amazon/smithy/aws/go/codegen/XMLProtocolUtils.java

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -275,8 +275,15 @@ public static void writeXmlErrorMessageCodeDeserializer(ProtocolGenerator.Genera
275275

276276
if (requiresS3Customization(service)) {
277277
writer.addUseImports(AwsCustomGoDependency.S3_SHARED_CUSTOMIZATION);
278-
writer.write("errorComponents, err := s3shared.GetErrorResponseComponents(errorBody)");
278+
if (isS3Service(service)){
279+
writer.write("errorComponents, err := s3shared.GetS3ErrorResponseComponents(errorBody, response.StatusCode)");
280+
} else {
281+
// s3 control
282+
writer.write("errorComponents, err := s3shared.GetErrorResponseComponents(errorBody)");
283+
}
284+
279285
writer.write("if err != nil { return err }");
286+
280287
writer.insertTrailingNewline();
281288
writer.openBlock("if hostID := errorComponents.HostID; len(hostID)!=0 {", "}", () -> {
282289
writer.write("s3shared.SetHostIDMetadata(metadata, hostID)");
@@ -307,6 +314,11 @@ private static boolean requiresS3Customization(ServiceShape service) {
307314
String serviceId= service.expectTrait(ServiceTrait.class).getSdkId();
308315
return serviceId.equalsIgnoreCase("S3") || serviceId.equalsIgnoreCase("S3 Control");
309316
}
317+
318+
private static boolean isS3Service(ServiceShape service) {
319+
String serviceId= service.expectTrait(ServiceTrait.class).getSdkId();
320+
return serviceId.equalsIgnoreCase("S3");
321+
}
310322
}
311323

312324

codegen/smithy-aws-go-codegen/src/main/java/software/amazon/smithy/aws/go/codegen/customization/AwsCustomGoDependency.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ private AwsCustomGoDependency() {
4545
}
4646

4747
private static final class Versions {
48-
private static final String INTERNAL_S3SHARED = "v0.1.0";
48+
private static final String INTERNAL_S3SHARED = "v0.26.1-0.20201016111247-66b2791dafc4";
4949
private static final String INTERNAL_ACCEPTENCODING = "v0.0.0-20200930084954-897dfb99530c";
5050
}
5151
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
package software.amazon.smithy.aws.go.codegen.customization;
2+
3+
import software.amazon.smithy.aws.traits.ServiceTrait;
4+
import software.amazon.smithy.go.codegen.GoSettings;
5+
import software.amazon.smithy.go.codegen.integration.GoIntegration;
6+
import software.amazon.smithy.model.Model;
7+
import software.amazon.smithy.model.shapes.OperationShape;
8+
import software.amazon.smithy.model.shapes.ServiceShape;
9+
import software.amazon.smithy.model.shapes.ShapeId;
10+
11+
/**
12+
* This integration removes incorrectly modeled S3 `NoSuchKey` error for HeadObject operation.
13+
* Related to aws/aws-sdk-go#1208
14+
*
15+
*/
16+
public class S3HeadObjectCustomizations implements GoIntegration {
17+
@Override
18+
public byte getOrder() {
19+
return 127;
20+
}
21+
22+
@Override
23+
public Model preprocessModel(
24+
Model model, GoSettings settings
25+
) {
26+
ServiceShape service = model.expectShape(settings.getService(), ServiceShape.class);
27+
if (!isS3Service(model, service)) {
28+
return model;
29+
}
30+
31+
for (ShapeId operationId :service.getAllOperations()) {
32+
if (operationId.getName().equalsIgnoreCase("HeadObject")) {
33+
Model.Builder modelBuilder = model.toBuilder();
34+
OperationShape operationShape = model.expectShape(operationId, OperationShape.class);
35+
OperationShape.Builder builder = operationShape.toBuilder();
36+
// clear all errors associated with 'HeadObject' operation aws/aws-sdk-go#1208
37+
builder.clearErrors();
38+
39+
// remove old operation shape and add the updated shape to model
40+
return modelBuilder.removeShape(operationId).addShape(builder.build()).build();
41+
}
42+
}
43+
return model;
44+
}
45+
46+
// returns true if service is s3
47+
private static boolean isS3Service(Model model, ServiceShape service) {
48+
String serviceId= service.expectTrait(ServiceTrait.class).getSdkId();
49+
return serviceId.equalsIgnoreCase("S3");
50+
}
51+
}

codegen/smithy-aws-go-codegen/src/main/resources/META-INF/services/software.amazon.smithy.go.codegen.integration.GoIntegration

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,3 +24,4 @@ software.amazon.smithy.aws.go.codegen.customization.S3AcceptEncodingGzip
2424
software.amazon.smithy.aws.go.codegen.customization.KinesisCustomizations
2525
software.amazon.smithy.aws.go.codegen.customization.S3ErrorWith200Status
2626
software.amazon.smithy.aws.go.codegen.customization.Route53Customizations
27+
software.amazon.smithy.aws.go.codegen.customization.S3HeadObjectCustomizations

service/internal/s3shared/xml_utils.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import (
44
"encoding/xml"
55
"fmt"
66
"io"
7+
"net/http"
8+
"strings"
79
)
810

911
// ErrorComponents represents the error response fields
@@ -23,3 +25,21 @@ func GetErrorResponseComponents(r io.Reader) (ErrorComponents, error) {
2325
}
2426
return errComponents, nil
2527
}
28+
29+
// GetS3ErrorResponseComponents returns the error fields from an S3 xml error response body.
30+
// If an error code or message is not retrieved, it is derived from the http status code
31+
func GetS3ErrorResponseComponents(r io.Reader, statusCode int) (ErrorComponents, error) {
32+
errComponents, err := GetErrorResponseComponents(r)
33+
if err != nil {
34+
return ErrorComponents{}, err
35+
}
36+
37+
// for S3 service, we derive err code and message, if none is found
38+
if len(errComponents.Code) == 0 && len(errComponents.Message) == 0 {
39+
// derive code and message from status code
40+
statusText := http.StatusText(statusCode)
41+
errComponents.Code = strings.Replace(statusText, " ", "", -1)
42+
errComponents.Message = statusText
43+
}
44+
return errComponents, nil
45+
}
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
package s3shared
2+
3+
import (
4+
"strings"
5+
"testing"
6+
)
7+
8+
func TestGetResponseErrorCode(t *testing.T) {
9+
const xmlErrorResponse = `<Error>
10+
<Type>Sender</Type>
11+
<Code>InvalidGreeting</Code>
12+
<Message>Hi</Message>
13+
<HostId>bar-id</HostId>
14+
<RequestId>foo-id</RequestId>
15+
</Error>`
16+
17+
cases := map[string]struct {
18+
getErr func() (ErrorComponents, error)
19+
expectedErrorCode string
20+
expectedErrorMessage string
21+
expectedErrorRequestID string
22+
expectedErrorHostID string
23+
}{
24+
"standard xml error": {
25+
getErr: func() (ErrorComponents, error) {
26+
errResp := strings.NewReader(xmlErrorResponse)
27+
return GetErrorResponseComponents(errResp)
28+
},
29+
expectedErrorCode: "InvalidGreeting",
30+
expectedErrorMessage: "Hi",
31+
expectedErrorRequestID: "foo-id",
32+
expectedErrorHostID: "bar-id",
33+
},
34+
35+
"s3 no response body": {
36+
getErr: func() (ErrorComponents, error) {
37+
errResp := strings.NewReader("")
38+
return GetS3ErrorResponseComponents(errResp, 400)
39+
},
40+
expectedErrorCode: "BadRequest",
41+
expectedErrorMessage: "Bad Request",
42+
},
43+
"s3control no response body": {
44+
getErr: func() (ErrorComponents, error) {
45+
errResp := strings.NewReader("")
46+
return GetErrorResponseComponents(errResp)
47+
},
48+
},
49+
}
50+
51+
for name, c := range cases {
52+
t.Run(name, func(t *testing.T) {
53+
ec, err := c.getErr()
54+
if err != nil {
55+
t.Fatalf("expected no error, got %v", err)
56+
}
57+
58+
if e, a := c.expectedErrorCode, ec.Code; !strings.EqualFold(e, a) {
59+
t.Fatalf("expected %v, got %v", e, a)
60+
}
61+
if e, a := c.expectedErrorMessage, ec.Message; !strings.EqualFold(e, a) {
62+
t.Fatalf("expected %v, got %v", e, a)
63+
}
64+
if e, a := c.expectedErrorRequestID, ec.RequestID; !strings.EqualFold(e, a) {
65+
t.Fatalf("expected %v, got %v", e, a)
66+
}
67+
if e, a := c.expectedErrorHostID, ec.HostID; !strings.EqualFold(e, a) {
68+
t.Fatalf("expected %v, got %v", e, a)
69+
}
70+
})
71+
}
72+
}

0 commit comments

Comments
 (0)