From 8661a43f784e6c2689466355dd5a0bdab4567346 Mon Sep 17 00:00:00 2001 From: Tim Emiola Date: Fri, 17 Apr 2015 13:29:59 -0700 Subject: Propagate metadata in BadStatus - allow BadStatus to contain metadata that's populated by keyword args - on servers, convert raised BadStatus metadata to trailers - on clients, convert trailers to BadStatus metadata when raising BadStatus --- src/ruby/.rubocop_todo.yml | 8 +-- src/ruby/lib/grpc/generic/active_call.rb | 13 +++-- src/ruby/lib/grpc/generic/rpc_desc.rb | 10 ++-- src/ruby/spec/generic/rpc_desc_spec.rb | 88 ++++++++++++++------------------ src/ruby/spec/generic/rpc_server_spec.rb | 56 +++++++++++++++++++- 5 files changed, 113 insertions(+), 62 deletions(-) (limited to 'src') diff --git a/src/ruby/.rubocop_todo.yml b/src/ruby/.rubocop_todo.yml index 02136a81a9..6b8f336055 100644 --- a/src/ruby/.rubocop_todo.yml +++ b/src/ruby/.rubocop_todo.yml @@ -1,18 +1,18 @@ # This configuration was generated by `rubocop --auto-gen-config` -# on 2015-04-16 12:30:09 -0700 using RuboCop version 0.30.0. +# on 2015-04-17 12:36:26 -0700 using RuboCop version 0.30.0. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new # versions of RuboCop, may require this file to be generated again. -# Offense count: 34 +# Offense count: 30 Metrics/AbcSize: - Max: 36 + Max: 37 # Offense count: 3 # Configuration parameters: CountComments. Metrics/ClassLength: - Max: 185 + Max: 179 # Offense count: 35 # Configuration parameters: CountComments. diff --git a/src/ruby/lib/grpc/generic/active_call.rb b/src/ruby/lib/grpc/generic/active_call.rb index 8d63de4145..274372e4d5 100644 --- a/src/ruby/lib/grpc/generic/active_call.rb +++ b/src/ruby/lib/grpc/generic/active_call.rb @@ -39,7 +39,10 @@ class Struct return nil if status.nil? fail GRPC::Cancelled if status.code == GRPC::Core::StatusCodes::CANCELLED if status.code != GRPC::Core::StatusCodes::OK - fail GRPC::BadStatus.new(status.code, status.details) + # raise BadStatus, propagating the metadata if present. + md = status.metadata + with_sym_keys = Hash[md.each_pair.collect { |x, y| [x.to_sym, y] }] + fail GRPC::BadStatus.new(status.code, status.details, **with_sym_keys) end status end @@ -192,9 +195,13 @@ module GRPC # @param details [String] details # @param assert_finished [true, false] when true(default), waits for # FINISHED. - def send_status(code = OK, details = '', assert_finished = false) + # + # == Keyword Arguments == + # any keyword arguments are treated as metadata to be sent to the server + # if a keyword value is a list, multiple metadata for it's key are sent + def send_status(code = OK, details = '', assert_finished = false, **kw) ops = { - SEND_STATUS_FROM_SERVER => Struct::Status.new(code, details) + SEND_STATUS_FROM_SERVER => Struct::Status.new(code, details, kw) } ops[RECV_CLOSE_ON_SERVER] = nil if assert_finished @call.run_batch(@cq, self, INFINITE_FUTURE, ops) diff --git a/src/ruby/lib/grpc/generic/rpc_desc.rb b/src/ruby/lib/grpc/generic/rpc_desc.rb index 3e48b8e51d..22b80d8938 100644 --- a/src/ruby/lib/grpc/generic/rpc_desc.rb +++ b/src/ruby/lib/grpc/generic/rpc_desc.rb @@ -82,10 +82,10 @@ module GRPC end send_status(active_call, OK, 'OK') rescue BadStatus => e - # this is raised by handlers that want GRPC to send an application - # error code and detail message. + # this is raised by handlers that want GRPC to send an application error + # code and detail message and some additional app-specific metadata. logger.debug("app err: #{active_call}, status:#{e.code}:#{e.details}") - send_status(active_call, e.code, e.details) + send_status(active_call, e.code, e.details, **e.metadata) rescue Core::CallError => e # This is raised by GRPC internals but should rarely, if ever happen. # Log it, but don't notify the other endpoint.. @@ -135,9 +135,9 @@ module GRPC "##{mth.name}: bad arg count; got:#{mth.arity}, want:#{want}, #{msg}" end - def send_status(active_client, code, details) + def send_status(active_client, code, details, **kw) details = 'Not sure why' if details.nil? - active_client.send_status(code, details, code == OK) + active_client.send_status(code, details, code == OK, **kw) rescue StandardError => e logger.warn("Could not send status #{code}:#{details}") logger.warn(e) diff --git a/src/ruby/spec/generic/rpc_desc_spec.rb b/src/ruby/spec/generic/rpc_desc_spec.rb index a68299465c..e5d05c8b9b 100644 --- a/src/ruby/spec/generic/rpc_desc_spec.rb +++ b/src/ruby/spec/generic/rpc_desc_spec.rb @@ -52,41 +52,47 @@ describe GRPC::RpcDesc do @ok_response = Object.new end + shared_examples 'it handles errors' do + it 'sends the specified status if BadStatus is raised' do + expect(@call).to receive(:remote_read).once.and_return(Object.new) + expect(@call).to receive(:send_status).once.with(@bs_code, 'NOK', false, + {}) + this_desc.run_server_method(@call, method(:bad_status)) + end + + it 'sends status UNKNOWN if other StandardErrors are raised' do + expect(@call).to receive(:remote_read).once.and_return(Object.new) + expect(@call).to receive(:send_status) .once.with(UNKNOWN, @no_reason, + false, {}) + this_desc.run_server_method(@call, method(:other_error)) + end + + it 'absorbs CallError with no further action' do + expect(@call).to receive(:remote_read).once.and_raise(CallError) + blk = proc do + this_desc.run_server_method(@call, method(:fake_reqresp)) + end + expect(&blk).to_not raise_error + end + end + describe '#run_server_method' do describe 'for request responses' do + let(:this_desc) { @request_response } before(:each) do @call = double('active_call') allow(@call).to receive(:single_req_view).and_return(@call) allow(@call).to receive(:gc) end - it 'sends the specified status if BadStatus is raised' do - expect(@call).to receive(:remote_read).once.and_return(Object.new) - expect(@call).to receive(:send_status).once.with(@bs_code, 'NOK', false) - @request_response.run_server_method(@call, method(:bad_status)) - end - - it 'sends status UNKNOWN if other StandardErrors are raised' do - expect(@call).to receive(:remote_read).once.and_return(Object.new) - expect(@call).to receive(:send_status) .once.with(UNKNOWN, @no_reason, - false) - @request_response.run_server_method(@call, method(:other_error)) - end - - it 'absorbs CallError with no further action' do - expect(@call).to receive(:remote_read).once.and_raise(CallError) - blk = proc do - @request_response.run_server_method(@call, method(:fake_reqresp)) - end - expect(&blk).to_not raise_error - end + it_behaves_like 'it handles errors' it 'sends a response and closes the stream if there no errors' do req = Object.new expect(@call).to receive(:remote_read).once.and_return(req) expect(@call).to receive(:remote_send).once.with(@ok_response) - expect(@call).to receive(:send_status).once.with(OK, 'OK', true) - @request_response.run_server_method(@call, method(:fake_reqresp)) + expect(@call).to receive(:send_status).once.with(OK, 'OK', true, {}) + this_desc.run_server_method(@call, method(:fake_reqresp)) end end @@ -98,13 +104,14 @@ describe GRPC::RpcDesc do end it 'sends the specified status if BadStatus is raised' do - expect(@call).to receive(:send_status).once.with(@bs_code, 'NOK', false) + expect(@call).to receive(:send_status).once.with(@bs_code, 'NOK', false, + {}) @client_streamer.run_server_method(@call, method(:bad_status_alt)) end it 'sends status UNKNOWN if other StandardErrors are raised' do expect(@call).to receive(:send_status) .once.with(UNKNOWN, @no_reason, - false) + false, {}) @client_streamer.run_server_method(@call, method(:other_error_alt)) end @@ -118,44 +125,26 @@ describe GRPC::RpcDesc do it 'sends a response and closes the stream if there no errors' do expect(@call).to receive(:remote_send).once.with(@ok_response) - expect(@call).to receive(:send_status).once.with(OK, 'OK', true) + expect(@call).to receive(:send_status).once.with(OK, 'OK', true, {}) @client_streamer.run_server_method(@call, method(:fake_clstream)) end end describe 'for server streaming' do + let(:this_desc) { @request_response } before(:each) do @call = double('active_call') allow(@call).to receive(:single_req_view).and_return(@call) allow(@call).to receive(:gc) end - it 'sends the specified status if BadStatus is raised' do - expect(@call).to receive(:remote_read).once.and_return(Object.new) - expect(@call).to receive(:send_status).once.with(@bs_code, 'NOK', false) - @server_streamer.run_server_method(@call, method(:bad_status)) - end - - it 'sends status UNKNOWN if other StandardErrors are raised' do - expect(@call).to receive(:remote_read).once.and_return(Object.new) - expect(@call).to receive(:send_status) .once.with(UNKNOWN, @no_reason, - false) - @server_streamer.run_server_method(@call, method(:other_error)) - end - - it 'absorbs CallError with no further action' do - expect(@call).to receive(:remote_read).once.and_raise(CallError) - blk = proc do - @server_streamer.run_server_method(@call, method(:fake_svstream)) - end - expect(&blk).to_not raise_error - end + it_behaves_like 'it handles errors' it 'sends a response and closes the stream if there no errors' do req = Object.new expect(@call).to receive(:remote_read).once.and_return(req) expect(@call).to receive(:remote_send).twice.with(@ok_response) - expect(@call).to receive(:send_status).once.with(OK, 'OK', true) + expect(@call).to receive(:send_status).once.with(OK, 'OK', true, {}) @server_streamer.run_server_method(@call, method(:fake_svstream)) end end @@ -172,20 +161,21 @@ describe GRPC::RpcDesc do it 'sends the specified status if BadStatus is raised' do e = GRPC::BadStatus.new(@bs_code, 'NOK') expect(@call).to receive(:run_server_bidi).and_raise(e) - expect(@call).to receive(:send_status).once.with(@bs_code, 'NOK', false) + expect(@call).to receive(:send_status).once.with(@bs_code, 'NOK', false, + {}) @bidi_streamer.run_server_method(@call, method(:bad_status_alt)) end it 'sends status UNKNOWN if other StandardErrors are raised' do expect(@call).to receive(:run_server_bidi).and_raise(StandardError) expect(@call).to receive(:send_status).once.with(UNKNOWN, @no_reason, - false) + false, {}) @bidi_streamer.run_server_method(@call, method(:other_error_alt)) end it 'closes the stream if there no errors' do expect(@call).to receive(:run_server_bidi) - expect(@call).to receive(:send_status).once.with(OK, 'OK', true) + expect(@call).to receive(:send_status).once.with(OK, 'OK', true, {}) @bidi_streamer.run_server_method(@call, method(:fake_bidistream)) end end diff --git a/src/ruby/spec/generic/rpc_server_spec.rb b/src/ruby/spec/generic/rpc_server_spec.rb index 202576c673..f1d95be553 100644 --- a/src/ruby/spec/generic/rpc_server_spec.rb +++ b/src/ruby/spec/generic/rpc_server_spec.rb @@ -58,7 +58,7 @@ class NoRpcImplementation rpc :an_rpc, EchoMsg, EchoMsg end -# A test service with an implementation. +# A test service with an echo implementation. class EchoService include GRPC::GenericService rpc :an_rpc, EchoMsg, EchoMsg @@ -77,6 +77,25 @@ end EchoStub = EchoService.rpc_stub_class +# A test service with an implementation that fails with BadStatus +class FailingService + include GRPC::GenericService + rpc :an_rpc, EchoMsg, EchoMsg + attr_reader :details, :code, :md + + def initialize(_default_var = 'ignored') + @details = 'app error' + @code = 101 + @md = { failed_method: 'an_rpc' } + end + + def an_rpc(_req, _call) + fail GRPC::BadStatus.new(@code, @details, **@md) + end +end + +FailingStub = FailingService.rpc_stub_class + # A slow test service. class SlowService include GRPC::GenericService @@ -514,5 +533,40 @@ describe GRPC::RpcServer do t.join end end + + context 'with metadata on failing' do + before(:each) do + server_opts = { + server_override: @server, + completion_queue_override: @server_queue, + poll_period: 1 + } + @srv = RpcServer.new(**server_opts) + end + + it 'should send receive metadata failed response', server: true do + service = FailingService.new + @srv.handle(service) + t = Thread.new { @srv.run } + @srv.wait_till_running + req = EchoMsg.new + stub = FailingStub.new(@host, **client_opts) + blk = proc { stub.an_rpc(req) } + + # confirm it raise the expected error + expect(&blk).to raise_error GRPC::BadStatus + + # call again and confirm exception has the expected fields + begin + blk.call + rescue GRPC::BadStatus => e + expect(e.code).to eq(service.code) + expect(e.details).to eq(service.details) + expect(e.metadata).to eq(service.md) + end + @srv.stop + t.join + end + end end end -- cgit v1.2.3