From 6002b8ff63b46afc8abb34081423692e2f02d2b3 Mon Sep 17 00:00:00 2001 From: Alexander Polcyn Date: Mon, 28 Nov 2016 17:41:13 -0800 Subject: [PATCH 1/3] add ruby subclasses of bad status for each GPRC status code --- src/ruby/lib/grpc/errors.rb | 126 ++++++++++++++++++++++++++++- src/ruby/spec/error_sanity_spec.rb | 58 +++++++++++++ 2 files changed, 182 insertions(+), 2 deletions(-) create mode 100644 src/ruby/spec/error_sanity_spec.rb diff --git a/src/ruby/lib/grpc/errors.rb b/src/ruby/lib/grpc/errors.rb index 23b2bb7e127..022a14b0a51 100644 --- a/src/ruby/lib/grpc/errors.rb +++ b/src/ruby/lib/grpc/errors.rb @@ -35,6 +35,13 @@ module GRPC # either end of a GRPC connection. When raised, it indicates that a status # error should be returned to the other end of a GRPC connection; when # caught it means that this end received a status error. + # + # There is also subclass of BadStatus in this module for each GRPC status. + # E.g., the GRPC::Cancelled class corresponds to status CANCELLED. + # + # See + # https://github.com/grpc/grpc/blob/master/include/grpc/impl/codegen/status.h + # for detailed descriptions of each status code. class BadStatus < StandardError attr_reader :code, :details, :metadata @@ -57,7 +64,122 @@ module GRPC end end - # Cancelled is an exception class that indicates that an rpc was cancelled. - class Cancelled < StandardError + # GRPC status code corresponding to status OK + class Ok < BadStatus + def initialize(details = 'unknown cause', metadata = {}) + super(Core::StatusCodes::OK, details, metadata) + end + end + + # GRPC status code corresponding to status CANCELLED + class Cancelled < BadStatus + def initialize(details = 'unknown cause', metadata = {}) + super(Core::StatusCodes::CANCELLED, details, metadata) + end + end + + # GRPC status code corresponding to status UNKNOWN + class Unknown < BadStatus + def initialize(details = 'unknown cause', metadata = {}) + super(Core::StatusCodes::UNKNOWN, details, metadata) + end + end + + # GRPC status code corresponding to status INVALID_ARGUMENT + class InvalidArgument < BadStatus + def initialize(details = 'unknown cause', metadata = {}) + super(Core::StatusCodes::INVALID_ARGUMENT, details, metadata) + end + end + + # GRPC status code corresponding to status DEADLINE_EXCEEDED + class DeadlineExceeded < BadStatus + def initialize(details = 'unknown cause', metadata = {}) + super(Core::StatusCodes::DEADLINE_EXCEEDED, details, metadata) + end + end + + # GRPC status code corresponding to status NOT_FOUND + class NotFound < BadStatus + def initialize(details = 'unknown cause', metadata = {}) + super(Core::StatusCodes::NOT_FOUND, details, metadata) + end + end + + # GRPC status code corresponding to status ALREADY_EXISTS + class AlreadyExists < BadStatus + def initialize(details = 'unknown cause', metadata = {}) + super(Core::StatusCodes::ALREADY_EXISTS, details, metadata) + end + end + + # GRPC status code corresponding to status PERMISSION_DENIED + class PermissionDenied < BadStatus + def initialize(details = 'unknown cause', metadata = {}) + super(Core::StatusCodes::PERMISSION_DENIED, details, metadata) + end + end + + # GRPC status code corresponding to status UNAUTHENTICATED + class Unauthenticated < BadStatus + def initialize(details = 'unknown cause', metadata = {}) + super(Core::StatusCodes::UNAUTHENTICATED, details, metadata) + end + end + + # GRPC status code corresponding to status RESOURCE_EXHAUSTED + class ResourceExhausted < BadStatus + def initialize(details = 'unknown cause', metadata = {}) + super(Core::StatusCodes::RESOURCE_EXHAUSTED, details, metadata) + end + end + + # GRPC status code corresponding to status FAILED_PRECONDITION + class FailedPrecondition < BadStatus + def initialize(details = 'unknown cause', metadata = {}) + super(Core::StatusCodes::FAILED_PRECONDITION, details, metadata) + end + end + + # GRPC status code corresponding to status ABORTED + class Aborted < BadStatus + def initialize(details = 'unknown cause', metadata = {}) + super(Core::StatusCodes::ABORTED, details, metadata) + end + end + + # GRPC status code corresponding to status OUT_OF_RANGE + class OutOfRange < BadStatus + def initialize(details = 'unknown cause', metadata = {}) + super(Core::StatusCodes::OUT_OF_RANGE, details, metadata) + end + end + + # GRPC status code corresponding to status UNIMPLEMENTED + class Unimplemented < BadStatus + def initialize(details = 'unknown cause', metadata = {}) + super(Core::StatusCodes::UNIMPLEMENTED, details, metadata) + end + end + + # GRPC status code corresponding to status INTERNAL + class Internal < BadStatus + def initialize(details = 'unknown cause', metadata = {}) + super(Core::StatusCodes::INTERNAL, details, metadata) + end + end + + # GRPC status code corresponding to status UNAVAILABLE + class Unavailable < BadStatus + def initialize(details = 'unknown cause', metadata = {}) + super(Core::StatusCodes::UNAVAILABLE, details, metadata) + end + end + + # GRPC status code corresponding to status DATA_LOSS + class DataLoss < BadStatus + def initialize(details = 'unknown cause', metadata = {}) + super(Core::StatusCodes::DATA_LOSS, details, metadata) + end end end diff --git a/src/ruby/spec/error_sanity_spec.rb b/src/ruby/spec/error_sanity_spec.rb new file mode 100644 index 00000000000..97712104fe1 --- /dev/null +++ b/src/ruby/spec/error_sanity_spec.rb @@ -0,0 +1,58 @@ +# Copyright 2015, Google Inc. +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above +# copyright notice, this list of conditions and the following disclaimer +# in the documentation and/or other materials provided with the +# distribution. +# * Neither the name of Google Inc. nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +require 'grpc' + +StatusCodes = GRPC::Core::StatusCodes + +describe StatusCodes do + # convert upper snake-case to camel case. + # e.g., DEADLINE_EXCEEDED -> DeadlineExceeded + def upper_snake_to_camel(name) + name.to_s.split('_').map(&:downcase).map(&:capitalize).join('') + end + + 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 + 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 + expect(error_class < GRPC::BadStatus) + + error_object = error_class.new + # check that the code matches the int value of the error's constant + expect(error_object.code).to eq(StatusCodes.const_get(status_name)) + + # check default parameters + expect(error_object.details).to eq('unknown cause') + expect(error_object.metadata).to eq({}) + end + end +end From acacd0d6467109e452e7375f662240c26fca004f Mon Sep 17 00:00:00 2001 From: Alexander Polcyn Date: Tue, 29 Nov 2016 23:42:27 -0800 Subject: [PATCH 2/3] add factory method to bad status to create correct subclass --- src/ruby/lib/grpc/errors.rb | 43 ++++++++++++++++++++++++++++++ src/ruby/spec/error_sanity_spec.rb | 8 +++++- 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/src/ruby/lib/grpc/errors.rb b/src/ruby/lib/grpc/errors.rb index 022a14b0a51..41c15de1d57 100644 --- a/src/ruby/lib/grpc/errors.rb +++ b/src/ruby/lib/grpc/errors.rb @@ -45,6 +45,8 @@ module GRPC class BadStatus < StandardError attr_reader :code, :details, :metadata + include GRPC::Core::StatusCodes + # @param code [Numeric] the status code # @param details [String] the details of the exception # @param metadata [Hash] the error's metadata @@ -62,6 +64,47 @@ module GRPC def to_status Struct::Status.new(code, details, @metadata) 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) + else + fail 'unknown code' + end + end end # GRPC status code corresponding to status OK diff --git a/src/ruby/spec/error_sanity_spec.rb b/src/ruby/spec/error_sanity_spec.rb index 97712104fe1..ca2d80e6850 100644 --- a/src/ruby/spec/error_sanity_spec.rb +++ b/src/ruby/spec/error_sanity_spec.rb @@ -48,11 +48,17 @@ describe StatusCodes do error_object = error_class.new # check that the code matches the int value of the error's constant - expect(error_object.code).to eq(StatusCodes.const_get(status_name)) + status_code = StatusCodes.const_get(status_name) + expect(error_object.code).to eq(status_code) # check default parameters expect(error_object.details).to eq('unknown cause') expect(error_object.metadata).to eq({}) + + # check that the BadStatus factory for creates the correct + # exception too + from_factory = GRPC::BadStatus.new_status_exception(status_code) + expect(from_factory.is_a?(error_class)).to be(true) end end end From 174aa915bae4b379932e4887e26f2953db52b954 Mon Sep 17 00:00:00 2001 From: Alexander Polcyn Date: Wed, 30 Nov 2016 08:35:52 -0800 Subject: [PATCH 3/3] 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 41c15de1d57..f6998e17c49 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 5787f534635..01797d13e1b 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 7cb9f1cc99d..0dbadcc19ef 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 4bce1744c48..6b2d852ebfd 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 b9af160e7a3..9ee5cdf9fd3 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 ca2d80e6850..77e94a88160 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 9c4e9cbd078..08a171e2991 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 31157cf161e..1689d2df681 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 1b2fa968271..2cc58f845b7 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