Skip to content

Commit ab68395

Browse files
kuhekuhe
authored
fix(core/protocols): awsQueryCompatibility error structuring (aws#7541)
1 parent a934dcb commit ab68395

File tree

8 files changed

+185
-74
lines changed

8 files changed

+185
-74
lines changed

‎clients/client-cloudwatch/test/e2e/CloudWatch.e2e.spec.ts‎

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -34,18 +34,10 @@ describe(CloudWatch.name, () => {
3434
}),
3535
};
3636

37-
it("can make requests with AWS Query protocol", async () => {
38-
const dashes = await cloudwatch.query.listDashboards();
39-
expect(dashes.DashboardEntries ?? []).toBeInstanceOf(Array);
40-
});
41-
42-
it("can make requests with Smithy RPCv2 CBOR protocol", async () => {
43-
const dashes = await cloudwatch.cbor.listDashboards();
44-
expect(dashes.DashboardEntries ?? []).toBeInstanceOf(Array);
45-
});
46-
47-
it("can make requests with AWS JSON RPC protocol", async () => {
48-
const dashes = await cloudwatch.json.listDashboards();
49-
expect(dashes.DashboardEntries ?? []).toBeInstanceOf(Array);
50-
});
37+
for (const client of Object.values(cloudwatch)) {
38+
it(`can make requests with ${client.config.protocol.constructor.name}`, async () => {
39+
const dashes = await cloudwatch.query.listDashboards();
40+
expect(dashes.DashboardEntries ?? []).toBeInstanceOf(Array);
41+
});
42+
}
5143
});

‎clients/client-cloudwatch/test/e2e/query-compatibility.e2e.spec.ts‎

Lines changed: 124 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,132 @@
1-
import { CloudWatchClient, GetDashboardCommand } from "@aws-sdk/client-cloudwatch";
2-
import { beforeAll, describe, expect, test as it } from "vitest";
1+
import {
2+
CloudWatchClient,
3+
CloudWatchServiceException,
4+
GetDashboardCommand,
5+
GetInsightRuleReportCommand,
6+
MissingRequiredParameterException,
7+
PutMetricAlarmCommand,
8+
ResourceNotFound,
9+
} from "@aws-sdk/client-cloudwatch";
10+
import { AwsJson1_0Protocol, AwsQueryProtocol, AwsSmithyRpcV2CborProtocol } from "@aws-sdk/core";
11+
import { describe, expect, test as it } from "vitest";
312

