From 4a69d1ac7e8ce6c3ab10cdfcd70f84552e4334b9 Mon Sep 17 00:00:00 2001 From: Juli Tera Date: Fri, 24 Jul 2026 11:21:07 -0700 Subject: [PATCH 01/15] Add a protocols registry to generated clients --- gems/smithy/lib/smithy/templates/client/client.erb | 7 +++++++ gems/smithy/lib/smithy/views/client/client.rb | 13 +++++++++++++ gems/smithy/lib/smithy/weld.rb | 8 ++++++++ 3 files changed, 28 insertions(+) diff --git a/gems/smithy/lib/smithy/templates/client/client.erb b/gems/smithy/lib/smithy/templates/client/client.erb index 27fb67497..b1c7bb53c 100644 --- a/gems/smithy/lib/smithy/templates/client/client.erb +++ b/gems/smithy/lib/smithy/templates/client/client.erb @@ -143,6 +143,13 @@ module <%= module_name %> def errors_module Errors end + + # @api private + def protocols +<% protocols.each do |line| -%> + <%= line %> +<% end -%> + end end end end diff --git a/gems/smithy/lib/smithy/views/client/client.rb b/gems/smithy/lib/smithy/views/client/client.rb index b401f1aef..838fcdc6b 100644 --- a/gems/smithy/lib/smithy/views/client/client.rb +++ b/gems/smithy/lib/smithy/views/client/client.rb @@ -39,6 +39,19 @@ def add_plugins @plugins.map(&:class_name) end + def protocols + weld_protocols = @plan.welds.map(&:add_protocols).reduce({}, :merge) + return ['{}'] if weld_protocols.empty? + + lines = ['{'] + weld_protocols.each do |name, protocol_class| + lines << " #{name}: #{protocol_class}," + end + lines.last.chomp!(',') if lines.last.end_with?(',') + lines << '}' + lines + end + def docstrings options = @plugins.map(&:options).flatten.sort_by(&:name) documentation = {} diff --git a/gems/smithy/lib/smithy/weld.rb b/gems/smithy/lib/smithy/weld.rb index 3f2779104..10ded5121 100644 --- a/gems/smithy/lib/smithy/weld.rb +++ b/gems/smithy/lib/smithy/weld.rb @@ -95,6 +95,14 @@ def remove_plugins [] end + # Called when constructing the client. Any protocols defined here will be + # merged into the client's protocol registry. The key is the protocol name + # (a Symbol) and the value is the fully qualified protocol class. + # @return [Hash] a mapping of protocol names to protocol classes. + def add_protocols + {} + end + # Called when creating the auth resolver and auth schemes. The value is the # absolute shape id of the auth scheme trait. def add_auth_schemes From b9292c82edba0cda753cf24705bdb52c7b7dde59 Mon Sep 17 00:00:00 2001 From: Juli Tera Date: Fri, 24 Jul 2026 11:23:59 -0700 Subject: [PATCH 02/15] Fold the protocol interface into NoOpProtocol Drop the base class in favor of a duck-typed interface documented on the standalone NoOpProtocol. Parse handlers now get context, so a protocol can reach request headers and operation errors. --- gems/smithy-client/lib/smithy-client.rb | 1 - .../lib/smithy-client/no_op_protocol.rb | 45 +++++++++++++---- .../lib/smithy-client/plugins/protocol.rb | 8 ++-- .../lib/smithy-client/protocol.rb | 31 ------------ .../sig/smithy-client/interfaces.rbs | 8 ++++ .../smithy-client/plugins/protocol_spec.rb | 48 ++++++++----------- 6 files changed, 69 insertions(+), 72 deletions(-) delete mode 100644 gems/smithy-client/lib/smithy-client/protocol.rb diff --git a/gems/smithy-client/lib/smithy-client.rb b/gems/smithy-client/lib/smithy-client.rb index 2a4f8bb04..b18f918ec 100644 --- a/gems/smithy-client/lib/smithy-client.rb +++ b/gems/smithy-client/lib/smithy-client.rb @@ -32,7 +32,6 @@ require_relative 'smithy-client/param_validator' require_relative 'smithy-client/plugin' require_relative 'smithy-client/plugin_list' -require_relative 'smithy-client/protocol' require_relative 'smithy-client/no_op_protocol' require_relative 'smithy-client/retry' require_relative 'smithy-client/service_error' diff --git a/gems/smithy-client/lib/smithy-client/no_op_protocol.rb b/gems/smithy-client/lib/smithy-client/no_op_protocol.rb index 9fc15fc59..8c127f945 100644 --- a/gems/smithy-client/lib/smithy-client/no_op_protocol.rb +++ b/gems/smithy-client/lib/smithy-client/no_op_protocol.rb @@ -1,19 +1,48 @@ # frozen_string_literal: true -require_relative 'protocol' - module Smithy module Client - # Default protocol used when no protocol is registered for a client. All - # methods are no-ops, so build/parse handlers can delegate safely without - # a nil check. + # Default protocol used when no protocol is registered for a client. + # Also documents the protocol interface: a protocol serializes requests, + # deserializes responses, and builds stubbed responses for a specific + # wire format. A custom protocol passed via +Client.new(protocol:)+ is + # any object that responds to these methods. + # + # All methods are no-ops, so handlers and stubbing can delegate safely + # without a nil check. # @api private - class NoOpProtocol < Protocol + class NoOpProtocol + # Serialize the request into the wire format. + # @param [HandlerContext] _context def build_request(_context); end - def parse_data(_response); end + # Deserialize a successful response body. + # @param [HandlerContext] _context + # @return [Object, nil] the response data + def parse_data(_context); end + + # Deserialize an error response into the modeled error. Called on + # every response; must return nil when the response is not an error. + # @param [HandlerContext] _context + # @return [StandardError, nil] + def parse_error(_context); end + + # Build a stubbed HTTP response for the given output data. + # @param [Configuration] _config + # @param [Schema::OperationShape] _operation + # @param [Object] _data + # @return [Http::Response] + def stub_data(_config, _operation, _data) + Http::Response.new + end - def parse_error(_response); end + # Build a stubbed HTTP error response for the given error code. + # @param [Configuration] _config + # @param [String] _error_code + # @return [Http::Response] + def stub_error(_config, _error_code) + Http::Response.new + end end end end diff --git a/gems/smithy-client/lib/smithy-client/plugins/protocol.rb b/gems/smithy-client/lib/smithy-client/plugins/protocol.rb index 1eb371380..ff353b3a0 100644 --- a/gems/smithy-client/lib/smithy-client/plugins/protocol.rb +++ b/gems/smithy-client/lib/smithy-client/plugins/protocol.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -require_relative '../protocol' require_relative '../no_op_protocol' module Smithy @@ -28,12 +27,15 @@ def call(context) end end + # Parses the response after the send handler returns. Send handlers + # must fully signal the response body (signal_done) before returning, + # since parsing happens inline here rather than in a body callback. # @api private class ParseHandler < Handler def call(context) response = @handler.call(context) - response.error = context.config.protocol.parse_error(response) unless response.error - response.data = context.config.protocol.parse_data(response) unless response.error + response.error = context.config.protocol.parse_error(context) unless response.error + response.data = context.config.protocol.parse_data(context) unless response.error response end end diff --git a/gems/smithy-client/lib/smithy-client/protocol.rb b/gems/smithy-client/lib/smithy-client/protocol.rb deleted file mode 100644 index 5bbedb7a2..000000000 --- a/gems/smithy-client/lib/smithy-client/protocol.rb +++ /dev/null @@ -1,31 +0,0 @@ -# frozen_string_literal: true - -module Smithy - module Client - # Base class documenting the protocol interface. A protocol serializes - # requests and deserializes responses for a specific wire format. - # - # A custom protocol passed via +Client.new(protocol:)+ is any object that - # responds to these methods; it need not inherit from this class. - # @api private - class Protocol - # Serialize the request into the wire format. - # @param [Interceptor::Context] _context - def build_request(_context) - raise NotImplementedError - end - - # Deserialize a successful response body. - # @param [Response] _response - def parse_data(_response) - raise NotImplementedError - end - - # Deserialize an error response into the modeled error. - # @param [Response] _response - def parse_error(_response) - raise NotImplementedError - end - end - end -end diff --git a/gems/smithy-client/sig/smithy-client/interfaces.rbs b/gems/smithy-client/sig/smithy-client/interfaces.rbs index 9c76ca51a..a92444b14 100644 --- a/gems/smithy-client/sig/smithy-client/interfaces.rbs +++ b/gems/smithy-client/sig/smithy-client/interfaces.rbs @@ -10,5 +10,13 @@ module Smithy interface _ReadableIO def read: (int length, ?string outbuf) -> String end + + interface _Protocol + def build_request: (HandlerContext context) -> void + def parse_data: (HandlerContext context) -> untyped + def parse_error: (HandlerContext context) -> StandardError? + def stub_data: (untyped config, untyped operation, untyped data) -> Http::Response + def stub_error: (untyped config, String error_code) -> Http::Response + end end end \ No newline at end of file diff --git a/gems/smithy-client/spec/smithy-client/plugins/protocol_spec.rb b/gems/smithy-client/spec/smithy-client/plugins/protocol_spec.rb index 34615a6bf..1056e87fb 100644 --- a/gems/smithy-client/spec/smithy-client/plugins/protocol_spec.rb +++ b/gems/smithy-client/spec/smithy-client/plugins/protocol_spec.rb @@ -19,14 +19,17 @@ module Plugins let(:client_options) { { endpoint: 'https://example.com' } } let(:fake_protocol_class) do - Class.new(Client::Protocol) do + Class.new do def build_request(_context); end - def parse_data(_response); end - def parse_error(_response); end + def parse_data(_context); end + def parse_error(_context); end + def stub_data(_config, _operation, _data); end + def stub_error(_config, _error_code); end end end - # TODO: remove this test-only registry once the `protocols` weld is impl + # Override the generated registry with a controlled double so the + # plugin's resolution logic is tested in isolation from any real protocol. before do protocols = { rpc_v2_cbor: fake_protocol_class } client_class.define_singleton_method(:protocols) { protocols } @@ -77,42 +80,29 @@ def parse_error(_response); end end end - describe Client::Protocol do - subject(:protocol) { described_class.new } - - it 'raises NotImplementedError for #build_request' do - expect { protocol.build_request(double('context')) } - .to raise_error(NotImplementedError) - end - - it 'raises NotImplementedError for #parse_data' do - expect { protocol.parse_data(double('response')) } - .to raise_error(NotImplementedError) - end - - it 'raises NotImplementedError for #parse_error' do - expect { protocol.parse_error(double('response')) } - .to raise_error(NotImplementedError) - end - end - describe Client::NoOpProtocol do subject(:protocol) { described_class.new } - it 'is a Protocol' do - expect(protocol).to be_a(Client::Protocol) - end - it 'returns nil from #build_request without raising' do expect(protocol.build_request(double('context'))).to be_nil end it 'returns nil from #parse_data without raising' do - expect(protocol.parse_data(double('response'))).to be_nil + expect(protocol.parse_data(double('context'))).to be_nil end it 'returns nil from #parse_error without raising' do - expect(protocol.parse_error(double('response'))).to be_nil + expect(protocol.parse_error(double('context'))).to be_nil + end + + it 'returns an empty response from #stub_data' do + response = protocol.stub_data(double('config'), double('operation'), {}) + expect(response).to be_a(Http::Response) + end + + it 'returns an empty response from #stub_error' do + response = protocol.stub_error(double('config'), 'ErrorCode') + expect(response).to be_a(Http::Response) end end end From bddc3d25f793645c547841de9b7513ccbd9f5906 Mon Sep 17 00:00:00 2001 From: Juli Tera Date: Fri, 24 Jul 2026 11:24:55 -0700 Subject: [PATCH 03/15] Make CBOR the first consumer of the generic Protocol plugin One RpcV2Cbor object replaces the per-service plugin, its two handlers, and the separate stubber. Stubbing runs through config.protocol (the object doubles as its own stubber). AWS protocols follow in a separate PR, so their stubbing stays red on version-4 until then. --- gems/smithy-client/lib/smithy-client.rb | 1 + .../lib/smithy-client/plugins/rpc_v2_cbor.rb | 21 -- .../smithy-client/plugins/stub_responses.rb | 6 +- .../lib/smithy-client/rpc_v2_cbor.rb | 185 ++++++++++++++++++ .../rpc_v2_cbor/error_handler.rb | 79 -------- .../lib/smithy-client/rpc_v2_cbor/handler.rb | 77 -------- .../lib/smithy-client/stubbing.rb | 1 - .../smithy-client/stubbing/null_protocol.rb | 18 -- .../lib/smithy-client/stubbing/rpc_v2_cbor.rb | 29 --- gems/smithy-client/lib/smithy-client/stubs.rb | 4 +- .../plugins/transfer_encoding_spec.rb | 2 +- .../spec/smithy-client/rpc_v2_cbor_spec.rb | 163 +++++++++++++++ gems/smithy/lib/smithy/welds/protocols.rb | 12 +- .../interfaces/client/stub_responses_spec.rb | 7 +- 14 files changed, 373 insertions(+), 232 deletions(-) delete mode 100644 gems/smithy-client/lib/smithy-client/plugins/rpc_v2_cbor.rb create mode 100644 gems/smithy-client/lib/smithy-client/rpc_v2_cbor.rb delete mode 100644 gems/smithy-client/lib/smithy-client/rpc_v2_cbor/error_handler.rb delete mode 100644 gems/smithy-client/lib/smithy-client/rpc_v2_cbor/handler.rb delete mode 100644 gems/smithy-client/lib/smithy-client/stubbing/null_protocol.rb delete mode 100644 gems/smithy-client/lib/smithy-client/stubbing/rpc_v2_cbor.rb create mode 100644 gems/smithy-client/spec/smithy-client/rpc_v2_cbor_spec.rb diff --git a/gems/smithy-client/lib/smithy-client.rb b/gems/smithy-client/lib/smithy-client.rb index b18f918ec..ae8dc683b 100644 --- a/gems/smithy-client/lib/smithy-client.rb +++ b/gems/smithy-client/lib/smithy-client.rb @@ -33,6 +33,7 @@ require_relative 'smithy-client/plugin' require_relative 'smithy-client/plugin_list' require_relative 'smithy-client/no_op_protocol' +require_relative 'smithy-client/rpc_v2_cbor' require_relative 'smithy-client/retry' require_relative 'smithy-client/service_error' require_relative 'smithy-client/util' diff --git a/gems/smithy-client/lib/smithy-client/plugins/rpc_v2_cbor.rb b/gems/smithy-client/lib/smithy-client/plugins/rpc_v2_cbor.rb deleted file mode 100644 index 05d7b2562..000000000 --- a/gems/smithy-client/lib/smithy-client/plugins/rpc_v2_cbor.rb +++ /dev/null @@ -1,21 +0,0 @@ -# frozen_string_literal: true - -require_relative '../rpc_v2_cbor/error_handler' -require_relative '../rpc_v2_cbor/handler' -require_relative '../stubbing/rpc_v2_cbor' - -module Smithy - module Client - module Plugins - # @api private - class RpcV2Cbor < Plugin - option(:protocol, default: 'smithy.protocols#rpcv2Cbor') - option(:cbor_codec) { Smithy::Cbor::Codec.new } - option(:stubber) { Stubbing::RpcV2Cbor.new } - - handler(Client::RpcV2Cbor::Handler) - handler(Client::RpcV2Cbor::ErrorHandler, step: :sign) - end - end - end -end diff --git a/gems/smithy-client/lib/smithy-client/plugins/stub_responses.rb b/gems/smithy-client/lib/smithy-client/plugins/stub_responses.rb index 9bd98b4eb..af46159d7 100644 --- a/gems/smithy-client/lib/smithy-client/plugins/stub_responses.rb +++ b/gems/smithy-client/lib/smithy-client/plugins/stub_responses.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require_relative '../no_op_protocol' + module Smithy module Client module Plugins @@ -19,7 +21,9 @@ class StubResponses < Plugin option(:stubs_mutex) { Mutex.new } option(:api_requests) { [] } option(:api_requests_mutex) { Mutex.new } - option(:stubber) { Stubbing::NullProtocol.new } + # Fallback so stubbing works on clients without the Protocol plugin. + # The Protocol plugin's before_initialize overrides this when present. + option(:protocol) { NoOpProtocol.new } def add_handlers(handlers, config) return unless config.stub_responses diff --git a/gems/smithy-client/lib/smithy-client/rpc_v2_cbor.rb b/gems/smithy-client/lib/smithy-client/rpc_v2_cbor.rb new file mode 100644 index 000000000..16b0b0ba9 --- /dev/null +++ b/gems/smithy-client/lib/smithy-client/rpc_v2_cbor.rb @@ -0,0 +1,185 @@ +# frozen_string_literal: true + +module Smithy + module Client + # Protocol implementation for Smithy RPC v2 CBOR. Serializes requests and + # deserializes responses (and errors) for the +smithy.protocols#rpcv2Cbor+ + # wire format, and builds stubbed responses for testing. + # @api private + class RpcV2Cbor + def initialize + @codec = Smithy::Cbor::Codec.new + end + + # Serialize the request into the RPC v2 CBOR wire format. + # @param [HandlerContext] context + def build_request(context) + context.http_request.http_method = 'POST' + apply_headers(context) + apply_body(context) + apply_url_path(context) + end + + # Deserialize a successful response body. + # @param [HandlerContext] context + # @return [Object] the parsed output data + def parse_data(context) + @codec.parse(context.operation.output, context.http_response.body.read) + end + + # Deserialize an error response into the modeled error. Called on every + # response; returns nil when the response is not an error. + # @param [HandlerContext] context + # @return [StandardError, nil] + def parse_error(context) + # Only inspect responses in the HTTP status range. Outside it (e.g. a + # status 0 from a signaled transport error), leave the response alone + # so the transport error propagates untouched. Mirrors the old + # ErrorHandler's on_done(200..599) gate. + return unless (200..599).cover?(context.http_response.status_code) + + # Malformed responses should raise an http-based error, so we validate + # the protocol header across the full 200..599 range. + unless valid_response?(context) + code, data = http_status_error(context) + return build_error(context, code, data) + end + return unless (400..599).cover?(context.http_response.status_code) + + error(context) + end + + # Build a stubbed HTTP response for the given output data. + # @param [Configuration] _config + # @param [Schema::OperationShape] operation + # @param [Object] data + # @return [Http::Response] + def stub_data(_config, operation, data) + response = Http::Response.new + response.status_code = 200 + response.headers['Smithy-Protocol'] = 'rpc-v2-cbor' + response.headers['Content-Type'] = 'application/cbor' + response.body = @codec.build(operation.output, data) + response + end + + # Build a stubbed HTTP error response for the given error code. + # @param [Configuration] _config + # @param [String] error_code + # @return [Http::Response] + def stub_error(_config, error_code) + response = Http::Response.new + response.status_code = 400 + response.headers['Smithy-Protocol'] = 'rpc-v2-cbor' + response.headers['Content-Type'] = 'application/cbor' + data = { '__type' => "smithy.ruby.tests##{error_code}", 'message' => 'stubbed-error-message' } + response.body = Cbor.encode(data) + response + end + + private + + def apply_headers(context) + context.http_request.headers['Smithy-Protocol'] = 'rpc-v2-cbor' + apply_content_type_header(context) + apply_accept_header(context) + end + + def apply_content_type_header(context) + input = context.operation.input + content_type = + if event_stream?(input) + 'application/vnd.amazon.eventstream' + elsif input != Schema::Shapes::Prelude::Unit + 'application/cbor' + end + + context.http_request.headers['Content-Type'] ||= content_type if content_type + end + + def apply_accept_header(context) + accept = + if event_stream?(context.operation.output) + 'application/vnd.amazon.eventstream' + else + 'application/cbor' + end + + context.http_request.headers['Accept'] ||= accept + end + + def apply_body(context) + context.http_request.body = @codec.build(context.operation.input, context.params) + end + + def apply_url_path(context) + base = context.http_request.endpoint + service_name = context.config.service.name + base.path += "/service/#{service_name}/operation/#{context.operation.name}" + end + + def event_stream?(input_shape) + input_shape.members.each_value do |member_shape| + shape = member_shape.target + return true if shape.traits.key?('smithy.api#streaming') && shape.is_a?(Schema::Shapes::UnionShape) + end + false + end + + def valid_response?(context) + req_header = context.http_request.headers['smithy-protocol'] + resp_header = context.http_response.headers['smithy-protocol'] + req_header == resp_header + end + + def error(context) + body = context.http_response.body.read + code, data = + if body.empty? + http_status_error(context) + else + extract_error(body, context) + end + build_error(context, code, data) + end + + def extract_error(body, context) + data = Cbor.decode(body) + code = error_code(context, data) + data = parse_error_data(context, body, code) + [code, data] + rescue Cbor::ParseError + [http_status_error_code(context), Schema::EmptyStructure.new] + end + + def parse_error_data(context, body, code) + data = Schema::EmptyStructure.new + context.operation.errors.each do |err_shape| + next unless err_shape.name == code + + data = Cbor::Parser.new.parse(err_shape, body, err_shape.type.new) + end + data + end + + def error_code(context, data) + code = data['__type'] + code ||= http_status_error_code(context) + code.split('#').last.split('$').first + end + + def build_error(context, code, data) + errors_module = context.client.class.errors_module + errors_module.error_class(code).new(context, data) + end + + def http_status_error(context) + [http_status_error_code(context), Schema::EmptyStructure.new] + end + + def http_status_error_code(context) + "HTTP#{context.http_response.status_code}Error" + end + end + end +end diff --git a/gems/smithy-client/lib/smithy-client/rpc_v2_cbor/error_handler.rb b/gems/smithy-client/lib/smithy-client/rpc_v2_cbor/error_handler.rb deleted file mode 100644 index af1ceda03..000000000 --- a/gems/smithy-client/lib/smithy-client/rpc_v2_cbor/error_handler.rb +++ /dev/null @@ -1,79 +0,0 @@ -# frozen_string_literal: true - -module Smithy - module Client - module RpcV2Cbor - # @api private - class ErrorHandler < Client::Handler - def call(context) - # Malformed responses should throw an http based error, so we check - # 200 range for error handling only for this case. - @handler.call(context).on_done(200..599) do |response| - if !valid_response?(context) - code, data = http_status_error(context) - response.error = build_error(context, code, data) - elsif (400..599).cover?(context.http_response.status_code) - response.error = error(context) - end - end - end - - private - - def valid_response?(context) - req_header = context.http_request.headers['smithy-protocol'] - resp_header = context.http_response.headers['smithy-protocol'] - req_header == resp_header - end - - def error(context) - body = context.http_response.body.read - if body.empty? - code, data = http_status_error(context) - else - code, data = extract_error(body, context) - end - build_error(context, code, data) - end - - def extract_error(body, context) - data = Cbor.decode(body) - code = error_code(context, data) - data = parse_error_data(context, body, code) - [code, data] - rescue Cbor::ParseError - [http_status_error_code(context), Schema::EmptyStructure.new] - end - - def parse_error_data(context, body, code) - data = Schema::EmptyStructure.new - context.operation.errors.each do |err_shape| - next unless err_shape.name == code - - data = Cbor::Parser.new.parse(err_shape, body, err_shape.type.new) - end - data - end - - def error_code(context, data) - code = data['__type'] - code ||= http_status_error_code(context) - code.split('#').last.split('$').first - end - - def build_error(context, code, data) - errors_module = context.client.class.errors_module - errors_module.error_class(code).new(context, data) - end - - def http_status_error(context) - [http_status_error_code(context), Schema::EmptyStructure.new] - end - - def http_status_error_code(context) - "HTTP#{context.http_response.status_code}Error" - end - end - end - end -end diff --git a/gems/smithy-client/lib/smithy-client/rpc_v2_cbor/handler.rb b/gems/smithy-client/lib/smithy-client/rpc_v2_cbor/handler.rb deleted file mode 100644 index 8fc724807..000000000 --- a/gems/smithy-client/lib/smithy-client/rpc_v2_cbor/handler.rb +++ /dev/null @@ -1,77 +0,0 @@ -# frozen_string_literal: true - -module Smithy - module Client - module RpcV2Cbor - # @api private - class Handler < Client::Handler - def call(context) - build_request(context) - response = @handler.call(context) - response.on_done(200..299) { |resp| resp.data = parse_body(context) } - response - end - - private - - def build_request(context) - context.http_request.http_method = 'POST' - apply_headers(context) - apply_body(context) - apply_url_path(context) - end - - def parse_body(context) - context.config.cbor_codec.parse(context.operation.output, context.http_response.body.read) - end - - def apply_headers(context) - context.http_request.headers['Smithy-Protocol'] = 'rpc-v2-cbor' - apply_content_type_header(context) - apply_accept_header(context) - end - - def apply_content_type_header(context) - input = context.operation.input - content_type = - if event_stream?(input) - 'application/vnd.amazon.eventstream' - elsif input != Schema::Shapes::Prelude::Unit - 'application/cbor' - end - - context.http_request.headers['Content-Type'] ||= content_type if content_type - end - - def apply_accept_header(context) - accept = - if event_stream?(context.operation.output) - 'application/vnd.amazon.eventstream' - else - 'application/cbor' - end - - context.http_request.headers['Accept'] ||= accept - end - - def apply_body(context) - context.http_request.body = context.config.cbor_codec.build(context.operation.input, context.params) - end - - def apply_url_path(context) - base = context.http_request.endpoint - service_name = context.config.service.name - base.path += "/service/#{service_name}/operation/#{context.operation.name}" - end - - def event_stream?(input_shape) - input_shape.members.each_value do |member_shape| - shape = member_shape.target - return true if shape.traits.key?('smithy.api#streaming') && shape.is_a?(Schema::Shapes::UnionShape) - end - false - end - end - end - end -end diff --git a/gems/smithy-client/lib/smithy-client/stubbing.rb b/gems/smithy-client/lib/smithy-client/stubbing.rb index 84a3f47b2..c61e001e2 100644 --- a/gems/smithy-client/lib/smithy-client/stubbing.rb +++ b/gems/smithy-client/lib/smithy-client/stubbing.rb @@ -2,7 +2,6 @@ require_relative 'stubbing/data_applicator' require_relative 'stubbing/empty_stub' -require_relative 'stubbing/null_protocol' require_relative 'stubbing/stub_data' module Smithy diff --git a/gems/smithy-client/lib/smithy-client/stubbing/null_protocol.rb b/gems/smithy-client/lib/smithy-client/stubbing/null_protocol.rb deleted file mode 100644 index 34107e78b..000000000 --- a/gems/smithy-client/lib/smithy-client/stubbing/null_protocol.rb +++ /dev/null @@ -1,18 +0,0 @@ -# frozen_string_literal: true - -module Smithy - module Client - module Stubbing - # @api private - class NullProtocol - def stub_data(_config, _operation, _data) - Http::Response.new - end - - def stub_error(_config, _error_code) - Http::Response.new - end - end - end - end -end diff --git a/gems/smithy-client/lib/smithy-client/stubbing/rpc_v2_cbor.rb b/gems/smithy-client/lib/smithy-client/stubbing/rpc_v2_cbor.rb deleted file mode 100644 index 5a3b6737d..000000000 --- a/gems/smithy-client/lib/smithy-client/stubbing/rpc_v2_cbor.rb +++ /dev/null @@ -1,29 +0,0 @@ -# frozen_string_literal: true - -module Smithy - module Client - module Stubbing - # @api private - class RpcV2Cbor - def stub_data(config, operation, data) - response = Http::Response.new - response.status_code = 200 - response.headers['Smithy-Protocol'] = 'rpc-v2-cbor' - response.headers['Content-Type'] = 'application/cbor' - response.body = config.cbor_codec.build(operation.output, data) - response - end - - def stub_error(_config, error_code) - response = Http::Response.new - response.status_code = 400 - response.headers['Smithy-Protocol'] = 'rpc-v2-cbor' - response.headers['Content-Type'] = 'application/cbor' - data = { '__type' => "smithy.ruby.tests##{error_code}", 'message' => 'stubbed-error-message' } - response.body = Cbor.encode(data) - response - end - end - end - end -end diff --git a/gems/smithy-client/lib/smithy-client/stubs.rb b/gems/smithy-client/lib/smithy-client/stubs.rb index d398aec46..da186cf9d 100644 --- a/gems/smithy-client/lib/smithy-client/stubs.rb +++ b/gems/smithy-client/lib/smithy-client/stubs.rb @@ -182,7 +182,7 @@ def convert_stub(operation_name, stub, context) end def service_error_stub(error_code) - { http: @config.stubber.stub_error(@config, error_code) } + { http: @config.protocol.stub_error(@config, error_code) } end def http_response_stub(operation_name, data) @@ -205,7 +205,7 @@ def data_to_http_response(operation_name, data) operation = @config.service.operation(operation_name) data = ParamConverter.new(operation.output).convert(data) ParamValidator.new(operation.output, validate_required: false).validate!(data, context: 'stub') - @config.stubber.stub_data(@config, operation, data) + @config.protocol.stub_data(@config, operation, data) end end end diff --git a/gems/smithy-client/spec/smithy-client/plugins/transfer_encoding_spec.rb b/gems/smithy-client/spec/smithy-client/plugins/transfer_encoding_spec.rb index 15570a688..bb8316012 100644 --- a/gems/smithy-client/spec/smithy-client/plugins/transfer_encoding_spec.rb +++ b/gems/smithy-client/spec/smithy-client/plugins/transfer_encoding_spec.rb @@ -13,7 +13,7 @@ module Plugins # Replace the RPC protocol plugin (which serializes all params into the body) # with a passthrough that pipes the streaming member directly to the # HTTP body, mimicking REST protocol behavior. - klass.remove_plugin(Plugins::RpcV2Cbor) + klass.remove_plugin(Plugins::Protocol) klass.add_plugin(streaming_body_plugin) klass end diff --git a/gems/smithy-client/spec/smithy-client/rpc_v2_cbor_spec.rb b/gems/smithy-client/spec/smithy-client/rpc_v2_cbor_spec.rb new file mode 100644 index 000000000..37426e1ec --- /dev/null +++ b/gems/smithy-client/spec/smithy-client/rpc_v2_cbor_spec.rb @@ -0,0 +1,163 @@ +# frozen_string_literal: true + +require_relative '../spec_helper' + +module Smithy + module Client + describe RpcV2Cbor do + subject(:protocol) { described_class.new } + + let(:sample_client) { ClientHelper.sample_client } + let(:client) { sample_client.const_get(:Client).new(endpoint: 'https://example.com', stub_responses: true) } + let(:operation) { client.config.service.operation(:operation) } + let(:config) { client.config } + + def build_context(http_request: Http::Request.new, http_response: Http::Response.new, params: {}) + http_request.endpoint = 'https://example.com' + HandlerContext.new( + operation_name: :operation, + operation: operation, + client: client, + params: params, + config: config, + http_request: http_request, + http_response: http_response + ) + end + + describe '#build_request' do + it 'sets the POST method' do + context = build_context + protocol.build_request(context) + expect(context.http_request.http_method).to eq('POST') + end + + it 'sets the Smithy-Protocol header' do + context = build_context + protocol.build_request(context) + expect(context.http_request.headers['Smithy-Protocol']).to eq('rpc-v2-cbor') + end + + it 'sets the Content-Type and Accept headers' do + context = build_context + protocol.build_request(context) + expect(context.http_request.headers['Content-Type']).to eq('application/cbor') + expect(context.http_request.headers['Accept']).to eq('application/cbor') + end + + it 'appends the rpc v2 service/operation url path' do + context = build_context + protocol.build_request(context) + service_name = config.service.name + expect(context.http_request.endpoint.path) + .to end_with("/service/#{service_name}/operation/#{operation.name}") + end + + it 'serializes the params into the body' do + context = build_context(params: { string: 'hello' }) + protocol.build_request(context) + expect(context.http_request.body.read).not_to be_empty + end + end + + describe '#parse_data' do + it 'parses the response body via the codec' do + data = { string: 'hello' } + body = Smithy::Cbor::Codec.new.build(operation.output, data) + context = build_context(http_response: Http::Response.new(status_code: 200, body: body)) + result = protocol.parse_data(context) + expect(result[:string]).to eq('hello') + end + end + + describe '#parse_error' do + let(:request_headers) { { 'smithy-protocol' => 'rpc-v2-cbor' } } + + def response(status_code:, body: '', protocol_header: 'rpc-v2-cbor') + headers = protocol_header ? { 'smithy-protocol' => protocol_header } : {} + Http::Response.new(status_code: status_code, headers: headers, body: body) + end + + it 'returns nil for a successful response' do + context = build_context( + http_request: Http::Request.new(headers: request_headers), + http_response: response(status_code: 200) + ) + expect(protocol.parse_error(context)).to be_nil + end + + it 'returns nil for a response outside the HTTP status range (e.g. a signaled transport error)' do + context = build_context( + http_request: Http::Request.new(headers: request_headers), + http_response: response(status_code: 0, protocol_header: nil) + ) + expect(protocol.parse_error(context)).to be_nil + end + + it 'returns an error when the protocol header does not match (even on 2xx)' do + context = build_context( + http_request: Http::Request.new(headers: request_headers), + http_response: response(status_code: 200, protocol_header: 'not-cbor') + ) + expect(protocol.parse_error(context)).to be_a(StandardError) + end + + it 'returns an error when the response is missing the protocol header' do + context = build_context( + http_request: Http::Request.new(headers: request_headers), + http_response: response(status_code: 200, protocol_header: nil) + ) + expect(protocol.parse_error(context)).to be_a(StandardError) + end + + it 'returns an HTTP status error for an error response with an empty body' do + context = build_context( + http_request: Http::Request.new(headers: request_headers), + http_response: response(status_code: 500) + ) + error = protocol.parse_error(context) + expect(error).to be_a(StandardError) + end + + it 'extracts the modeled error from the __type in the body' do + body = Cbor.encode('__type' => 'smithy.ruby.tests#Error', 'message' => 'boom') + context = build_context( + http_request: Http::Request.new(headers: request_headers), + http_response: response(status_code: 400, body: body) + ) + error = protocol.parse_error(context) + expect(error).to be_a(StandardError) + end + + it 'falls back to an HTTP status error when the body is not valid CBOR' do + context = build_context( + http_request: Http::Request.new(headers: request_headers), + http_response: response(status_code: 400, body: 'not-cbor') + ) + error = protocol.parse_error(context) + expect(error).to be_a(StandardError) + end + end + + describe '#stub_data' do + it 'builds a 200 CBOR response with protocol headers' do + response = protocol.stub_data(config, operation, { string: 'hello' }) + expect(response.status_code).to eq(200) + expect(response.headers['Smithy-Protocol']).to eq('rpc-v2-cbor') + expect(response.headers['Content-Type']).to eq('application/cbor') + expect(response.body.read).not_to be_empty + end + end + + describe '#stub_error' do + it 'builds a 400 CBOR error response with protocol headers' do + response = protocol.stub_error(config, 'Error') + expect(response.status_code).to eq(400) + expect(response.headers['Smithy-Protocol']).to eq('rpc-v2-cbor') + decoded = Cbor.decode(response.body.read) + expect(decoded['__type']).to eq('smithy.ruby.tests#Error') + end + end + end + end +end diff --git a/gems/smithy/lib/smithy/welds/protocols.rb b/gems/smithy/lib/smithy/welds/protocols.rb index c0cd68dc9..9e41ed640 100644 --- a/gems/smithy/lib/smithy/welds/protocols.rb +++ b/gems/smithy/lib/smithy/welds/protocols.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true -require 'smithy-client/plugins/rpc_v2_cbor' +require 'smithy-client/plugins/protocol' +require 'smithy-client/rpc_v2_cbor' module Smithy module Welds @@ -23,7 +24,14 @@ def for?(service) def add_plugins case @protocol when 'smithy.protocols#rpcv2Cbor' - { Smithy::Client::Plugins::RpcV2Cbor => { require_path: 'smithy-client/plugins/rpc_v2_cbor' } } + { Smithy::Client::Plugins::Protocol => { require_path: 'smithy-client/plugins/protocol' } } + end + end + + def add_protocols + case @protocol + when 'smithy.protocols#rpcv2Cbor' + { rpc_v2_cbor: Smithy::Client::RpcV2Cbor } end end diff --git a/gems/smithy/spec/interfaces/client/stub_responses_spec.rb b/gems/smithy/spec/interfaces/client/stub_responses_spec.rb index f893bbad8..74543433a 100644 --- a/gems/smithy/spec/interfaces/client/stub_responses_spec.rb +++ b/gems/smithy/spec/interfaces/client/stub_responses_spec.rb @@ -35,7 +35,12 @@ } end - before(:all) { Shapes::Client.add_plugin(Smithy::Client::Plugins::RpcV2Cbor) } + before(:all) do + Shapes::Client.add_plugin(Smithy::Client::Plugins::Protocol) + Shapes::Client.define_singleton_method(:protocols) do + { rpc_v2_cbor: Smithy::Client::RpcV2Cbor } + end + end before do allow(Time).to receive(:now).and_return(now) From 90ac31e0900db09d73c1004700ff43acd39ad00a Mon Sep 17 00:00:00 2001 From: Juli Tera Date: Mon, 27 Jul 2026 12:34:13 -0700 Subject: [PATCH 04/15] Parse protocol errors in a dedicated handler at the sign step --- .../lib/smithy-client/plugins/protocol.rb | 15 +++++++++++---- .../spec/smithy-client/plugins/protocol_spec.rb | 9 ++++++++- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/gems/smithy-client/lib/smithy-client/plugins/protocol.rb b/gems/smithy-client/lib/smithy-client/plugins/protocol.rb index ff353b3a0..905f1615b 100644 --- a/gems/smithy-client/lib/smithy-client/plugins/protocol.rb +++ b/gems/smithy-client/lib/smithy-client/plugins/protocol.rb @@ -27,24 +27,31 @@ def call(context) end end - # Parses the response after the send handler returns. Send handlers - # must fully signal the response body (signal_done) before returning, - # since parsing happens inline here rather than in a body callback. # @api private class ParseHandler < Handler def call(context) response = @handler.call(context) - response.error = context.config.protocol.parse_error(context) unless response.error response.data = context.config.protocol.parse_data(context) unless response.error response end end + # @api private + class ErrorHandler < Handler + def call(context) + response = @handler.call(context) + response.error = context.config.protocol.parse_error(context) if response.error.nil? + response + end + end + def add_handlers(handlers, _config) handlers.add(BuildHandler) handlers.add(ParseHandler) + handlers.add(ErrorHandler, step: :sign) end + # TODO: pass in relevant settings to protocol instance on client init def before_initialize(client_class, options) case options[:protocol] when nil diff --git a/gems/smithy-client/spec/smithy-client/plugins/protocol_spec.rb b/gems/smithy-client/spec/smithy-client/plugins/protocol_spec.rb index 1056e87fb..a783c8e5a 100644 --- a/gems/smithy-client/spec/smithy-client/plugins/protocol_spec.rb +++ b/gems/smithy-client/spec/smithy-client/plugins/protocol_spec.rb @@ -73,10 +73,17 @@ def stub_error(_config, _error_code); end end end - it 'adds the build and parse handlers' do + it 'adds the build, parse, and error handlers' do client = client_class.new(client_options) expect(client.handlers).to include(Protocol::BuildHandler) expect(client.handlers).to include(Protocol::ParseHandler) + expect(client.handlers).to include(Protocol::ErrorHandler) + end + + it 'adds the error handler at the :sign step (inside the retry loop)' do + client = client_class.new(client_options) + entry = client.handlers.entries.find { |e| e.handler_class == Protocol::ErrorHandler } + expect(entry.step).to eq(:sign) end end From cfa1a291cc27b1c385ea31feefc974c56a6b8fb1 Mon Sep 17 00:00:00 2001 From: Juli Tera Date: Mon, 27 Jul 2026 12:34:20 -0700 Subject: [PATCH 05/15] Add the generic Protocol plugin as a default plugin --- gems/smithy/lib/smithy/welds/default_plugins.rb | 2 ++ gems/smithy/lib/smithy/welds/protocols.rb | 13 ++++--------- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/gems/smithy/lib/smithy/welds/default_plugins.rb b/gems/smithy/lib/smithy/welds/default_plugins.rb index 6d1797f20..7c313f106 100644 --- a/gems/smithy/lib/smithy/welds/default_plugins.rb +++ b/gems/smithy/lib/smithy/welds/default_plugins.rb @@ -10,6 +10,7 @@ require 'smithy-client/plugins/pageable_response' require 'smithy-client/plugins/param_converter' require 'smithy-client/plugins/param_validator' +require 'smithy-client/plugins/protocol' require 'smithy-client/plugins/raise_response_errors' require 'smithy-client/plugins/request_compression' require 'smithy-client/plugins/resolve_auth' @@ -42,6 +43,7 @@ def add_plugins # rubocop:disable Metrics/MethodLength Smithy::Client::Plugins::PageableResponse => { require_path: "#{base_path}/pageable_response" }, Smithy::Client::Plugins::ParamConverter => { require_path: "#{base_path}/param_converter" }, Smithy::Client::Plugins::ParamValidator => { require_path: "#{base_path}/param_validator" }, + Smithy::Client::Plugins::Protocol => { require_path: "#{base_path}/protocol" }, Smithy::Client::Plugins::RaiseResponseErrors => { require_path: "#{base_path}/raise_response_errors" }, Smithy::Client::Plugins::RequestCompression => { require_path: "#{base_path}/request_compression" }, Smithy::Client::Plugins::ResolveAuth => { require_path: "#{base_path}/resolve_auth" }, diff --git a/gems/smithy/lib/smithy/welds/protocols.rb b/gems/smithy/lib/smithy/welds/protocols.rb index 9e41ed640..b053e96bc 100644 --- a/gems/smithy/lib/smithy/welds/protocols.rb +++ b/gems/smithy/lib/smithy/welds/protocols.rb @@ -1,11 +1,13 @@ # frozen_string_literal: true -require 'smithy-client/plugins/protocol' require 'smithy-client/rpc_v2_cbor' module Smithy module Welds - # Adds a supported protocol plugin to the client if the service has the trait, prioritized by a list. + # Registers a supported protocol for the client if the service has the trait, + # prioritized by a list. The generic Protocol plugin itself is added as a + # default plugin (see DefaultPlugins); this weld only contributes the + # protocol registry entry and its runtime dependency. class Protocols < Weld PROTOCOL_PRIORITY = ['smithy.protocols#rpcv2Cbor'].freeze @@ -21,13 +23,6 @@ def for?(service) false end - def add_plugins - case @protocol - when 'smithy.protocols#rpcv2Cbor' - { Smithy::Client::Plugins::Protocol => { require_path: 'smithy-client/plugins/protocol' } } - end - end - def add_protocols case @protocol when 'smithy.protocols#rpcv2Cbor' From 1dd38439cb48c8c29856e2b6b0b1cbf56b999a5f Mon Sep 17 00:00:00 2001 From: Juli Tera Date: Mon, 27 Jul 2026 12:34:36 -0700 Subject: [PATCH 06/15] Parse CBOR error data through the configured codec --- gems/smithy-client/lib/smithy-client/rpc_v2_cbor.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gems/smithy-client/lib/smithy-client/rpc_v2_cbor.rb b/gems/smithy-client/lib/smithy-client/rpc_v2_cbor.rb index 16b0b0ba9..56f6f4a93 100644 --- a/gems/smithy-client/lib/smithy-client/rpc_v2_cbor.rb +++ b/gems/smithy-client/lib/smithy-client/rpc_v2_cbor.rb @@ -157,7 +157,7 @@ def parse_error_data(context, body, code) context.operation.errors.each do |err_shape| next unless err_shape.name == code - data = Cbor::Parser.new.parse(err_shape, body, err_shape.type.new) + data = @codec.parse(err_shape, body, err_shape.type.new) end data end From 9d1f4bae8d16eff718ff8e9c52de2129fe0aaf21 Mon Sep 17 00:00:00 2001 From: Juli Tera Date: Mon, 27 Jul 2026 12:53:04 -0700 Subject: [PATCH 07/15] Remove redundant protocol fallback from stub responses --- .../lib/smithy-client/plugins/stub_responses.rb | 5 ----- .../spec/smithy-client/plugins/transfer_encoding_spec.rb | 6 ++---- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/gems/smithy-client/lib/smithy-client/plugins/stub_responses.rb b/gems/smithy-client/lib/smithy-client/plugins/stub_responses.rb index af46159d7..393413fa1 100644 --- a/gems/smithy-client/lib/smithy-client/plugins/stub_responses.rb +++ b/gems/smithy-client/lib/smithy-client/plugins/stub_responses.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -require_relative '../no_op_protocol' - module Smithy module Client module Plugins @@ -21,9 +19,6 @@ class StubResponses < Plugin option(:stubs_mutex) { Mutex.new } option(:api_requests) { [] } option(:api_requests_mutex) { Mutex.new } - # Fallback so stubbing works on clients without the Protocol plugin. - # The Protocol plugin's before_initialize overrides this when present. - option(:protocol) { NoOpProtocol.new } def add_handlers(handlers, config) return unless config.stub_responses diff --git a/gems/smithy-client/spec/smithy-client/plugins/transfer_encoding_spec.rb b/gems/smithy-client/spec/smithy-client/plugins/transfer_encoding_spec.rb index bb8316012..d486949f2 100644 --- a/gems/smithy-client/spec/smithy-client/plugins/transfer_encoding_spec.rb +++ b/gems/smithy-client/spec/smithy-client/plugins/transfer_encoding_spec.rb @@ -10,10 +10,8 @@ module Plugins let(:sample_client) { ClientHelper.sample_client(shapes: shapes) } let(:client_class) do klass = sample_client.const_get(:Client) - # Replace the RPC protocol plugin (which serializes all params into the body) - # with a passthrough that pipes the streaming member directly to the - # HTTP body, mimicking REST protocol behavior. - klass.remove_plugin(Plugins::Protocol) + # Add a passthrough plugin that pipes the streaming member directly to + # the HTTP body, mimicking REST protocol behavior. klass.add_plugin(streaming_body_plugin) klass end From e1e08752e717f90daeedd5de61d77a45aeafe55f Mon Sep 17 00:00:00 2001 From: Juli Tera Date: Tue, 28 Jul 2026 14:19:42 -0700 Subject: [PATCH 08/15] Add require_path support to the protocol registry --- .../lib/smithy/templates/client/client.erb | 3 +++ gems/smithy/lib/smithy/views/client.rb | 1 + gems/smithy/lib/smithy/views/client/client.rb | 24 +++++++++++++++---- .../lib/smithy/views/client/protocol.rb | 23 ++++++++++++++++++ gems/smithy/lib/smithy/weld.rb | 7 ++++-- gems/smithy/lib/smithy/welds/protocols.rb | 9 +++++-- 6 files changed, 59 insertions(+), 8 deletions(-) create mode 100644 gems/smithy/lib/smithy/views/client/protocol.rb diff --git a/gems/smithy/lib/smithy/templates/client/client.erb b/gems/smithy/lib/smithy/templates/client/client.erb index b1c7bb53c..709982310 100644 --- a/gems/smithy/lib/smithy/templates/client/client.erb +++ b/gems/smithy/lib/smithy/templates/client/client.erb @@ -5,6 +5,9 @@ <%require_plugins.each do |require| -%> <%= require %> <% end -%> +<%require_protocols.each do |require| -%> +<%= require %> +<% end -%> module <%= module_name %> # An API client for <%= module_name %>. diff --git a/gems/smithy/lib/smithy/views/client.rb b/gems/smithy/lib/smithy/views/client.rb index d99bc3709..022a5a1c4 100644 --- a/gems/smithy/lib/smithy/views/client.rb +++ b/gems/smithy/lib/smithy/views/client.rb @@ -13,6 +13,7 @@ module Client; end require_relative 'client/operation_examples' require_relative 'client/plugin' require_relative 'client/plugin_list' +require_relative 'client/protocol' require_relative 'client/request_response_example' require_relative 'client/shape_to_hash' diff --git a/gems/smithy/lib/smithy/views/client/client.rb b/gems/smithy/lib/smithy/views/client/client.rb index 838fcdc6b..6837b01a1 100644 --- a/gems/smithy/lib/smithy/views/client/client.rb +++ b/gems/smithy/lib/smithy/views/client/client.rb @@ -10,6 +10,7 @@ def initialize(plan, code_generated_plugins) @model = plan.model @service_id, @service = plan.service.first @plugins = PluginList.new(plan, code_generated_plugins) + @protocols = build_protocols(plan) super() end @@ -23,6 +24,17 @@ def require_plugins requires end + def require_protocols + requires = [] + @protocols.each do |protocol| + next unless protocol.require_path + next if !@plan.destination_root && protocol.require_relative? + + requires << "require#{'_relative' if protocol.require_relative?} '#{protocol.require_path}'" + end + requires + end + def module_name @plan.module_name end @@ -40,12 +52,11 @@ def add_plugins end def protocols - weld_protocols = @plan.welds.map(&:add_protocols).reduce({}, :merge) - return ['{}'] if weld_protocols.empty? + return ['{}'] if @protocols.empty? lines = ['{'] - weld_protocols.each do |name, protocol_class| - lines << " #{name}: #{protocol_class}," + @protocols.each do |protocol| + lines << " #{protocol.name}: #{protocol.class_name}," end lines.last.chomp!(',') if lines.last.end_with?(',') lines << '}' @@ -93,6 +104,11 @@ def waiters private + def build_protocols(plan) + weld_protocols = plan.welds.map(&:add_protocols).reduce({}, :merge) + weld_protocols.map { |name, options| Protocol.new(options.merge(name: name)) } + end + def option_docstrings(option) lines = [] lines << option_tag(option) diff --git a/gems/smithy/lib/smithy/views/client/protocol.rb b/gems/smithy/lib/smithy/views/client/protocol.rb new file mode 100644 index 000000000..4bf9aa1ff --- /dev/null +++ b/gems/smithy/lib/smithy/views/client/protocol.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module Smithy + module Views + module Client + # @api private + class Protocol + def initialize(options = {}) + @name = options[:name] + @class_name = options[:class_name] + @require_path = options[:require_path] + @require_relative = options.fetch(:require_relative, false) + end + + attr_accessor :name, :class_name, :require_path + + def require_relative? + @require_relative + end + end + end + end +end diff --git a/gems/smithy/lib/smithy/weld.rb b/gems/smithy/lib/smithy/weld.rb index 10ded5121..a4f685158 100644 --- a/gems/smithy/lib/smithy/weld.rb +++ b/gems/smithy/lib/smithy/weld.rb @@ -97,8 +97,11 @@ def remove_plugins # Called when constructing the client. Any protocols defined here will be # merged into the client's protocol registry. The key is the protocol name - # (a Symbol) and the value is the fully qualified protocol class. - # @return [Hash] a mapping of protocol names to protocol classes. + # (a Symbol), and the value is a hash with any of the following keys: + # * :class_name - the fully qualified class name of the protocol + # * :require_path - the path to require the protocol from the client + # * :require_relative - true if the path should be required relative to the client + # @return [Hash] a mapping of protocol names to protocol options. def add_protocols {} end diff --git a/gems/smithy/lib/smithy/welds/protocols.rb b/gems/smithy/lib/smithy/welds/protocols.rb index b053e96bc..7c3cbbd9a 100644 --- a/gems/smithy/lib/smithy/welds/protocols.rb +++ b/gems/smithy/lib/smithy/welds/protocols.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require 'smithy-client/rpc_v2_cbor' +require_relative '../../../../smithy-client/lib/smithy-client/rpc_v2_cbor' module Smithy module Welds @@ -26,7 +26,12 @@ def for?(service) def add_protocols case @protocol when 'smithy.protocols#rpcv2Cbor' - { rpc_v2_cbor: Smithy::Client::RpcV2Cbor } + { + rpc_v2_cbor: { + class_name: Smithy::Client::RpcV2Cbor, + require_path: 'smithy-client/rpc_v2_cbor' + } + } end end From fbc27e750b386d70af66ac8ea3fd0c4e7ad5badc Mon Sep 17 00:00:00 2001 From: Juli Tera Date: Thu, 30 Jul 2026 13:24:06 -0700 Subject: [PATCH 09/15] Strengthen and consolidate RPC v2 CBOR protocol specs Consolidate the build_request examples with aggregate_failures and merge the two returns-nil cases. Strengthen error assertions to check the resolved error class/code and a parsed field (not just StandardError), and round-trip stubbed data through the codec. Reuse NoOpProtocol as the resolution test double. --- .../smithy-client/plugins/protocol_spec.rb | 13 +-- .../spec/smithy-client/rpc_v2_cbor_spec.rb | 84 +++++++++---------- 2 files changed, 43 insertions(+), 54 deletions(-) diff --git a/gems/smithy-client/spec/smithy-client/plugins/protocol_spec.rb b/gems/smithy-client/spec/smithy-client/plugins/protocol_spec.rb index a783c8e5a..c728095b7 100644 --- a/gems/smithy-client/spec/smithy-client/plugins/protocol_spec.rb +++ b/gems/smithy-client/spec/smithy-client/plugins/protocol_spec.rb @@ -18,15 +18,10 @@ module Plugins end let(:client_options) { { endpoint: 'https://example.com' } } - let(:fake_protocol_class) do - Class.new do - def build_request(_context); end - def parse_data(_context); end - def parse_error(_context); end - def stub_data(_config, _operation, _data); end - def stub_error(_config, _error_code); end - end - end + # A distinct subclass of NoOpProtocol (not NoOpProtocol itself) so the + # "defaults to the first registered protocol" example proves resolution + # against the registry, separate from the empty-registry fallback. + let(:fake_protocol_class) { Class.new(Client::NoOpProtocol) } # Override the generated registry with a controlled double so the # plugin's resolution logic is tested in isolation from any real protocol. diff --git a/gems/smithy-client/spec/smithy-client/rpc_v2_cbor_spec.rb b/gems/smithy-client/spec/smithy-client/rpc_v2_cbor_spec.rb index 37426e1ec..c15d07156 100644 --- a/gems/smithy-client/spec/smithy-client/rpc_v2_cbor_spec.rb +++ b/gems/smithy-client/spec/smithy-client/rpc_v2_cbor_spec.rb @@ -26,37 +26,19 @@ def build_context(http_request: Http::Request.new, http_response: Http::Response end describe '#build_request' do - it 'sets the POST method' do - context = build_context - protocol.build_request(context) - expect(context.http_request.http_method).to eq('POST') - end - - it 'sets the Smithy-Protocol header' do - context = build_context - protocol.build_request(context) - expect(context.http_request.headers['Smithy-Protocol']).to eq('rpc-v2-cbor') - end - - it 'sets the Content-Type and Accept headers' do - context = build_context - protocol.build_request(context) - expect(context.http_request.headers['Content-Type']).to eq('application/cbor') - expect(context.http_request.headers['Accept']).to eq('application/cbor') - end - - it 'appends the rpc v2 service/operation url path' do - context = build_context - protocol.build_request(context) - service_name = config.service.name - expect(context.http_request.endpoint.path) - .to end_with("/service/#{service_name}/operation/#{operation.name}") - end - - it 'serializes the params into the body' do + it 'builds a valid RPC v2 CBOR request' do context = build_context(params: { string: 'hello' }) protocol.build_request(context) - expect(context.http_request.body.read).not_to be_empty + service_name = config.service.name + aggregate_failures do + expect(context.http_request.http_method).to eq('POST') + expect(context.http_request.headers['Smithy-Protocol']).to eq('rpc-v2-cbor') + expect(context.http_request.headers['Content-Type']).to eq('application/cbor') + expect(context.http_request.headers['Accept']).to eq('application/cbor') + expect(context.http_request.endpoint.path) + .to end_with("/service/#{service_name}/operation/#{operation.name}") + expect(context.http_request.body.read).not_to be_empty + end end end @@ -78,20 +60,20 @@ def response(status_code:, body: '', protocol_header: 'rpc-v2-cbor') Http::Response.new(status_code: status_code, headers: headers, body: body) end - it 'returns nil for a successful response' do - context = build_context( + it 'returns nil when there is no error to raise' do + success = build_context( http_request: Http::Request.new(headers: request_headers), http_response: response(status_code: 200) ) - expect(protocol.parse_error(context)).to be_nil - end - - it 'returns nil for a response outside the HTTP status range (e.g. a signaled transport error)' do - context = build_context( + # status 0 => a signaled transport error passes through untouched + transport_error = build_context( http_request: Http::Request.new(headers: request_headers), http_response: response(status_code: 0, protocol_header: nil) ) - expect(protocol.parse_error(context)).to be_nil + aggregate_failures do + expect(protocol.parse_error(success)).to be_nil + expect(protocol.parse_error(transport_error)).to be_nil + end end it 'returns an error when the protocol header does not match (even on 2xx)' do @@ -116,17 +98,24 @@ def response(status_code:, body: '', protocol_header: 'rpc-v2-cbor') http_response: response(status_code: 500) ) error = protocol.parse_error(context) - expect(error).to be_a(StandardError) + aggregate_failures do + expect(error.class.name).to end_with('::HTTP500Error') + expect(error.code).to eq('HTTP500Error') + end end it 'extracts the modeled error from the __type in the body' do - body = Cbor.encode('__type' => 'smithy.ruby.tests#Error', 'message' => 'boom') + body = Smithy::Cbor.encode('__type' => 'smithy.ruby.tests#Error', 'message' => 'boom') context = build_context( http_request: Http::Request.new(headers: request_headers), http_response: response(status_code: 400, body: body) ) error = protocol.parse_error(context) - expect(error).to be_a(StandardError) + aggregate_failures do + expect(error.class.name).to end_with('::Error') + expect(error.code).to eq('Error') + expect(error.data.message).to eq('boom') + end end it 'falls back to an HTTP status error when the body is not valid CBOR' do @@ -135,17 +124,22 @@ def response(status_code:, body: '', protocol_header: 'rpc-v2-cbor') http_response: response(status_code: 400, body: 'not-cbor') ) error = protocol.parse_error(context) - expect(error).to be_a(StandardError) + expect(error.code).to eq('HTTP400Error') end end describe '#stub_data' do it 'builds a 200 CBOR response with protocol headers' do response = protocol.stub_data(config, operation, { string: 'hello' }) - expect(response.status_code).to eq(200) - expect(response.headers['Smithy-Protocol']).to eq('rpc-v2-cbor') - expect(response.headers['Content-Type']).to eq('application/cbor') - expect(response.body.read).not_to be_empty + body = response.body.read + decoded = Smithy::Cbor::Codec.new.parse(operation.output, body) + aggregate_failures do + expect(response.status_code).to eq(200) + expect(response.headers['Smithy-Protocol']).to eq('rpc-v2-cbor') + expect(response.headers['Content-Type']).to eq('application/cbor') + expect(body).not_to be_empty + expect(decoded[:string]).to eq('hello') + end end end From 4e962e74c6ac83013805319d94a0312bb0d2cf00 Mon Sep 17 00:00:00 2001 From: Juli Tera Date: Thu, 30 Jul 2026 13:24:13 -0700 Subject: [PATCH 10/15] Fully-qualify Smithy::Cbor references and document the raw error decode Qualify the bare Cbor constant to Smithy::Cbor for clarity and to avoid ambiguity if a Smithy::Client::Cbor is ever introduced. Add a comment explaining why extract_error uses a raw decode (to read __type before the modeled error shape is known) rather than the schema-aware codec. Drop a stale comment referencing the old ErrorHandler. --- gems/smithy-client/lib/smithy-client/rpc_v2_cbor.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/gems/smithy-client/lib/smithy-client/rpc_v2_cbor.rb b/gems/smithy-client/lib/smithy-client/rpc_v2_cbor.rb index 56f6f4a93..e7ef5e678 100644 --- a/gems/smithy-client/lib/smithy-client/rpc_v2_cbor.rb +++ b/gems/smithy-client/lib/smithy-client/rpc_v2_cbor.rb @@ -34,8 +34,7 @@ def parse_data(context) def parse_error(context) # Only inspect responses in the HTTP status range. Outside it (e.g. a # status 0 from a signaled transport error), leave the response alone - # so the transport error propagates untouched. Mirrors the old - # ErrorHandler's on_done(200..599) gate. + # so the transport error propagates untouched. return unless (200..599).cover?(context.http_response.status_code) # Malformed responses should raise an http-based error, so we validate @@ -73,7 +72,7 @@ def stub_error(_config, error_code) response.headers['Smithy-Protocol'] = 'rpc-v2-cbor' response.headers['Content-Type'] = 'application/cbor' data = { '__type' => "smithy.ruby.tests##{error_code}", 'message' => 'stubbed-error-message' } - response.body = Cbor.encode(data) + response.body = Smithy::Cbor.encode(data) response end @@ -144,11 +143,12 @@ def error(context) end def extract_error(body, context) - data = Cbor.decode(body) + # Raw decode to read __type before the modeled error shape is known (@codec.parse needs a shape; see below). + data = Smithy::Cbor.decode(body) code = error_code(context, data) data = parse_error_data(context, body, code) [code, data] - rescue Cbor::ParseError + rescue Smithy::Cbor::ParseError [http_status_error_code(context), Schema::EmptyStructure.new] end From 0a565b2ac28e83c4e2e5e74ca0834a42d74b2648 Mon Sep 17 00:00:00 2001 From: Juli Tera Date: Thu, 30 Jul 2026 13:24:23 -0700 Subject: [PATCH 11/15] Document protocol codegen wiring and drop redundant plugin add Add explanatory comments to the protocol codegen views and weld registry. Remove the now-redundant Protocol plugin add in the stub responses interface spec (it is a default plugin), keeping the registry override with a comment on why the Shapes fixture still needs it. --- gems/smithy/lib/smithy/views/client/client.rb | 1 + gems/smithy/lib/smithy/views/client/protocol.rb | 2 ++ gems/smithy/lib/smithy/welds/protocols.rb | 2 ++ gems/smithy/spec/interfaces/client/stub_responses_spec.rb | 5 ++++- 4 files changed, 9 insertions(+), 1 deletion(-) diff --git a/gems/smithy/lib/smithy/views/client/client.rb b/gems/smithy/lib/smithy/views/client/client.rb index 6837b01a1..617a4c415 100644 --- a/gems/smithy/lib/smithy/views/client/client.rb +++ b/gems/smithy/lib/smithy/views/client/client.rb @@ -24,6 +24,7 @@ def require_plugins requires end + # Mirrors require_plugins: emits a require line for each registered protocol's source file. def require_protocols requires = [] @protocols.each do |protocol| diff --git a/gems/smithy/lib/smithy/views/client/protocol.rb b/gems/smithy/lib/smithy/views/client/protocol.rb index 4bf9aa1ff..2eb6bff80 100644 --- a/gems/smithy/lib/smithy/views/client/protocol.rb +++ b/gems/smithy/lib/smithy/views/client/protocol.rb @@ -3,6 +3,8 @@ module Smithy module Views module Client + # A typed data-holder over a weld's protocol registry entry + # (name/class_name/require_path/require_relative), analogous to the plugin view. # @api private class Protocol def initialize(options = {}) diff --git a/gems/smithy/lib/smithy/welds/protocols.rb b/gems/smithy/lib/smithy/welds/protocols.rb index 7c3cbbd9a..b1ac71b08 100644 --- a/gems/smithy/lib/smithy/welds/protocols.rb +++ b/gems/smithy/lib/smithy/welds/protocols.rb @@ -28,6 +28,8 @@ def add_protocols when 'smithy.protocols#rpcv2Cbor' { rpc_v2_cbor: { + # class_name is emitted into the generated `protocols` hash and instantiated per client; + # require_path lets the generated client `require` the protocol's source file. class_name: Smithy::Client::RpcV2Cbor, require_path: 'smithy-client/rpc_v2_cbor' } diff --git a/gems/smithy/spec/interfaces/client/stub_responses_spec.rb b/gems/smithy/spec/interfaces/client/stub_responses_spec.rb index 74543433a..88e90a10d 100644 --- a/gems/smithy/spec/interfaces/client/stub_responses_spec.rb +++ b/gems/smithy/spec/interfaces/client/stub_responses_spec.rb @@ -36,7 +36,10 @@ end before(:all) do - Shapes::Client.add_plugin(Smithy::Client::Plugins::Protocol) + # The Shapes test fixture model has no smithy.protocols#rpcv2Cbor trait, + # so the Protocols weld never populates the protocol registry. Fake it + # here so CBOR stubbing is exercised. (The Protocol plugin itself is a + # default plugin now, so it does not need to be added manually.) Shapes::Client.define_singleton_method(:protocols) do { rpc_v2_cbor: Smithy::Client::RpcV2Cbor } end From 72558531c1ac9425c809ee06704adc0ca8428483 Mon Sep 17 00:00:00 2001 From: Juli Tera Date: Thu, 30 Jul 2026 13:47:08 -0700 Subject: [PATCH 12/15] Trim redundant CBOR spec assertions and inline sample client Drop the brittle error class-name checks (error.code already identifies the resolved error) and inline the single-use sample_client let into the client let. --- .../spec/smithy-client/rpc_v2_cbor_spec.rb | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/gems/smithy-client/spec/smithy-client/rpc_v2_cbor_spec.rb b/gems/smithy-client/spec/smithy-client/rpc_v2_cbor_spec.rb index c15d07156..a0e279134 100644 --- a/gems/smithy-client/spec/smithy-client/rpc_v2_cbor_spec.rb +++ b/gems/smithy-client/spec/smithy-client/rpc_v2_cbor_spec.rb @@ -7,8 +7,9 @@ module Client describe RpcV2Cbor do subject(:protocol) { described_class.new } - let(:sample_client) { ClientHelper.sample_client } - let(:client) { sample_client.const_get(:Client).new(endpoint: 'https://example.com', stub_responses: true) } + let(:client) do + ClientHelper.sample_client.const_get(:Client).new(endpoint: 'https://example.com', stub_responses: true) + end let(:operation) { client.config.service.operation(:operation) } let(:config) { client.config } @@ -98,10 +99,7 @@ def response(status_code:, body: '', protocol_header: 'rpc-v2-cbor') http_response: response(status_code: 500) ) error = protocol.parse_error(context) - aggregate_failures do - expect(error.class.name).to end_with('::HTTP500Error') - expect(error.code).to eq('HTTP500Error') - end + expect(error.code).to eq('HTTP500Error') end it 'extracts the modeled error from the __type in the body' do @@ -112,7 +110,6 @@ def response(status_code:, body: '', protocol_header: 'rpc-v2-cbor') ) error = protocol.parse_error(context) aggregate_failures do - expect(error.class.name).to end_with('::Error') expect(error.code).to eq('Error') expect(error.data.message).to eq('boom') end From 008f7fe9548f140eab0085ba2eefaa72286b2f84 Mon Sep 17 00:00:00 2001 From: Juli Tera Date: Thu, 30 Jul 2026 13:47:17 -0700 Subject: [PATCH 13/15] Drop redundant parse_error comment and clarify stub registry fake Remove the self-evident status-range comment on parse_error. Expand the stub responses spec comment to explain why the fake protocol registry is needed (NoOpProtocol's stubbing is a no-op, so the assertions would fail without it). --- gems/smithy-client/lib/smithy-client/rpc_v2_cbor.rb | 3 --- .../smithy/spec/interfaces/client/stub_responses_spec.rb | 9 ++++++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/gems/smithy-client/lib/smithy-client/rpc_v2_cbor.rb b/gems/smithy-client/lib/smithy-client/rpc_v2_cbor.rb index e7ef5e678..e6a61b0ba 100644 --- a/gems/smithy-client/lib/smithy-client/rpc_v2_cbor.rb +++ b/gems/smithy-client/lib/smithy-client/rpc_v2_cbor.rb @@ -32,9 +32,6 @@ def parse_data(context) # @param [HandlerContext] context # @return [StandardError, nil] def parse_error(context) - # Only inspect responses in the HTTP status range. Outside it (e.g. a - # status 0 from a signaled transport error), leave the response alone - # so the transport error propagates untouched. return unless (200..599).cover?(context.http_response.status_code) # Malformed responses should raise an http-based error, so we validate diff --git a/gems/smithy/spec/interfaces/client/stub_responses_spec.rb b/gems/smithy/spec/interfaces/client/stub_responses_spec.rb index 88e90a10d..21a8ba4fe 100644 --- a/gems/smithy/spec/interfaces/client/stub_responses_spec.rb +++ b/gems/smithy/spec/interfaces/client/stub_responses_spec.rb @@ -37,9 +37,12 @@ before(:all) do # The Shapes test fixture model has no smithy.protocols#rpcv2Cbor trait, - # so the Protocols weld never populates the protocol registry. Fake it - # here so CBOR stubbing is exercised. (The Protocol plugin itself is a - # default plugin now, so it does not need to be added manually.) + # so the Protocols weld never populates the protocol registry. Without + # this, the Protocol plugin resolves to NoOpProtocol, whose stub_data / + # stub_error are no-ops - the stub assertions below would get empty data + # and fail. Fake the registry so stubbing runs through real CBOR. (The + # Protocol plugin itself is a default plugin now, so it does not need to + # be added manually.) Shapes::Client.define_singleton_method(:protocols) do { rpc_v2_cbor: Smithy::Client::RpcV2Cbor } end From 2c75556363fa58fce8b08cca1f0c7f03b651a68f Mon Sep 17 00:00:00 2001 From: Juli Tera Date: Thu, 30 Jul 2026 14:28:07 -0700 Subject: [PATCH 14/15] Document supported protocols in the :protocol option docs Append the service's registered protocols (first is the default) to the generated :protocol option docstring, so each client lists exactly what it supports. Emits nothing when no protocol is registered. --- gems/smithy/lib/smithy/views/client/client.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/gems/smithy/lib/smithy/views/client/client.rb b/gems/smithy/lib/smithy/views/client/client.rb index 617a4c415..94c0bc03f 100644 --- a/gems/smithy/lib/smithy/views/client/client.rb +++ b/gems/smithy/lib/smithy/views/client/client.rb @@ -115,9 +115,20 @@ def option_docstrings(option) lines << option_tag(option) documentation = option.docstring.split("\n").map { |line| " #{line}" } lines.concat(documentation) + lines.concat(protocol_docstrings) if option.name == :protocol lines end + # Appends the service's registered protocols to the +:protocol+ option + # docs so the generated client lists exactly what it supports (the + # first is the default). Emits nothing when no protocol is registered. + def protocol_docstrings + return [] if @protocols.empty? + + names = @protocols.map { |protocol| "+:#{protocol.name}+" } + [" Supported protocols: #{names.join(', ')} (defaults to #{names.first})."] + end + def option_tag(option) tag = StringIO.new tag << '@option options' From d64f0ab1bfe62600fe53da975ce0fb14863cdedd Mon Sep 17 00:00:00 2001 From: Juli Tera Date: Thu, 30 Jul 2026 14:36:21 -0700 Subject: [PATCH 15/15] Drop unused require_relative from the protocol view Protocols always ship in an installed gem and are required by absolute path, so the require_relative variant (copied from the plugin view, where code-generated plugins need it) was dead code. Simplify the protocol view and require_protocols to emit a plain require. --- gems/smithy/lib/smithy/views/client/client.rb | 7 ++++--- gems/smithy/lib/smithy/views/client/protocol.rb | 8 ++------ 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/gems/smithy/lib/smithy/views/client/client.rb b/gems/smithy/lib/smithy/views/client/client.rb index 94c0bc03f..f14090599 100644 --- a/gems/smithy/lib/smithy/views/client/client.rb +++ b/gems/smithy/lib/smithy/views/client/client.rb @@ -24,14 +24,15 @@ def require_plugins requires end - # Mirrors require_plugins: emits a require line for each registered protocol's source file. + # Emits a require line for each registered protocol's source file. + # Protocols always ship in an installed gem, so they are required by + # absolute path (unlike code-generated plugins, which are relative). def require_protocols requires = [] @protocols.each do |protocol| next unless protocol.require_path - next if !@plan.destination_root && protocol.require_relative? - requires << "require#{'_relative' if protocol.require_relative?} '#{protocol.require_path}'" + requires << "require '#{protocol.require_path}'" end requires end diff --git a/gems/smithy/lib/smithy/views/client/protocol.rb b/gems/smithy/lib/smithy/views/client/protocol.rb index 2eb6bff80..0b8013b6b 100644 --- a/gems/smithy/lib/smithy/views/client/protocol.rb +++ b/gems/smithy/lib/smithy/views/client/protocol.rb @@ -4,21 +4,17 @@ module Smithy module Views module Client # A typed data-holder over a weld's protocol registry entry - # (name/class_name/require_path/require_relative), analogous to the plugin view. + # (name/class_name/require_path). Protocols ship in an installed gem, so + # unlike the plugin view there is no require_relative variant. # @api private class Protocol def initialize(options = {}) @name = options[:name] @class_name = options[:class_name] @require_path = options[:require_path] - @require_relative = options.fetch(:require_relative, false) end attr_accessor :name, :class_name, :require_path - - def require_relative? - @require_relative - end end end end