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.
pull/12192/head
John Millikin 7 years ago
parent e60c0f82b5
commit e387852303
No known key found for this signature in database
GPG Key ID: 1F7686B8DA217791
  1. 6
      src/ruby/lib/grpc/generic/rpc_desc.rb
  2. 38
      src/ruby/spec/generic/rpc_desc_spec.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}")

@ -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

Loading…
Cancel
Save