413
describe("CloudWatch Query Compatibility E2E", () => {
5-
let client: CloudWatchClient;
6-
7-
beforeAll(async () => {
8-
client = new CloudWatchClient({
14+
const cloudwatch = {
15+
cbor: new CloudWatchClient({
16+
region: "us-west-2",
17+
protocol: new AwsSmithyRpcV2CborProtocol({
18+
defaultNamespace: "com.amazonaws.cloudwatch",
19+
awsQueryCompatible: true,
20+
}),
21+
}),
22+
query: new CloudWatchClient({
23+
region: "us-west-2",
24+
protocol: new AwsQueryProtocol({
25+
defaultNamespace: "com.amazonaws.cloudwatch",
26+
xmlNamespace: "http://monitoring.amazonaws.com/doc/2010-08-01/",
27+
version: "2010-08-01",
28+
}),
29+
}),
30+
json: new CloudWatchClient({
931
region: "us-west-2",
32+
protocol: new AwsJson1_0Protocol({
33+
defaultNamespace: "com.amazonaws.cloudwatch",
34+
serviceTarget: "GraniteServiceVersion20100801",
35+
awsQueryCompatible: true,
36+
}),
37+
}),
38+
};
39+
40+
for (const client of Object.values(cloudwatch)) {
41+
const ctorName = client.config.protocol.constructor.name;
42+
43+
it(`resolve errors with the query compat header and not the __type field (${ctorName})`, async () => {
44+
const error = await client
45+
.send(
46+
new GetDashboardCommand({
47+
DashboardName: "does-not-exist",
48+
})
49+
)
50+
.catch((_) => _);
51+
52+
expect(error).toBeInstanceOf(Error);
53+
expect(error).toBeInstanceOf(CloudWatchServiceException);
54+
expect(error).toBeInstanceOf(ResourceNotFound);
55+
56+
expect(error.constructor.prototype.name).toBe("Error");
57+
expect(error.constructor.name).toBe("ResourceNotFound");
58+
expect(error.name).toBe("ResourceNotFound");
59+
60+
expect(error.message).toBe("Dashboard does-not-exist does not exist");
61+
expect(error.Type).toEqual("Sender");
62+
expect(error.Code).toEqual("ResourceNotFound");
63+
expect(error.Error).toEqual({
64+
Type: "Sender",
65+
Code: "ResourceNotFound",
66+
Message: "Dashboard does-not-exist does not exist",
67+
});
68+
expect(error.$metadata.httpStatusCode).toBe(404);
1069
});
11-
});
1270

13-
it("AmbiguousErrorResolution", async () => {
14-
const command = new GetDashboardCommand({
15-
DashboardName: "foo",
71+
it(`have consistent error structure (modeled title-case 'Message') ${ctorName}`, async () => {
72+
const error = await client
73+
.send(
74+
new PutMetricAlarmCommand({
75+
AlarmName: "",
76+
ComparisonOperator: "GreaterThanThreshold",
77+
EvaluationPeriods: 5,
78+
MetricName: "CPUUtilization",
79+
Namespace: "AWS/EC2",
80+
Period: 60,
81+
Statistic: "Average",
82+
Threshold: 50.0,
83+
})
84+
)
85+
.catch((_) => _);
86+
87+
const msg =
88+
`1 validation error detected: Value '' at 'alarmName' failed to satisfy ` +
89+
`constraint: Member must have length greater than or equal to 1`;
90+
91+
expect(error).toBeInstanceOf(Error);
92+
expect(error).toBeInstanceOf(CloudWatchServiceException);
93+
94+
expect(error.constructor.prototype.name).toBe("Error");
95+
expect(error.constructor.name).toBe("CloudWatchServiceException");
96+
expect(error.name).toBe("ValidationError");
97+
98+
expect(error.message).toBe(msg);
99+
expect(error.Type).toEqual("Sender");
100+
expect(error.Code).toEqual("ValidationError");
101+
expect(error.Error).toEqual({
102+
Type: "Sender",
103+
Code: "ValidationError",
104+
Message: msg,
105+
});
106+
expect(error.$metadata.httpStatusCode).toBe(400);
16107
});
17108

18-
try {
19-
await client.send(command);
20-
fail("Expected ResourceNotFound error");
21-
} catch (error: any) {
22-
expect(error.name).toBe("ResourceNotFound");
23-
}
24-
});
109+
it(`have consistent error structure (modeled lowercase 'message') ${ctorName}`, async () => {
110+
const error = await client.send(new GetInsightRuleReportCommand({} as any)).catch((_) => _);
111+
const msg = `MISSING_RULE_NAME: The RuleName parameter must be present.`;
112+
113+
expect(error).toBeInstanceOf(Error);
114+
expect(error).toBeInstanceOf(CloudWatchServiceException);
115+
expect(error).toBeInstanceOf(MissingRequiredParameterException);
116+
117+
expect(error.constructor.prototype.name).toBe("Error");
118+
expect(error.constructor.name).toBe("MissingRequiredParameterException");
119+
expect(error.name).toBe("MissingRequiredParameterException");
120+
121+
expect(error.message).toBe(msg);
122+
expect(error.Type).toEqual("Sender");
123+
expect(error.Code).toEqual("MissingParameter");
124+
expect(error.Error).toEqual({
125+
Type: "Sender",
126+
Code: "MissingParameter",
127+
Message: msg,
128+
});
129+
expect(error.$metadata.httpStatusCode).toBe(400);
130+
});
131+
}
25132
});

‎packages/core/src/submodules/protocols/ProtocolLib.ts‎

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,11 +110,23 @@ export class ProtocolLib {
110110
): E {
111111
if (this.queryCompat) {
112112
const msg = (exception as any).Message ?? additions.Message;
113-
const error = decorateServiceException(exception, additions);
113+
const error: any = decorateServiceException(exception, additions);
114114
if (msg) {
115-
(error as any).Message = msg;
116-
(error as any).message = msg;
115+
error.message = msg;
117116
}
117+
118+
error.Error = {
119+
...error.Error,
120+
Type: error.Error.Type,
121+
Code: error.Error.Code,
122+
Message: error.Error.message ?? error.Error.Message ?? msg,
123+
};
124+
125+
const reqId = error.$metadata.requestId;
126+
if (reqId) {
127+
error.RequestId = reqId;
128+
}
129+
118130
return error;
119131
}
120132
return decorateServiceException(exception, additions);
@@ -138,7 +150,7 @@ export class ProtocolLib {
138150
} as any;
139151
Object.assign(output, Error);
140152
for (const [k, v] of entries) {
141-
Error[k] = v;
153+
Error[k === "message" ? "Message" : k] = v;
142154
}
143155
delete Error.__type;
144156
output.Error = Error;
@@ -161,4 +173,19 @@ export class ProtocolLib {
161173
errorData.Code = queryCompatErrorData.Code;
162174
}
163175
}
176+
177+
/**
178+
* Finds the canonical modeled error using the awsQueryError alias.
179+
* @param registry - service error registry.
180+
* @param errorName - awsQueryError name or regular qualified shapeId.
181+
*/
182+
public findQueryCompatibleError(registry: TypeRegistry, errorName: string) {
183+
try {
184+
return registry.getSchema(errorName) as StaticErrorSchema;
185+
} catch (e) {
186+
return registry.find(
187+
(schema) => (NormalizedSchema.of(schema).getMergedTraits().awsQueryError as any)?.[0] === errorName
188+
) as StaticErrorSchema;
189+
}
190+
}
164191
}

‎packages/core/src/submodules/protocols/cbor/AwsSmithyRpcV2CborProtocol.spec.ts‎

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -60,28 +60,18 @@ describe(AwsSmithyRpcV2CborProtocol.name, () => {
6060
// set by deserializer middleware, not Protocol.
6161
expect(error.$response).toEqual(undefined);
6262

63-
expect(error.Code).toEqual(MyQueryError.name);
64-
expect(error.Error.Code).toEqual(MyQueryError.name);
65-
66-
expect(error.Message).toEqual("oh no");
67-
expect(error.Prop2).toEqual(9999);
68-
69-
expect(error.Error.Message).toEqual("oh no");
70-
expect(error.Error.Prop2).toEqual(9999);
71-
7263
expect(error).toMatchObject({
7364
$fault: "client",
74-
Message: "oh no",
7565
message: "oh no",
7666
Prop2: 9999,
7767
Error: {
78-
Code: "MyQueryError",
68+
Code: MyQueryError.name,
7969
Message: "oh no",
8070
Type: "Client",
8171
Prop2: 9999,
8272
},
8373
Type: "Client",
84-
Code: "MyQueryError",
74+
Code: MyQueryError.name,
8575
});
8676
});
8777

‎packages/core/src/submodules/protocols/cbor/AwsSmithyRpcV2CborProtocol.ts‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,14 +61,21 @@ export class AwsSmithyRpcV2CborProtocol extends SmithyRpcV2CborProtocol {
6161
if (this.awsQueryCompatible) {
6262
this.mixin.setQueryCompatError(dataObject, response);
6363
}
64-
const errorName = loadSmithyRpcV2CborErrorCode(response, dataObject) ?? "Unknown";
64+
const errorName = (() => {
65+
const compatHeader = response.headers["x-amzn-query-error"];
66+
if (compatHeader && this.awsQueryCompatible) {
67+
return compatHeader.split(";")[0];
68+
}
69+
return loadSmithyRpcV2CborErrorCode(response, dataObject) ?? "Unknown";
70+
})();
6571

6672
const { errorSchema, errorMetadata } = await this.mixin.getErrorSchemaOrThrowBaseException(
6773
errorName,
6874
this.options.defaultNamespace,
6975
response,
7076
dataObject,
71-
metadata
77+
metadata,
78+
this.awsQueryCompatible ? this.mixin.findQueryCompatibleError : undefined
7279
);
7380

7481
const ns = NormalizedSchema.of(errorSchema);
@@ -78,7 +85,9 @@ export class AwsSmithyRpcV2CborProtocol extends SmithyRpcV2CborProtocol {
7885

7986
const output = {} as any;
8087
for (const [name, member] of ns.structIterator()) {
81-
output[name] = this.deserializer.readValue(member, dataObject[name]);
88+
if (dataObject[name] != null) {
89+
output[name] = this.deserializer.readValue(member, dataObject[name]);
90+
}
8291
}
8392

8493
if (this.awsQueryCompatible) {

‎packages/core/src/submodules/protocols/json/AwsJsonRpcProtocol.spec.ts‎

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -83,28 +83,18 @@ describe(AwsJsonRpcProtocol.name, () => {
8383
// set by deserializer middleware, not protocol
8484
expect(error.$response).toEqual(undefined);
8585

86-
expect(error.Code).toEqual(MyQueryError.name);
87-
expect(error.Error.Code).toEqual(MyQueryError.name);
88-
89-
expect(error.Message).toEqual("oh no");
90-
expect(error.Prop2).toEqual(9999);
91-
92-
expect(error.Error.Message).toEqual("oh no");
93-
expect(error.Error.Prop2).toEqual(9999);
94-
9586
expect(error).toMatchObject({
9687
$fault: "client",
97-
Message: "oh no",
9888
message: "oh no",
9989
Prop2: 9999,
10090
Error: {
101-
Code: "MyQueryError",
91+
Code: MyQueryError.name,
10292
Message: "oh no",
10393
Type: "Client",
10494
Prop2: 9999,
10595
},
10696
Type: "Client",
107-
Code: "MyQueryError",
97+
Code: MyQueryError.name,
10898
});
10999
});
110100
});

‎packages/core/src/submodules/protocols/json/AwsJsonRpcProtocol.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,8 @@ export abstract class AwsJsonRpcProtocol extends RpcProtocol {
110110
this.options.defaultNamespace,
111111
response,
112112
dataObject,
113-
metadata
113+
metadata,
114+
this.awsQueryCompatible ? this.mixin.findQueryCompatibleError : undefined
114115
);
115116

116117
const ns = NormalizedSchema.of(errorSchema);
@@ -120,8 +121,9 @@ export abstract class AwsJsonRpcProtocol extends RpcProtocol {
120121

121122
const output = {} as any;
122123
for (const [name, member] of ns.structIterator()) {
123-
const target = member.getMergedTraits().jsonName ?? name;
124-
output[name] = this.codec.createDeserializer().readObject(member, dataObject[target]);
124+
if (dataObject[name] != null) {
125+
output[name] = this.codec.createDeserializer().readObject(member, dataObject[name]);
126+
}
125127
}
126128

127129
if (this.awsQueryCompatible) {

‎packages/core/src/submodules/protocols/query/AwsQueryProtocol.ts‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -161,22 +161,16 @@ export class AwsQueryProtocol extends RpcProtocol {
161161
response,
162162
errorData,
163163
metadata,
164-
(registry: TypeRegistry, errorName: string) => {
165-
try {
166-
return registry.getSchema(errorName) as StaticErrorSchema;
167-
} catch (e) {
168-
return registry.find(
169-
(schema) => (NormalizedSchema.of(schema).getMergedTraits().awsQueryError as any)?.[0] === errorName
170-
) as StaticErrorSchema;
171-
}
172-
}
164+
this.mixin.findQueryCompatibleError
173165
);
174166

175167
const ns = NormalizedSchema.of(errorSchema);
176168
const ErrorCtor = TypeRegistry.for(errorSchema[1]).getErrorCtor(errorSchema) ?? Error;
177169
const exception = new ErrorCtor(message);
178170

179171
const output = {
172+
Type: errorData.Error.Type,
173+
Code: errorData.Error.Code,
180174
Error: errorData.Error,
181175
} as any;
182176

0 commit comments

Comments
 (0)