Merge pull request #12192 from jmillikin-stripe/ruby-catch-script-error

Catch NotImplementedError exceptions and forward them to the client.

2 files changed, 43 insertions, 1 deletions

diff --git a/src/ruby/lib/grpc/generic/rpc_desc.rb b/src/ruby/lib/grpc/generic/rpc_desc.rb
index 89cf8ff6a0..6fb6c412fb 100644
--- a/src/ruby/lib/grpc/generic/rpc_desc.rb
+++ b/src/ruby/lib/grpc/generic/rpc_desc.rb
@@ -99,9 +99,13 @@ module GRPC
       # event.  Send a status of deadline exceeded
       GRPC.logger.warn("late call: #{active_call}")
       send_status(active_call, DEADLINE_EXCEEDED, 'late')
-    rescue StandardError => e
+    rescue StandardError, NotImplementedError => e
       # This will usuaally be an unhandled error in the handling code.
       # Send back a UNKNOWN status to the client
+      #
+      # Note: this intentionally does not map NotImplementedError to
+      # UNIMPLEMENTED because NotImplementedError is intended for low-level
+      # OS interaction (e.g. syscalls) not supported by the current OS.
       GRPC.logger.warn("failed handler: #{active_call}; sending status:UNKNOWN")
       GRPC.logger.warn(e)
       send_status(active_call, UNKNOWN, "#{e.class}: #{e.message}")
diff --git a/src/ruby/spec/generic/rpc_desc_spec.rb b/src/ruby/spec/generic/rpc_desc_spec.rb
index be578c40d3..6852b9804f 100644
--- a/src/ruby/spec/generic/rpc_desc_spec.rb
+++ b/src/ruby/spec/generic/rpc_desc_spec.rb
@@ -52,6 +52,13 @@ describe GRPC::RpcDesc do
       this_desc.run_server_method(@call, method(:other_error))
     end
 
+    it 'sends status UNKNOWN if NotImplementedErrors are raised' do
+      expect(@call).to receive(:read_unary_request).once.and_return(Object.new)
+      expect(@call).to receive(:send_status).once.with(
+        UNKNOWN, not_implemented_error_msg, false, metadata: {})
+      this_desc.run_server_method(@call, method(:not_implemented))
+    end
+
     it 'absorbs CallError with no further action' do
       expect(@call).to receive(:read_unary_request).once.and_raise(CallError)
       blk = proc do
@@ -102,6 +109,12 @@ describe GRPC::RpcDesc do
         @client_streamer.run_server_method(@call, method(:other_error_alt))
       end
 
+      it 'sends status UNKNOWN if NotImplementedErrors are raised' do
+        expect(@call).to receive(:send_status).once.with(
+          UNKNOWN, not_implemented_error_msg, false, metadata: {})
+        @client_streamer.run_server_method(@call, method(:not_implemented_alt))
+      end
+
       it 'absorbs CallError with no further action' do
         expect(@call).to receive(:server_unary_response).once.and_raise(
           CallError)
@@ -166,6 +179,14 @@ describe GRPC::RpcDesc do
         @bidi_streamer.run_server_method(@call, method(:other_error_alt))
       end
 
+      it 'sends status UNKNOWN if NotImplementedErrors are raised' do
+        expect(@call).to receive(:run_server_bidi).and_raise(
+          not_implemented_error)
+        expect(@call).to receive(:send_status).once.with(
+          UNKNOWN, not_implemented_error_msg, false, metadata: {})
+        @bidi_streamer.run_server_method(@call, method(:not_implemented_alt))
+      end
+
       it 'closes the stream if there no errors' do
         expect(@call).to receive(:run_server_bidi)
         expect(@call).to receive(:output_metadata).and_return(fake_md)
@@ -329,8 +350,25 @@ describe GRPC::RpcDesc do
     fail(ArgumentError, 'other error')
   end
 
+  def not_implemented(_req, _call)
+    fail not_implemented_error
+  end
+
+  def not_implemented_alt(_call)
+    fail not_implemented_error
+  end
+
   def arg_error_msg(error = nil)
     error ||= ArgumentError.new('other error')
     "#{error.class}: #{error.message}"
   end
+
+  def not_implemented_error
+    NotImplementedError.new('some OS feature not implemented')
+  end
+
+  def not_implemented_error_msg(error = nil)
+    error ||= not_implemented_error
+    "#{error.class}: #{error.message}"
+  end
 end