From ef35c21e2a587dc4136ff25acef774eabd7777fa Mon Sep 17 00:00:00 2001
From: Stefan Oderbolz <stefan.oderbolz@liip.ch>
Date: Thu, 15 Jan 2015 00:07:26 +0100
Subject: [PATCH] Improve exception handling with custom exception

1. Try whenever possible to catch specific exceptions
2. Raise custom exception where appropriate
3. Fix the exception handling in _get_group and _get_organization
---
 ckanext/harvest/harvesters/ckanharvester.py | 30 ++++++++++++++-------
 1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/ckanext/harvest/harvesters/ckanharvester.py b/ckanext/harvest/harvesters/ckanharvester.py
index 3fb3935b8..e6fe50349 100644
--- a/ckanext/harvest/harvesters/ckanharvester.py
+++ b/ckanext/harvest/harvesters/ckanharvester.py
@@ -40,8 +40,14 @@ def _get_content(self, url):
         api_key = self.config.get('api_key',None)
         if api_key:
             http_request.add_header('Authorization',api_key)
-        http_response = urllib2.urlopen(http_request)
 
+        try:
+            http_response = urllib2.urlopen(http_request)
+        except urllib2.HTTPError, e:
+            raise ContentFetchError(
+                'Could not fetch url: %s, HTTP error code: %s' % 
+                (url, e.code)
+            )
         return http_response.read()
 
     def _get_group(self, base_url, group_name):
@@ -49,8 +55,9 @@ def _get_group(self, base_url, group_name):
         try:
             content = self._get_content(url)
             return json.loads(content)
-        except Exception, e:
-            raise e
+        except (ContentFetchError, ValueError):
+            log.debug('Could not fetch/decode remote group');
+            raise RemoteResourceError('Could not fetch/decode remote group')
 
     def _get_organization(self, base_url, org_name):
         url = base_url + self._get_action_api_offset() + '/organization_show?id=' + org_name
@@ -58,8 +65,9 @@ def _get_organization(self, base_url, org_name):
             content = self._get_content(url)
             content_dict = json.loads(content)
             return content_dict['result']
-        except Exception, e:
-            raise e
+        except (ContentFetchError, ValueError, KeyError):
+            log.debug('Could not fetch/decode remote group');
+            raise RemoteResourceError('Could not fetch/decode remote organization')
 
     def _set_config(self,config_str):
         if config_str:
@@ -294,7 +302,7 @@ def import_stage(self,harvest_object):
                         if remote_groups == 'create':
                             try:
                                 group = self._get_group(harvest_object.source.url, group_name)
-                            except:
+                            except RemoteResourceError:
                                 log.error('Could not get remote group %s' % group_name)
                                 continue
 
@@ -339,10 +347,9 @@ def import_stage(self,harvest_object):
                             try:
                                 try:
                                     org = self._get_organization(harvest_object.source.url, remote_org)
-                                except:
+                                except RemoteResourceError:
                                     # fallback if remote CKAN exposes organizations as groups
                                     # this especially targets older versions of CKAN
-                                    log.debug('_get_organization() failed, now trying _get_group()')
                                     org = self._get_group(harvest_object.source.url, remote_org)
 
                                 for key in ['packages', 'created', 'users', 'groups', 'tags', 'extras', 'display_name', 'type']:
@@ -350,7 +357,7 @@ def import_stage(self,harvest_object):
                                 get_action('organization_create')(context, org)
                                 log.info('Organization %s has been newly created' % remote_org)
                                 validated_org = org['id']
-                            except:
+                            except (RemoteResourceError, ValidationError):
                                 log.error('Could not get remote org %s' % remote_org)
 
                 package_dict['owner_org'] = validated_org or local_org
@@ -425,3 +432,8 @@ def import_stage(self,harvest_object):
         except Exception, e:
             self._save_object_error('%r'%e,harvest_object,'Import')
 
+class ContentFetchError(Exception):
+    pass
+
+class RemoteResourceError(Exception):
+    pass