[yast-devel] Re: [yast-commit] <web-client> master : handle PackageKit errors (bnc #559233)
Stefan Schubert write:
ref: refs/heads/master commit 547aa5b85029bdfd4b37ef9464be2494508f4e0d Author: Stefan Schubert <schubi@suse.de> Date: Fri Dec 18 13:51:40 2009 +0100
handle PackageKit errors (bnc #559233) --- .../app/controllers/patch_updates_controller.rb | 32 ++++++++++++++++--- .../views/patch_updates/_patch_summary.html.erb | 2 +- .../app/views/patch_updates/index.html.erb | 2 +- .../package/yast2-webclient-patch_updates.changes | 5 +++ 4 files changed, 34 insertions(+), 7 deletions(-)
diff --git a/plugins/patch_updates/app/controllers/patch_updates_controller.rb b/plugins/patch_updates/app/controllers/patch_updates_controller.rb index 79d5f73..db14fb3 100644 --- a/plugins/patch_updates/app/controllers/patch_updates_controller.rb +++ b/plugins/patch_updates/app/controllers/patch_updates_controller.rb @@ -13,7 +13,19 @@ class PatchUpdatesController < ApplicationController # GET /patch_updates # GET /patch_updates.xml def index - @patch_updates = load_proxy 'org.opensuse.yast.system.patches', :all + begin + @patch_updates = load_proxy 'org.opensuse.yast.system.patches', :all + rescue ActiveResource::ServerError => e + error_hash = Hash.from_xml e.response.body + logger.warn error_hash.inspect + if error_hash["error"] && error_hash["error"]["type"] == "PACKAGEKIT_ERROR" + flash[:error] = error_hash["error"]["description"] + @patch_updates = [] + @error = true + else + raise e + end + end
You can benefit from ClientException class which handles exceptions from backend. It contains implementation details which you can abstract ( and don't need to change code when implementation change). so your code could look like this one which looks more readable for me: rescue ActiveResource::ServerError => e ce = ClientException.new e if ce.backend_exception_type == "PACKAGEKIT_ERROR" flash[:error] = ce.message @patch_updates = [] @error = true else raise e end end
logger.debug "Available patches: #{@patch_updates.inspect}" end
@@ -23,11 +35,21 @@ class PatchUpdatesController < ApplicationController patch_updates = nil begin patch_updates = load_proxy 'org.opensuse.yast.system.patches', :all - rescue Exception => e + rescue ActiveResource::ClientError => e error = ClientException.new(e) patch_updates = nil - end - + error_string = _("A problem occured when loading patch information.") + rescue ActiveResource::ServerError => e + error_hash = Hash.from_xml e.response.body + logger.warn error_hash.inspect + if error_hash["error"] && error_hash["error"]["type"] == "PACKAGEKIT_ERROR" + error_string = error_hash["error"]["description"] + else + error_string = _("A problem occured when loading patch information.") + end + error = ClientException.new(e)
^^^ I don't understand much why you set error, but you should benefit from it same as I show above, so: error_string = _("A problem occured when loading patch information.") error = ClientException.new(e) error_string = error.message if error.backend_exception_type == "PACKAGEKIT_ERROR"
+ patch_updates = nil + end patches_summary = nil <snip/>
-- Josef Reidinger YaST team maintainer of perl-Bootloader, YaST2-Repair, webyast (language,time,basesystem,ntp) -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org For additional commands, e-mail: yast-devel+help@opensuse.org
Josef Reidinger schrieb:
Stefan Schubert write:
ref: refs/heads/master commit 547aa5b85029bdfd4b37ef9464be2494508f4e0d Author: Stefan Schubert <schubi@suse.de> Date: Fri Dec 18 13:51:40 2009 +0100
handle PackageKit errors (bnc #559233) --- .../app/controllers/patch_updates_controller.rb | 32 ++++++++++++++++--- .../views/patch_updates/_patch_summary.html.erb | 2 +- .../app/views/patch_updates/index.html.erb | 2 +- .../package/yast2-webclient-patch_updates.changes | 5 +++ 4 files changed, 34 insertions(+), 7 deletions(-)
diff --git a/plugins/patch_updates/app/controllers/patch_updates_controller.rb b/plugins/patch_updates/app/controllers/patch_updates_controller.rb index 79d5f73..db14fb3 100644 --- a/plugins/patch_updates/app/controllers/patch_updates_controller.rb +++ b/plugins/patch_updates/app/controllers/patch_updates_controller.rb @@ -13,7 +13,19 @@ class PatchUpdatesController < ApplicationController # GET /patch_updates # GET /patch_updates.xml def index - @patch_updates = load_proxy 'org.opensuse.yast.system.patches', :all + begin + @patch_updates = load_proxy 'org.opensuse.yast.system.patches', :all + rescue ActiveResource::ServerError => e + error_hash = Hash.from_xml e.response.body + logger.warn error_hash.inspect + if error_hash["error"] && error_hash["error"]["type"] == "PACKAGEKIT_ERROR" + flash[:error] = error_hash["error"]["description"] + @patch_updates = [] + @error = true + else + raise e + end + end
You can benefit from ClientException class which handles exceptions from backend. It contains implementation details which you can abstract ( and don't need to change code when implementation change).
Thanks for suggestions.
so your code could look like this one which looks more readable for me: rescue ActiveResource::ServerError => e ce = ClientException.new e
While developing I have struggled over the class naming. I have seen the ClientException class have searched for a ServerException class, but I have not found in the lib directory. I have thought that ClientException is valid for ActiveResource::ClientError only. Thats why I have made it manually. Hm, perhaps someone else "falls" in the same problem :-) Would an alias to the class be possible or another notation ? Greetings Stefan
if ce.backend_exception_type == "PACKAGEKIT_ERROR" flash[:error] = ce.message @patch_updates = [] @error = true else raise e end end
logger.debug "Available patches: #{@patch_updates.inspect}" end
@@ -23,11 +35,21 @@ class PatchUpdatesController < ApplicationController patch_updates = nil begin patch_updates = load_proxy 'org.opensuse.yast.system.patches', :all - rescue Exception => e + rescue ActiveResource::ClientError => e error = ClientException.new(e) patch_updates = nil - end - + error_string = _("A problem occured when loading patch information.") + rescue ActiveResource::ServerError => e + error_hash = Hash.from_xml e.response.body + logger.warn error_hash.inspect + if error_hash["error"] && error_hash["error"]["type"] == "PACKAGEKIT_ERROR" + error_string = error_hash["error"]["description"] + else + error_string = _("A problem occured when loading patch information.") + end + error = ClientException.new(e)
^^^ I don't understand much why you set error, but you should benefit from it same as I show above, so: error_string = _("A problem occured when loading patch information.") error = ClientException.new(e) error_string = error.message if error.backend_exception_type == "PACKAGEKIT_ERROR"
+ patch_updates = nil + end patches_summary = nil <snip/>
-- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org For additional commands, e-mail: yast-devel+help@opensuse.org
participants (2)
-
Josef Reidinger
-
Stefan Schubert