From 0d4068e5e890aad959419ad932a162ab170868c5 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Wed, 15 Oct 2008 16:24:12 -0700 Subject: [PATCH] Use exceptions for all errors sent to us from the backend --- lib/mogilefs/backend.rb | 60 +++++++++++++++++++++++++++++++++++++++++------- lib/mogilefs/client.rb | 4 ---- lib/mogilefs/mogilefs.rb | 17 +++----------- test/setup.rb | 9 ++++++++ test/test_backend.rb | 23 +++++++++++++++---- test/test_mogilefs.rb | 18 +++++++++------ 6 files changed, 93 insertions(+), 38 deletions(-) diff --git a/lib/mogilefs/backend.rb b/lib/mogilefs/backend.rb index 3b381eb..fb7d01b 100644 --- a/lib/mogilefs/backend.rb +++ b/lib/mogilefs/backend.rb @@ -18,31 +18,29 @@ class MogileFS::Backend end end + BACKEND_ERRORS = {} + # this converts an error code from a mogilefsd tracker to an exception: # # Examples of some exceptions that get created: # class AfterMismatchError < MogileFS::Error; end # class DomainNotFoundError < MogileFS::Error; end # class InvalidCharsError < MogileFS::Error; end - def error(err_snake) + def self.add_error(err_snake) err_camel = err_snake.gsub(/(?:^|_)([a-z])/) { $1.upcase } << 'Error' - unless self.class.const_defined?(err_camel) - self.class.class_eval("class #{err_camel} < MogileFS::Error; end") + unless self.const_defined?(err_camel) + self.class_eval("class #{err_camel} < MogileFS::Error; end") end - self.class.const_get(err_camel) + BACKEND_ERRORS[err_snake] = self.const_get(err_camel) end ## # The last error - #-- - # TODO Use Exceptions attr_reader :lasterr ## # The string attached to the last error - #-- - # TODO Use Exceptions attr_reader :lasterrstr @@ -106,6 +104,43 @@ class MogileFS::Backend add_command :delete_host add_command :set_state + # Errors copied from MogileFS/Worker/Query.pm + add_error 'dup' + add_error 'after_mismatch' + add_error 'bad_params' + add_error 'class_exists' + add_error 'class_has_files' + add_error 'class_not_found' + add_error 'db' + add_error 'domain_has_files' + add_error 'domain_exists' + add_error 'domain_not_empty' + add_error 'domain_not_found' + add_error 'failure' + add_error 'host_exists' + add_error 'host_mismatch' + add_error 'host_not_empty' + add_error 'host_not_found' + add_error 'invalid_chars' + add_error 'invalid_checker_level' + add_error 'invalid_mindevcount' + add_error 'key_exists' + add_error 'no_class' + add_error 'no_devices' + add_error 'no_domain' + add_error 'no_host' + add_error 'no_ip' + add_error 'no_key' + add_error 'no_port' + add_error 'none_match' + add_error 'plugin_aborted' + add_error 'state_too_high' + add_error 'unknown_command' + add_error 'unknown_host' + add_error 'unknown_key' + add_error 'unknown_state' + add_error 'unreg_domain' + private unless defined? $TESTING ## @@ -146,6 +181,14 @@ class MogileFS::Backend return "#{cmd} #{url_encode args}\r\n" end + # this converts an error code from a mogilefsd tracker to an exception + # Most of these exceptions should already be defined, but since the + # MogileFS server code is liable to change and we may not always be + # able to keep up with the changes + def error(err_snake) + BACKEND_ERRORS[err_snake] || self.class.add_error(err_snake) + end + ## # Turns the +line+ response from the server into a Hash of options, an # error, or raises, as appropriate. @@ -154,6 +197,7 @@ class MogileFS::Backend if line =~ /^ERR\s+(\w+)\s*(.*)/ then @lasterr = $1 @lasterrstr = $2 ? url_unescape($2) : nil + raise error(@lasterr) return nil end diff --git a/lib/mogilefs/client.rb b/lib/mogilefs/client.rb index 75d35d2..56b483a 100644 --- a/lib/mogilefs/client.rb +++ b/lib/mogilefs/client.rb @@ -38,8 +38,6 @@ class MogileFS::Client ## # The last error reported by the backend. - #-- - # TODO use Exceptions def err @backend.lasterr @@ -47,8 +45,6 @@ class MogileFS::Client ## # The last error message reported by the backend. - #-- - # TODO use Exceptions def errstr @backend.lasterrstr diff --git a/lib/mogilefs/mogilefs.rb b/lib/mogilefs/mogilefs.rb index cca08e2..848061e 100644 --- a/lib/mogilefs/mogilefs.rb +++ b/lib/mogilefs/mogilefs.rb @@ -109,8 +109,6 @@ class MogileFS::MogileFS < MogileFS::Client noverify = noverify ? 1 : 0 res = @backend.get_paths(:domain => @domain, :key => key, :noverify => noverify, :zone => zone) - - return nil if res.nil? and @backend.lasterr == 'unknown_key' paths = (1..res['paths'].to_i).map { |i| res["path#{i}"] } return paths if paths.empty? return paths if paths.first =~ /^http:\/\// @@ -128,8 +126,6 @@ class MogileFS::MogileFS < MogileFS::Client res = @backend.create_open(:domain => @domain, :class => klass, :key => key, :multi_dest => 1) - raise "#{@backend.lasterr}: #{@backend.lasterrstr}" if res.nil? || res == {} # HACK - dests = nil if res.include? 'dev_count' then # HACK HUH? @@ -200,11 +196,7 @@ class MogileFS::MogileFS < MogileFS::Client def delete(key) raise 'readonly mogilefs' if readonly? - res = @backend.delete :domain => @domain, :key => key - - if res.nil? and @backend.lasterr != 'unknown_key' then - raise "unable to delete #{key}: #{@backend.lasterr}" - end + @backend.delete :domain => @domain, :key => key end ## @@ -220,11 +212,8 @@ class MogileFS::MogileFS < MogileFS::Client def rename(from, to) raise 'readonly mogilefs' if readonly? - res = @backend.rename :domain => @domain, :from_key => from, :to_key => to - - if res.nil? and @backend.lasterr != 'unknown_key' then - raise "unable to rename #{from} to #{to}: #{@backend.lasterr}" - end + @backend.rename :domain => @domain, :from_key => from, :to_key => to + nil end ## diff --git a/test/setup.rb b/test/setup.rb index 94b7f15..5063d8f 100644 --- a/test/setup.rb +++ b/test/setup.rb @@ -21,6 +21,14 @@ class FakeBackend @lasterrstr = nil end + def error(err_snake) + err_camel = err_snake.gsub(/(?:^|_)([a-z])/) { $1.upcase } << 'Error' + unless MogileFS::Backend.const_defined?(err_camel) + MogileFS::Backend.class_eval("class #{err_camel} < MogileFS::Error; end") + end + MogileFS::Backend.const_get(err_camel) + end + def method_missing(meth, *args) meth = meth.to_s if meth =~ /(.*)=$/ then @@ -31,6 +39,7 @@ class FakeBackend when Array then @lasterr = response.first @lasterrstr = response.last + raise error(@lasterr), @lasterrstr return nil end return response diff --git a/test/test_backend.rb b/test/test_backend.rb index f9eed08..e30398e 100644 --- a/test/test_backend.rb +++ b/test/test_backend.rb @@ -66,10 +66,15 @@ class TestBackend < Test::Unit::TestCase end def test_automatic_exception - assert ! MogileFS::Backend.const_defined?('NoDevicesError') - assert @backend.error('no_devices') - assert_equal MogileFS::Error, @backend.error('no_devices').superclass - assert MogileFS::Backend.const_defined?('NoDevicesError') + assert ! MogileFS::Backend.const_defined?('PebkacError') + assert @backend.error('pebkac') + assert_equal MogileFS::Error, @backend.error('PebkacError').superclass + assert MogileFS::Backend.const_defined?('PebkacError') + + assert ! MogileFS::Backend.const_defined?('PebKacError') + assert @backend.error('peb_kac') + assert_equal MogileFS::Error, @backend.error('PebKacError').superclass + assert MogileFS::Backend.const_defined?('PebKacError') end def test_do_request_truncated @@ -93,7 +98,15 @@ class TestBackend < Test::Unit::TestCase def test_parse_response assert_equal({'foo' => 'bar', 'baz' => 'hoge'}, @backend.parse_response('OK 1 foo=bar&baz=hoge')) - assert_equal nil, @backend.parse_response('ERR you totally suck') + + err = nil + begin + @backend.parse_response('ERR you totally suck') + rescue MogileFS::Error => err + assert_equal 'MogileFS::Backend::YouError', err.class.to_s + end + assert_equal 'MogileFS::Backend::YouError', err.class.to_s + assert_equal 'you', @backend.lasterr assert_equal 'totally suck', @backend.lasterrstr diff --git a/test/test_mogilefs.rb b/test/test_mogilefs.rb index e6970f7..96d7532 100644 --- a/test/test_mogilefs.rb +++ b/test/test_mogilefs.rb @@ -83,7 +83,9 @@ class TestMogileFS__MogileFS < TestMogileFS def test_get_paths_unknown_key @backend.get_paths = ['unknown_key', ''] - assert_equal nil, @client.get_paths('key') + assert_raises MogileFS::Backend::UnknownKeyError do + assert_equal nil, @client.get_paths('key') + end end def test_delete_existing @@ -95,8 +97,8 @@ class TestMogileFS__MogileFS < TestMogileFS def test_delete_nonexisting @backend.delete = 'unknown_key', '' - assert_nothing_raised do - assert_equal nil, @client.delete('no_such_key') + assert_raises MogileFS::Backend::UnknownKeyError do + @client.delete('no_such_key') end end @@ -279,17 +281,19 @@ Content-Length: 0\r def test_rename_nonexisting @backend.rename = 'unknown_key', '' - assert_nil @client.rename('from_key', 'to_key') + assert_raises MogileFS::Backend::UnknownKeyError do + @client.rename('from_key', 'to_key') + end end def test_rename_no_key - @backend.rename = 'no_key', '' + @backend.rename = 'no_key', 'no_key' - e = assert_raises RuntimeError do + e = assert_raises MogileFS::Backend::NoKeyError do @client.rename 'new_key', 'test' end - assert_equal 'unable to rename new_key to test: no_key', e.message + assert_equal 'no_key', e.message end def test_rename_readonly -- 2.11.4.GIT