From 174aa915bae4b379932e4887e26f2953db52b954 Mon Sep 17 00:00:00 2001
From: Alexander Polcyn <apolcyn@google.com>
Date: Wed, 30 Nov 2016 08:35:52 -0800
Subject: [PATCH] change client code to use specific exceptions and throw bad
status if unkown code
---
src/ruby/lib/grpc/errors.rb | 59 +++++++++--------------
src/ruby/lib/grpc/generic/active_call.rb | 3 +-
src/ruby/lib/grpc/generic/service.rb | 3 +-
src/ruby/pb/grpc/health/checker.rb | 4 +-
src/ruby/pb/test/client.rb | 7 +--
src/ruby/spec/error_sanity_spec.rb | 2 +-
src/ruby/spec/generic/client_stub_spec.rb | 9 ++--
src/ruby/spec/generic/rpc_server_spec.rb | 5 +-
src/ruby/spec/pb/health/checker_spec.rb | 8 +--
9 files changed, 43 insertions(+), 57 deletions(-)
diff --git a/src/ruby/lib/grpc/errors.rb b/src/ruby/lib/grpc/errors.rb
index 41c15de1d5..f6998e17c4 100644
--- a/src/ruby/lib/grpc/errors.rb
+++ b/src/ruby/lib/grpc/errors.rb
@@ -66,43 +66,30 @@ module GRPC
end
def self.new_status_exception(code, details = 'unkown cause', metadata = {})
- case code
- when OK
- Ok.new(details, metadata)
- when CANCELLED
- Cancelled.new(details, metadata)
- when UNKNOWN
- Unknown.new(details, metadata)
- when INVALID_ARGUMENT
- InvalidArgument.new(details, metadata)
- when DEADLINE_EXCEEDED
- DeadlineExceeded.new(details, metadata)
- when NOT_FOUND
- NotFound.new(details, metadata)
- when ALREADY_EXISTS
- AlreadyExists.new(details, metadata)
- when PERMISSION_DENIED
- PermissionDenied.new(details, metadata)
- when UNAUTHENTICATED
- Unauthenticated.new(details, metadata)
- when RESOURCE_EXHAUSTED
- ResourceExhausted.new(details, metadata)
- when FAILED_PRECONDITION
- FailedPrecondition.new(details, metadata)
- when ABORTED
- Aborted.new(details, metadata)
- when OUT_OF_RANGE
- OutOfRange.new(details, metadata)
- when UNIMPLEMENTED
- Unimplemented.new(details, metadata)
- when INTERNAL
- Internal.new(details, metadata)
- when UNAVAILABLE
- Unavailable.new(details, metadata)
- when DATA_LOSS
- DataLoss.new(details, metadata)
+ codes = {}
+ codes[OK] = Ok
+ codes[CANCELLED] = Cancelled
+ codes[UNKNOWN] = Unknown
+ codes[INVALID_ARGUMENT] = InvalidArgument
+ codes[DEADLINE_EXCEEDED] = DeadlineExceeded
+ codes[NOT_FOUND] = NotFound
+ codes[ALREADY_EXISTS] = AlreadyExists
+ codes[PERMISSION_DENIED] = PermissionDenied
+ codes[UNAUTHENTICATED] = Unauthenticated
+ codes[RESOURCE_EXHAUSTED] = ResourceExhausted
+ codes[FAILED_PRECONDITION] = FailedPrecondition
+ codes[ABORTED] = Aborted
+ codes[OUT_OF_RANGE] = OutOfRange
+ codes[UNIMPLEMENTED] = Unimplemented
+ codes[INTERNAL] = Internal
+ codes[UNIMPLEMENTED] = Unimplemented
+ codes[UNAVAILABLE] = Unavailable
+ codes[DATA_LOSS] = DataLoss
+
+ if codes[code].nil?
+ BadStatus.new(code, details, metadata)
else
- fail 'unknown code'
+ codes[code].new(details, metadata)
end
end
end
diff --git a/src/ruby/lib/grpc/generic/active_call.rb b/src/ruby/lib/grpc/generic/active_call.rb
index 5787f53463..01797d13e1 100644
--- a/src/ruby/lib/grpc/generic/active_call.rb
+++ b/src/ruby/lib/grpc/generic/active_call.rb
@@ -43,7 +43,8 @@ class Struct
GRPC.logger.debug("Failing with status #{status}")
# raise BadStatus, propagating the metadata if present.
md = status.metadata
- fail GRPC::BadStatus.new(status.code, status.details, md)
+ fail GRPC::BadStatus.new_status_exception(
+ status.code, status.details, md)
end
status
end
diff --git a/src/ruby/lib/grpc/generic/service.rb b/src/ruby/lib/grpc/generic/service.rb
index 7cb9f1cc99..0dbadcc19e 100644
--- a/src/ruby/lib/grpc/generic/service.rb
+++ b/src/ruby/lib/grpc/generic/service.rb
@@ -111,7 +111,8 @@ module GRPC
marshal_class_method,
unmarshal_class_method)
define_method(name) do
- fail GRPC::BadStatus, GRPC::Core::StatusCodes::UNIMPLEMENTED
+ fail GRPC::BadStatus.new_status_exception(
+ GRPC::Core::StatusCodes::UNIMPLEMENTED)
end
end
diff --git a/src/ruby/pb/grpc/health/checker.rb b/src/ruby/pb/grpc/health/checker.rb
index 4bce1744c4..6b2d852ebf 100644
--- a/src/ruby/pb/grpc/health/checker.rb
+++ b/src/ruby/pb/grpc/health/checker.rb
@@ -52,7 +52,9 @@ module Grpc
@status_mutex.synchronize do
status = @statuses["#{req.service}"]
end
- fail GRPC::BadStatus, StatusCodes::NOT_FOUND if status.nil?
+ if status.nil?
+ fail GRPC::BadStatus.new_status_exception(StatusCodes::NOT_FOUND)
+ end
HealthCheckResponse.new(status: status)
end
diff --git a/src/ruby/pb/test/client.rb b/src/ruby/pb/test/client.rb
index b9af160e7a..9ee5cdf9fd 100755
--- a/src/ruby/pb/test/client.rb
+++ b/src/ruby/pb/test/client.rb
@@ -338,11 +338,8 @@ class NamedTests
deadline = GRPC::Core::TimeConsts::from_relative_time(1)
resps = @stub.full_duplex_call(enum.each_item, deadline: deadline)
resps.each { } # wait to receive each request (or timeout)
- fail 'Should have raised GRPC::BadStatus(DEADLINE_EXCEEDED)'
- rescue GRPC::BadStatus => e
- assert("#{__callee__}: status was wrong") do
- e.code == GRPC::Core::StatusCodes::DEADLINE_EXCEEDED
- end
+ fail 'Should have raised GRPC::DeadlineExceeded'
+ rescue GRPC::DeadlineExceeded
end
def empty_stream
diff --git a/src/ruby/spec/error_sanity_spec.rb b/src/ruby/spec/error_sanity_spec.rb
index ca2d80e685..77e94a8816 100644
--- a/src/ruby/spec/error_sanity_spec.rb
+++ b/src/ruby/spec/error_sanity_spec.rb
@@ -40,7 +40,7 @@ describe StatusCodes do
StatusCodes.constants.each do |status_name|
it 'there is a subclass of BadStatus corresponding to StatusCode: ' \
- "#{name} that has code: #{StatusCodes.const_get(status_name)}" do
+ "#{status_name} that has code: #{StatusCodes.const_get(status_name)}" do
camel_case = upper_snake_to_camel(status_name)
error_class = GRPC.const_get(camel_case)
# expect the error class to be a subclass of BadStatus
diff --git a/src/ruby/spec/generic/client_stub_spec.rb b/src/ruby/spec/generic/client_stub_spec.rb
index 9c4e9cbd07..08a171e299 100644
--- a/src/ruby/spec/generic/client_stub_spec.rb
+++ b/src/ruby/spec/generic/client_stub_spec.rb
@@ -190,15 +190,14 @@ describe 'ClientStub' do
end
creds = GRPC::Core::CallCredentials.new(failing_auth)
- error_occured = false
+ unauth_error_occured = false
begin
get_response(stub, credentials: creds)
- rescue GRPC::BadStatus => e
- error_occured = true
- expect(e.code).to eq(GRPC::Core::StatusCodes::UNAUTHENTICATED)
+ rescue GRPC::Unauthenticated => e
+ unauth_error_occured = true
expect(e.details.include?(error_message)).to be true
end
- expect(error_occured).to eq(true)
+ expect(unauth_error_occured).to eq(true)
# Kill the server thread so tests can complete
th.kill
diff --git a/src/ruby/spec/generic/rpc_server_spec.rb b/src/ruby/spec/generic/rpc_server_spec.rb
index 31157cf161..1689d2df68 100644
--- a/src/ruby/spec/generic/rpc_server_spec.rb
+++ b/src/ruby/spec/generic/rpc_server_spec.rb
@@ -414,9 +414,8 @@ describe GRPC::RpcServer do
stub = SlowStub.new(alt_host, :this_channel_is_insecure)
begin
stub.an_rpc(req)
- rescue GRPC::BadStatus => e
- one_failed_as_unavailable =
- e.code == StatusCodes::RESOURCE_EXHAUSTED
+ rescue GRPC::ResourceExhausted
+ one_failed_as_unavailable = true
end
end
end
diff --git a/src/ruby/spec/pb/health/checker_spec.rb b/src/ruby/spec/pb/health/checker_spec.rb
index 1b2fa96827..2cc58f845b 100644
--- a/src/ruby/spec/pb/health/checker_spec.rb
+++ b/src/ruby/spec/pb/health/checker_spec.rb
@@ -119,7 +119,7 @@ describe Grpc::Health::Checker do
subject.check(HCReq.new(service: t[:service]), nil)
end
expected_msg = /#{StatusCodes::NOT_FOUND}/
- expect(&blk).to raise_error GRPC::BadStatus, expected_msg
+ expect(&blk).to raise_error GRPC::NotFound, expected_msg
end
end
end
@@ -137,7 +137,7 @@ describe Grpc::Health::Checker do
subject.check(HCReq.new(service: t[:service]), nil)
end
expected_msg = /#{StatusCodes::NOT_FOUND}/
- expect(&blk).to raise_error GRPC::BadStatus, expected_msg
+ expect(&blk).to raise_error GRPC::NotFound, expected_msg
end
end
end
@@ -158,7 +158,7 @@ describe Grpc::Health::Checker do
subject.check(HCReq.new(service: t[:service]), nil)
end
expected_msg = /#{StatusCodes::NOT_FOUND}/
- expect(&blk).to raise_error GRPC::BadStatus, expected_msg
+ expect(&blk).to raise_error GRPC::NotFound, expected_msg
end
end
end
@@ -206,7 +206,7 @@ describe Grpc::Health::Checker do
stub.check(HCReq.new(service: 'unknown'))
end
expected_msg = /#{StatusCodes::NOT_FOUND}/
- expect(&blk).to raise_error GRPC::BadStatus, expected_msg
+ expect(&blk).to raise_error GRPC::NotFound, expected_msg
@srv.stop
t.join
end
--
GitLab