From 2e802c52ad177006e35b31b0c92b571f148cb575 Mon Sep 17 00:00:00 2001 From: George Fu Date: Wed, 3 Dec 2025 16:56:50 -0500 Subject: [PATCH] fix(core/protocols): awsQueryCompatibility error structuring --- .../test/e2e/CloudWatch.e2e.spec.ts | 20 +-- .../test/e2e/query-compatibility.e2e.spec.ts | 141 +++++++++++++++--- .../src/submodules/protocols/ProtocolLib.ts | 35 ++++- .../cbor/AwsSmithyRpcV2CborProtocol.spec.ts | 14 +- .../cbor/AwsSmithyRpcV2CborProtocol.ts | 15 +- .../protocols/json/AwsJsonRpcProtocol.spec.ts | 14 +- .../protocols/json/AwsJsonRpcProtocol.ts | 8 +- .../protocols/query/AwsQueryProtocol.ts | 12 +- 8 files changed, 185 insertions(+), 74 deletions(-) diff --git a/clients/client-cloudwatch/test/e2e/CloudWatch.e2e.spec.ts b/clients/client-cloudwatch/test/e2e/CloudWatch.e2e.spec.ts index 0bbad7991a15..60239ade488e 100644 --- a/clients/client-cloudwatch/test/e2e/CloudWatch.e2e.spec.ts +++ b/clients/client-cloudwatch/test/e2e/CloudWatch.e2e.spec.ts @@ -34,18 +34,10 @@ describe(CloudWatch.name, () => { }), }; - it("can make requests with AWS Query protocol", async () => { - const dashes = await cloudwatch.query.listDashboards(); - expect(dashes.DashboardEntries ?? []).toBeInstanceOf(Array); - }); - - it("can make requests with Smithy RPCv2 CBOR protocol", async () => { - const dashes = await cloudwatch.cbor.listDashboards(); - expect(dashes.DashboardEntries ?? []).toBeInstanceOf(Array); - }); - - it("can make requests with AWS JSON RPC protocol", async () => { - const dashes = await cloudwatch.json.listDashboards(); - expect(dashes.DashboardEntries ?? []).toBeInstanceOf(Array); - }); + for (const client of Object.values(cloudwatch)) { + it(`can make requests with ${client.config.protocol.constructor.name}`, async () => { + const dashes = await cloudwatch.query.listDashboards(); + expect(dashes.DashboardEntries ?? []).toBeInstanceOf(Array); + }); + } }); diff --git a/clients/client-cloudwatch/test/e2e/query-compatibility.e2e.spec.ts b/clients/client-cloudwatch/test/e2e/query-compatibility.e2e.spec.ts index ec05f43f2877..2fa565289eeb 100644 --- a/clients/client-cloudwatch/test/e2e/query-compatibility.e2e.spec.ts +++ b/clients/client-cloudwatch/test/e2e/query-compatibility.e2e.spec.ts @@ -1,25 +1,132 @@ -import { CloudWatchClient, GetDashboardCommand } from "@aws-sdk/client-cloudwatch"; -import { beforeAll, describe, expect, test as it } from "vitest"; +import { + CloudWatchClient, + CloudWatchServiceException, + GetDashboardCommand, + GetInsightRuleReportCommand, + MissingRequiredParameterException, + PutMetricAlarmCommand, + ResourceNotFound, +} from "@aws-sdk/client-cloudwatch"; +import { AwsJson1_0Protocol, AwsQueryProtocol, AwsSmithyRpcV2CborProtocol } from "@aws-sdk/core"; +import { describe, expect, test as it } from "vitest"; describe("CloudWatch Query Compatibility E2E", () => { - let client: CloudWatchClient; - - beforeAll(async () => { - client = new CloudWatchClient({ + const cloudwatch = { + cbor: new CloudWatchClient({ + region: "us-west-2", + protocol: new AwsSmithyRpcV2CborProtocol({ + defaultNamespace: "com.amazonaws.cloudwatch", + awsQueryCompatible: true, + }), + }), + query: new CloudWatchClient({ + region: "us-west-2", + protocol: new AwsQueryProtocol({ + defaultNamespace: "com.amazonaws.cloudwatch", + xmlNamespace: "http://monitoring.amazonaws.com/doc/2010-08-01/", + version: "2010-08-01", + }), + }), + json: new CloudWatchClient({ region: "us-west-2", + protocol: new AwsJson1_0Protocol({ + defaultNamespace: "com.amazonaws.cloudwatch", + serviceTarget: "GraniteServiceVersion20100801", + awsQueryCompatible: true, + }), + }), + }; + + for (const client of Object.values(cloudwatch)) { + const ctorName = client.config.protocol.constructor.name; + + it(`resolve errors with the query compat header and not the __type field (${ctorName})`, async () => { + const error = await client + .send( + new GetDashboardCommand({ + DashboardName: "does-not-exist", + }) + ) + .catch((_) => _); + + expect(error).toBeInstanceOf(Error); + expect(error).toBeInstanceOf(CloudWatchServiceException); + expect(error).toBeInstanceOf(ResourceNotFound); + + expect(error.constructor.prototype.name).toBe("Error"); + expect(error.constructor.name).toBe("ResourceNotFound"); + expect(error.name).toBe("ResourceNotFound"); + + expect(error.message).toBe("Dashboard does-not-exist does not exist"); + expect(error.Type).toEqual("Sender"); + expect(error.Code).toEqual("ResourceNotFound"); + expect(error.Error).toEqual({ + Type: "Sender", + Code: "ResourceNotFound", + Message: "Dashboard does-not-exist does not exist", + }); + expect(error.$metadata.httpStatusCode).toBe(404); }); - }); - it("AmbiguousErrorResolution", async () => { - const command = new GetDashboardCommand({ - DashboardName: "foo", + it(`have consistent error structure (modeled title-case 'Message') ${ctorName}`, async () => { + const error = await client + .send( + new PutMetricAlarmCommand({ + AlarmName: "", + ComparisonOperator: "GreaterThanThreshold", + EvaluationPeriods: 5, + MetricName: "CPUUtilization", + Namespace: "AWS/EC2", + Period: 60, + Statistic: "Average", + Threshold: 50.0, + }) + ) + .catch((_) => _); + + const msg = + `1 validation error detected: Value '' at 'alarmName' failed to satisfy ` + + `constraint: Member must have length greater than or equal to 1`; + + expect(error).toBeInstanceOf(Error); + expect(error).toBeInstanceOf(CloudWatchServiceException); + + expect(error.constructor.prototype.name).toBe("Error"); + expect(error.constructor.name).toBe("CloudWatchServiceException"); + expect(error.name).toBe("ValidationError"); + + expect(error.message).toBe(msg); + expect(error.Type).toEqual("Sender"); + expect(error.Code).toEqual("ValidationError"); + expect(error.Error).toEqual({ + Type: "Sender", + Code: "ValidationError", + Message: msg, + }); + expect(error.$metadata.httpStatusCode).toBe(400); }); - try { - await client.send(command); - fail("Expected ResourceNotFound error"); - } catch (error: any) { - expect(error.name).toBe("ResourceNotFound"); - } - }); + it(`have consistent error structure (modeled lowercase 'message') ${ctorName}`, async () => { + const error = await client.send(new GetInsightRuleReportCommand({} as any)).catch((_) => _); + const msg = `MISSING_RULE_NAME: The RuleName parameter must be present.`; + + expect(error).toBeInstanceOf(Error); + expect(error).toBeInstanceOf(CloudWatchServiceException); + expect(error).toBeInstanceOf(MissingRequiredParameterException); + + expect(error.constructor.prototype.name).toBe("Error"); + expect(error.constructor.name).toBe("MissingRequiredParameterException"); + expect(error.name).toBe("MissingRequiredParameterException"); + + expect(error.message).toBe(msg); + expect(error.Type).toEqual("Sender"); + expect(error.Code).toEqual("MissingParameter"); + expect(error.Error).toEqual({ + Type: "Sender", + Code: "MissingParameter", + Message: msg, + }); + expect(error.$metadata.httpStatusCode).toBe(400); + }); + } }); diff --git a/packages/core/src/submodules/protocols/ProtocolLib.ts b/packages/core/src/submodules/protocols/ProtocolLib.ts index 719f14b51de6..e1fd3869ac2e 100644 --- a/packages/core/src/submodules/protocols/ProtocolLib.ts +++ b/packages/core/src/submodules/protocols/ProtocolLib.ts @@ -110,11 +110,23 @@ export class ProtocolLib { ): E { if (this.queryCompat) { const msg = (exception as any).Message ?? additions.Message; - const error = decorateServiceException(exception, additions); + const error: any = decorateServiceException(exception, additions); if (msg) { - (error as any).Message = msg; - (error as any).message = msg; + error.message = msg; } + + error.Error = { + ...error.Error, + Type: error.Error.Type, + Code: error.Error.Code, + Message: error.Error.message ?? error.Error.Message ?? msg, + }; + + const reqId = error.$metadata.requestId; + if (reqId) { + error.RequestId = reqId; + } + return error; } return decorateServiceException(exception, additions); @@ -138,7 +150,7 @@ export class ProtocolLib { } as any; Object.assign(output, Error); for (const [k, v] of entries) { - Error[k] = v; + Error[k === "message" ? "Message" : k] = v; } delete Error.__type; output.Error = Error; @@ -161,4 +173,19 @@ export class ProtocolLib { errorData.Code = queryCompatErrorData.Code; } } + + /** + * Finds the canonical modeled error using the awsQueryError alias. + * @param registry - service error registry. + * @param errorName - awsQueryError name or regular qualified shapeId. + */ + public findQueryCompatibleError(registry: TypeRegistry, errorName: string) { + try { + return registry.getSchema(errorName) as StaticErrorSchema; + } catch (e) { + return registry.find( + (schema) => (NormalizedSchema.of(schema).getMergedTraits().awsQueryError as any)?.[0] === errorName + ) as StaticErrorSchema; + } + } } diff --git a/packages/core/src/submodules/protocols/cbor/AwsSmithyRpcV2CborProtocol.spec.ts b/packages/core/src/submodules/protocols/cbor/AwsSmithyRpcV2CborProtocol.spec.ts index 3a15613d08cb..e3b9512250c1 100644 --- a/packages/core/src/submodules/protocols/cbor/AwsSmithyRpcV2CborProtocol.spec.ts +++ b/packages/core/src/submodules/protocols/cbor/AwsSmithyRpcV2CborProtocol.spec.ts @@ -60,28 +60,18 @@ describe(AwsSmithyRpcV2CborProtocol.name, () => { // set by deserializer middleware, not Protocol. expect(error.$response).toEqual(undefined); - expect(error.Code).toEqual(MyQueryError.name); - expect(error.Error.Code).toEqual(MyQueryError.name); - - expect(error.Message).toEqual("oh no"); - expect(error.Prop2).toEqual(9999); - - expect(error.Error.Message).toEqual("oh no"); - expect(error.Error.Prop2).toEqual(9999); - expect(error).toMatchObject({ $fault: "client", - Message: "oh no", message: "oh no", Prop2: 9999, Error: { - Code: "MyQueryError", + Code: MyQueryError.name, Message: "oh no", Type: "Client", Prop2: 9999, }, Type: "Client", - Code: "MyQueryError", + Code: MyQueryError.name, }); }); diff --git a/packages/core/src/submodules/protocols/cbor/AwsSmithyRpcV2CborProtocol.ts b/packages/core/src/submodules/protocols/cbor/AwsSmithyRpcV2CborProtocol.ts index 90c087b774db..eadea1415383 100644 --- a/packages/core/src/submodules/protocols/cbor/AwsSmithyRpcV2CborProtocol.ts +++ b/packages/core/src/submodules/protocols/cbor/AwsSmithyRpcV2CborProtocol.ts @@ -61,14 +61,21 @@ export class AwsSmithyRpcV2CborProtocol extends SmithyRpcV2CborProtocol { if (this.awsQueryCompatible) { this.mixin.setQueryCompatError(dataObject, response); } - const errorName = loadSmithyRpcV2CborErrorCode(response, dataObject) ?? "Unknown"; + const errorName = (() => { + const compatHeader = response.headers["x-amzn-query-error"]; + if (compatHeader && this.awsQueryCompatible) { + return compatHeader.split(";")[0]; + } + return loadSmithyRpcV2CborErrorCode(response, dataObject) ?? "Unknown"; + })(); const { errorSchema, errorMetadata } = await this.mixin.getErrorSchemaOrThrowBaseException( errorName, this.options.defaultNamespace, response, dataObject, - metadata + metadata, + this.awsQueryCompatible ? this.mixin.findQueryCompatibleError : undefined ); const ns = NormalizedSchema.of(errorSchema); @@ -78,7 +85,9 @@ export class AwsSmithyRpcV2CborProtocol extends SmithyRpcV2CborProtocol { const output = {} as any; for (const [name, member] of ns.structIterator()) { - output[name] = this.deserializer.readValue(member, dataObject[name]); + if (dataObject[name] != null) { + output[name] = this.deserializer.readValue(member, dataObject[name]); + } } if (this.awsQueryCompatible) { diff --git a/packages/core/src/submodules/protocols/json/AwsJsonRpcProtocol.spec.ts b/packages/core/src/submodules/protocols/json/AwsJsonRpcProtocol.spec.ts index 1ce4f8dd676f..4adcafe41156 100644 --- a/packages/core/src/submodules/protocols/json/AwsJsonRpcProtocol.spec.ts +++ b/packages/core/src/submodules/protocols/json/AwsJsonRpcProtocol.spec.ts @@ -83,28 +83,18 @@ describe(AwsJsonRpcProtocol.name, () => { // set by deserializer middleware, not protocol expect(error.$response).toEqual(undefined); - expect(error.Code).toEqual(MyQueryError.name); - expect(error.Error.Code).toEqual(MyQueryError.name); - - expect(error.Message).toEqual("oh no"); - expect(error.Prop2).toEqual(9999); - - expect(error.Error.Message).toEqual("oh no"); - expect(error.Error.Prop2).toEqual(9999); - expect(error).toMatchObject({ $fault: "client", - Message: "oh no", message: "oh no", Prop2: 9999, Error: { - Code: "MyQueryError", + Code: MyQueryError.name, Message: "oh no", Type: "Client", Prop2: 9999, }, Type: "Client", - Code: "MyQueryError", + Code: MyQueryError.name, }); }); }); diff --git a/packages/core/src/submodules/protocols/json/AwsJsonRpcProtocol.ts b/packages/core/src/submodules/protocols/json/AwsJsonRpcProtocol.ts index ece94fe900cd..8471d606c3e4 100644 --- a/packages/core/src/submodules/protocols/json/AwsJsonRpcProtocol.ts +++ b/packages/core/src/submodules/protocols/json/AwsJsonRpcProtocol.ts @@ -110,7 +110,8 @@ export abstract class AwsJsonRpcProtocol extends RpcProtocol { this.options.defaultNamespace, response, dataObject, - metadata + metadata, + this.awsQueryCompatible ? this.mixin.findQueryCompatibleError : undefined ); const ns = NormalizedSchema.of(errorSchema); @@ -120,8 +121,9 @@ export abstract class AwsJsonRpcProtocol extends RpcProtocol { const output = {} as any; for (const [name, member] of ns.structIterator()) { - const target = member.getMergedTraits().jsonName ?? name; - output[name] = this.codec.createDeserializer().readObject(member, dataObject[target]); + if (dataObject[name] != null) { + output[name] = this.codec.createDeserializer().readObject(member, dataObject[name]); + } } if (this.awsQueryCompatible) { diff --git a/packages/core/src/submodules/protocols/query/AwsQueryProtocol.ts b/packages/core/src/submodules/protocols/query/AwsQueryProtocol.ts index 9426ed3863ae..7b5abd92ee5b 100644 --- a/packages/core/src/submodules/protocols/query/AwsQueryProtocol.ts +++ b/packages/core/src/submodules/protocols/query/AwsQueryProtocol.ts @@ -161,15 +161,7 @@ export class AwsQueryProtocol extends RpcProtocol { response, errorData, metadata, - (registry: TypeRegistry, errorName: string) => { - try { - return registry.getSchema(errorName) as StaticErrorSchema; - } catch (e) { - return registry.find( - (schema) => (NormalizedSchema.of(schema).getMergedTraits().awsQueryError as any)?.[0] === errorName - ) as StaticErrorSchema; - } - } + this.mixin.findQueryCompatibleError ); const ns = NormalizedSchema.of(errorSchema); @@ -177,6 +169,8 @@ export class AwsQueryProtocol extends RpcProtocol { const exception = new ErrorCtor(message); const output = { + Type: errorData.Error.Type, + Code: errorData.Error.Code, Error: errorData.Error, } as any;