From e3878523034dfa64e7c5525ea7afe1127f73c19b Mon Sep 17 00:00:00 2001
From: John Millikin <jmillikin@stripe.com>
Date: Tue, 15 Aug 2017 14:35:38 -0700
Subject: [PATCH] Catch NotImplementedError exceptions and forward them to the
 client.

The old code only caught `StandardError`, which doesn't include
`NotImplementedError`. Despite the name, this error indicates a failure
of low-level OS interaction rather than unimplemented user code.

Any errors not caught by this section will cause the server to
terminate, which is generally undesirable because it might be happily
handling other requests.
---
 src/ruby/lib/grpc/generic/rpc_desc.rb  |  6 +++-
 src/ruby/spec/generic/rpc_desc_spec.rb | 38 ++++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/src/ruby/lib/grpc/generic/rpc_desc.rb b/src/ruby/lib/grpc/generic/rpc_desc.rb
index 89cf8ff6a0a..6fb6c412fb1 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 be578c40d3f..6852b9804fe 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