From 1212b43e668c470796d9b5b56cd5c86fe3c70ffb Mon Sep 17 00:00:00 2001 From: Alex Woods Date: Wed, 21 Aug 2024 11:18:05 -0700 Subject: [PATCH] transport agnostic error test fixes + protocol fixes. --- .../rails_json/lib/rails_json/parsers.rb | 8 ++-- .../rails_json/spec/protocol_spec.rb | 6 +-- .../rpcv2_cbor/lib/rpcv2_cbor/parsers.rb | 37 ++++++++++--------- .../rpcv2_cbor/spec/protocol_spec.rb | 6 +-- .../white_label/spec/errors_spec.rb | 5 ++- .../spec/event_stream_stubs_spec.rb | 2 +- .../integration-specs/errors_spec.rb | 5 ++- .../event_stream_stubs_spec.rb | 2 +- .../generators/HttpProtocolTestGenerator.java | 2 +- .../generators/RestParserGeneratorBase.java | 6 +-- .../rpcv2cbor/generators/ParserGenerator.java | 13 ++++--- .../hearth/event_stream/handler_base_spec.rb | 7 ++-- .../spec/hearth/http/error_inspector_spec.rb | 3 +- hearth/spec/hearth/http/error_parser_spec.rb | 14 +++++-- 14 files changed, 65 insertions(+), 51 deletions(-) diff --git a/codegen/projections/rails_json/lib/rails_json/parsers.rb b/codegen/projections/rails_json/lib/rails_json/parsers.rb index 4c81e3354..f50fb0147 100644 --- a/codegen/projections/rails_json/lib/rails_json/parsers.rb +++ b/codegen/projections/rails_json/lib/rails_json/parsers.rb @@ -30,13 +30,13 @@ def self.parse(list) # Error Parser for ComplexError class ComplexError - def self.parse(http_resp) + def self.parse(http_resp, **kwargs) data = Types::ComplexError.new data.header = http_resp.headers['X-Header'] map = Hearth::JSON.parse(http_resp.body.read) data.top_level = map['top_level'] unless map['top_level'].nil? data.nested = Parsers::ComplexNestedErrorData.parse(map['nested']) unless map['nested'].nil? - data + Errors::ComplexError.new(data: data, **kwargs) end end @@ -499,11 +499,11 @@ def self.parse(list) # Error Parser for InvalidGreeting class InvalidGreeting - def self.parse(http_resp) + def self.parse(http_resp, **kwargs) data = Types::InvalidGreeting.new map = Hearth::JSON.parse(http_resp.body.read) data.message = map['message'] unless map['message'].nil? - data + Errors::InvalidGreeting.new(data: data, **kwargs) end end diff --git a/codegen/projections/rails_json/spec/protocol_spec.rb b/codegen/projections/rails_json/spec/protocol_spec.rb index b8dfe59f7..714b9f34a 100644 --- a/codegen/projections/rails_json/spec/protocol_spec.rb +++ b/codegen/projections/rails_json/spec/protocol_spec.rb @@ -1349,7 +1349,7 @@ module RailsJson begin output = client.greeting_with_errors({}, auth_resolver: Hearth::AnonymousAuthResolver.new) rescue Errors::InvalidGreeting => e - expect(e.http_status).to eq(400) + expect(e.metadata[:http_resp].status).to eq(400) expect(e.data.to_h).to eq({ message: "Hi" }) @@ -1401,7 +1401,7 @@ module RailsJson begin output = client.greeting_with_errors({}, auth_resolver: Hearth::AnonymousAuthResolver.new) rescue Errors::ComplexError => e - expect(e.http_status).to eq(403) + expect(e.metadata[:http_resp].status).to eq(403) expect(e.data.to_h).to eq({ header: "Header", top_level: "Top level", @@ -1436,7 +1436,7 @@ module RailsJson begin output = client.greeting_with_errors({}, auth_resolver: Hearth::AnonymousAuthResolver.new) rescue Errors::ComplexError => e - expect(e.http_status).to eq(403) + expect(e.metadata[:http_resp].status).to eq(403) expect(e.data.to_h).to eq({}) end end diff --git a/codegen/projections/rpcv2_cbor/lib/rpcv2_cbor/parsers.rb b/codegen/projections/rpcv2_cbor/lib/rpcv2_cbor/parsers.rb index 682e9f229..fe2285760 100644 --- a/codegen/projections/rpcv2_cbor/lib/rpcv2_cbor/parsers.rb +++ b/codegen/projections/rpcv2_cbor/lib/rpcv2_cbor/parsers.rb @@ -29,14 +29,15 @@ def self.parse(list) # Error Parser for ComplexError class ComplexError - def self.parse(http_resp) + def self.parse(http_resp, **kwargs) data = Types::ComplexError.new body = http_resp.body.read - return data if body.empty? - map = Hearth::CBOR.decode(body) - data.top_level = map['TopLevel'] - data.nested = (ComplexNestedErrorData.parse(map['Nested']) unless map['Nested'].nil?) - data + unless body.empty? + map = Hearth::CBOR.decode(body) + data.top_level = map['TopLevel'] + data.nested = (ComplexNestedErrorData.parse(map['Nested']) unless map['Nested'].nil?) + end + Errors::ComplexError.new(data: data, **kwargs) end end @@ -175,13 +176,14 @@ def self.parse(list) # Error Parser for InvalidGreeting class InvalidGreeting - def self.parse(http_resp) + def self.parse(http_resp, **kwargs) data = Types::InvalidGreeting.new body = http_resp.body.read - return data if body.empty? - map = Hearth::CBOR.decode(body) - data.message = map['Message'] - data + unless body.empty? + map = Hearth::CBOR.decode(body) + data.message = map['Message'] + end + Errors::InvalidGreeting.new(data: data, **kwargs) end end @@ -477,14 +479,15 @@ def self.parse(list) # Error Parser for ValidationException class ValidationException - def self.parse(http_resp) + def self.parse(http_resp, **kwargs) data = Types::ValidationException.new body = http_resp.body.read - return data if body.empty? - map = Hearth::CBOR.decode(body) - data.message = map['message'] - data.field_list = (ValidationExceptionFieldList.parse(map['fieldList']) unless map['fieldList'].nil?) - data + unless body.empty? + map = Hearth::CBOR.decode(body) + data.message = map['message'] + data.field_list = (ValidationExceptionFieldList.parse(map['fieldList']) unless map['fieldList'].nil?) + end + Errors::ValidationException.new(data: data, **kwargs) end end diff --git a/codegen/projections/rpcv2_cbor/spec/protocol_spec.rb b/codegen/projections/rpcv2_cbor/spec/protocol_spec.rb index 3d9666385..89505909a 100644 --- a/codegen/projections/rpcv2_cbor/spec/protocol_spec.rb +++ b/codegen/projections/rpcv2_cbor/spec/protocol_spec.rb @@ -355,7 +355,7 @@ module Rpcv2Cbor begin output = client.greeting_with_errors({}, auth_resolver: Hearth::AnonymousAuthResolver.new) rescue Errors::InvalidGreeting => e - expect(e.http_status).to eq(400) + expect(e.metadata[:http_resp].status).to eq(400) expect(e.data.to_h).to eq({ message: "Hi" }) @@ -399,7 +399,7 @@ module Rpcv2Cbor begin output = client.greeting_with_errors({}, auth_resolver: Hearth::AnonymousAuthResolver.new) rescue Errors::ComplexError => e - expect(e.http_status).to eq(400) + expect(e.metadata[:http_resp].status).to eq(400) expect(e.data.to_h).to eq({ top_level: "Top level", nested: { @@ -433,7 +433,7 @@ module Rpcv2Cbor begin output = client.greeting_with_errors({}, auth_resolver: Hearth::AnonymousAuthResolver.new) rescue Errors::ComplexError => e - expect(e.http_status).to eq(400) + expect(e.metadata[:http_resp].status).to eq(400) expect(e.data.to_h).to eq({}) end end diff --git a/codegen/projections/white_label/spec/errors_spec.rb b/codegen/projections/white_label/spec/errors_spec.rb index 884a4bab4..773d4def4 100644 --- a/codegen/projections/white_label/spec/errors_spec.rb +++ b/codegen/projections/white_label/spec/errors_spec.rb @@ -51,7 +51,8 @@ module Errors describe ClientError do it 'is retryable without throttling' do - error = ClientError.new(data: nil, metadata: {}, + error = ClientError.new(data: Types::ClientError.new, + metadata: {}, error_code: 'error') expect(error.retryable?).to be true expect(error.throttling?).to be false @@ -60,7 +61,7 @@ module Errors describe ServerError do it 'is retryable with throttling' do - error = ServerError.new(data: nil, metadata: {}, + error = ServerError.new(data: Types::ServerError.new, metadata: {}, error_code: 'error') expect(error.retryable?).to be true expect(error.throttling?).to be true diff --git a/codegen/projections/white_label/spec/event_stream_stubs_spec.rb b/codegen/projections/white_label/spec/event_stream_stubs_spec.rb index 3f227e0ad..6e0f63db2 100644 --- a/codegen/projections/white_label/spec/event_stream_stubs_spec.rb +++ b/codegen/projections/white_label/spec/event_stream_stubs_spec.rb @@ -287,7 +287,7 @@ subject.stub_responses( :start_event_stream, WhiteLabel::Errors::ClientError.new( - data: nil, + data: Types::ClientError.new, error_code: 'ClientError' ) ) diff --git a/codegen/smithy-ruby-codegen-test/integration-specs/errors_spec.rb b/codegen/smithy-ruby-codegen-test/integration-specs/errors_spec.rb index 884a4bab4..773d4def4 100644 --- a/codegen/smithy-ruby-codegen-test/integration-specs/errors_spec.rb +++ b/codegen/smithy-ruby-codegen-test/integration-specs/errors_spec.rb @@ -51,7 +51,8 @@ module Errors describe ClientError do it 'is retryable without throttling' do - error = ClientError.new(data: nil, metadata: {}, + error = ClientError.new(data: Types::ClientError.new, + metadata: {}, error_code: 'error') expect(error.retryable?).to be true expect(error.throttling?).to be false @@ -60,7 +61,7 @@ module Errors describe ServerError do it 'is retryable with throttling' do - error = ServerError.new(data: nil, metadata: {}, + error = ServerError.new(data: Types::ServerError.new, metadata: {}, error_code: 'error') expect(error.retryable?).to be true expect(error.throttling?).to be true diff --git a/codegen/smithy-ruby-codegen-test/integration-specs/event_stream_stubs_spec.rb b/codegen/smithy-ruby-codegen-test/integration-specs/event_stream_stubs_spec.rb index 3f227e0ad..6e0f63db2 100644 --- a/codegen/smithy-ruby-codegen-test/integration-specs/event_stream_stubs_spec.rb +++ b/codegen/smithy-ruby-codegen-test/integration-specs/event_stream_stubs_spec.rb @@ -287,7 +287,7 @@ subject.stub_responses( :start_event_stream, WhiteLabel::Errors::ClientError.new( - data: nil, + data: Types::ClientError.new, error_code: 'ClientError' ) ) diff --git a/codegen/smithy-ruby-codegen/src/main/java/software/amazon/smithy/ruby/codegen/generators/HttpProtocolTestGenerator.java b/codegen/smithy-ruby-codegen/src/main/java/software/amazon/smithy/ruby/codegen/generators/HttpProtocolTestGenerator.java index c212b3fc3..512a8fe5a 100644 --- a/codegen/smithy-ruby-codegen/src/main/java/software/amazon/smithy/ruby/codegen/generators/HttpProtocolTestGenerator.java +++ b/codegen/smithy-ruby-codegen/src/main/java/software/amazon/smithy/ruby/codegen/generators/HttpProtocolTestGenerator.java @@ -299,7 +299,7 @@ private void renderErrorTests(OperationShape operation) { .dedent() .write("rescue Errors::$L => e", error.getId().getName()) .indent() - .write("expect(e.http_status).to eq($L)", testCase.getCode()) + .write("expect(e.metadata[:http_resp].status).to eq($L)", testCase.getCode()) .write("expect(e.data.to_h).to eq($L)", getRubyHashFromParams(error, testCase.getParams())) .closeBlock("end") diff --git a/codegen/smithy-ruby-codegen/src/main/java/software/amazon/smithy/ruby/codegen/generators/RestParserGeneratorBase.java b/codegen/smithy-ruby-codegen/src/main/java/software/amazon/smithy/ruby/codegen/generators/RestParserGeneratorBase.java index 73ade8e9b..cd5a246a2 100644 --- a/codegen/smithy-ruby-codegen/src/main/java/software/amazon/smithy/ruby/codegen/generators/RestParserGeneratorBase.java +++ b/codegen/smithy-ruby-codegen/src/main/java/software/amazon/smithy/ruby/codegen/generators/RestParserGeneratorBase.java @@ -135,12 +135,12 @@ protected void renderOperationParseMethod(OperationShape operation, Shape output @Override protected void renderErrorParseMethod(Shape s) { writer - .openBlock("def self.parse(http_resp)") + .openBlock("def self.parse(http_resp, **kwargs)") .write("data = $T.new", context.symbolProvider().toSymbol(s)) .call(() -> renderHeaderParsers(s)) - .call(() -> renderPrefixHeaderParsers(s)) .call(() -> renderOperationBodyParser(s)) - .write("data") + .write("Errors::$L.new(data: data, **kwargs)", + context.symbolProvider().toSymbol(s).getName()) .closeBlock("end"); } diff --git a/codegen/smithy-ruby-codegen/src/main/java/software/amazon/smithy/ruby/codegen/protocol/rpcv2cbor/generators/ParserGenerator.java b/codegen/smithy-ruby-codegen/src/main/java/software/amazon/smithy/ruby/codegen/protocol/rpcv2cbor/generators/ParserGenerator.java index c85a7e3df..21347d674 100644 --- a/codegen/smithy-ruby-codegen/src/main/java/software/amazon/smithy/ruby/codegen/protocol/rpcv2cbor/generators/ParserGenerator.java +++ b/codegen/smithy-ruby-codegen/src/main/java/software/amazon/smithy/ruby/codegen/protocol/rpcv2cbor/generators/ParserGenerator.java @@ -152,14 +152,15 @@ protected void renderOperationParseMethod(OperationShape operation, Shape output @Override protected void renderErrorParseMethod(Shape shape) { writer - .openBlock("def self.parse(http_resp)") + .openBlock("def self.parse(http_resp, **kwargs)") .write("data = $T.new", context.symbolProvider().toSymbol(shape)) .write("body = http_resp.body.read") - .write("return data if body.empty?") - .write("map = $T.decode(body)", Hearth.CBOR); - renderMemberParsers(shape); - writer - .write("data") + .openBlock("unless body.empty?") + .write("map = $T.decode(body)", Hearth.CBOR) + .call(() -> renderMemberParsers(shape)) + .closeBlock("end") + .write("Errors::$L.new(data: data, **kwargs)", + context.symbolProvider().toSymbol(shape).getName()) .closeBlock("end"); } diff --git a/hearth/spec/hearth/event_stream/handler_base_spec.rb b/hearth/spec/hearth/event_stream/handler_base_spec.rb index 2a1456b57..cb494117d 100644 --- a/hearth/spec/hearth/event_stream/handler_base_spec.rb +++ b/hearth/spec/hearth/event_stream/handler_base_spec.rb @@ -31,6 +31,7 @@ module EventStream Class.new(HandlerBase) do def parse_event(_, _); end def parse_error_event(_); end + def parse_exception_event(_, _); end end.new end @@ -87,9 +88,9 @@ def parse_error_event(_); end let(:exception_class) { Class.new } let(:exception_event) { exception_class.new } - it 'calls registered MyException handlers' do - subject.send(:on, exception_class, handler1) - expect(subject).to receive(:parse_event) + it 'calls registered error handlers' do + subject.on_error(&handler1) + expect(subject).to receive(:parse_exception_event) .with(exception_type, message) .and_return(exception_event) expect(handler1).to receive(:call).with(exception_event) diff --git a/hearth/spec/hearth/http/error_inspector_spec.rb b/hearth/spec/hearth/http/error_inspector_spec.rb index 3cc2e76fe..78afe21dd 100644 --- a/hearth/spec/hearth/http/error_inspector_spec.rb +++ b/hearth/spec/hearth/http/error_inspector_spec.rb @@ -12,8 +12,7 @@ module HTTP let(:message) { 'message' } let(:error) do - Hearth::HTTP::ApiError.new( - http_resp: http_resp, + Hearth::ApiError.new( error_code: 'error_code', metadata: {}, message: message diff --git a/hearth/spec/hearth/http/error_parser_spec.rb b/hearth/spec/hearth/http/error_parser_spec.rb index 3902e3f47..0a00de40a 100644 --- a/hearth/spec/hearth/http/error_parser_spec.rb +++ b/hearth/spec/hearth/http/error_parser_spec.rb @@ -7,7 +7,7 @@ def self.error_code(_http_resp) nil end - class ApiError < Hearth::HTTP::ApiError; end + class ApiError < Hearth::ApiError; end class ApiRedirectError < ApiError def initialize(location:, **kwargs) @@ -23,12 +23,20 @@ class ApiClientError < ApiError; end class ApiServerError < ApiError; end class ModeledError < ApiClientError; end + + module Parsers + class ModeledError + def self.parse(_http_resp, **_kwargs) + TestErrors::ModeledError.new(error_code: 'ModeledError') + end + end + end end describe ErrorParser do let(:error_module) { TestErrors } let(:success_status) { 200 } - let(:errors) { [TestErrors::ModeledError] } + let(:error_parsers) { [TestErrors::Parsers::ModeledError] } let(:resp_status) { 200 } let(:fields) { Fields.new } @@ -39,7 +47,7 @@ class ModeledError < ApiClientError; end Hearth::HTTP::ErrorParser.new( error_module: error_module, success_status: success_status, - errors: errors + error_parsers: error_parsers ) end