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

5 files changed, 113 insertions, 62 deletions

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