trivial: Remove broad exceptions where possible

The are two somewhat significant changes:

* The behavior of 'Bundle.add_patch' is changed. Previously this would
  raise an exception if the provided patch already existed in the
  bundle. Since this code was only used in one location, change this to
  return the BundlePatch if valid else None and change the calling code
  to check return value instead of catching the exception.
* Use a context manager to open the config file in pwclient. This loses
  a little granularity in error messaging, but this is a worthy
  compromise.

Signed-off-by: Stephen Finucane <stephenfinucane@hotmail.com>
Reviewed-by: Daniel Axtens <dja@axtens.net>

diff --git a/patchwork/bin/pwclient b/patchwork/bin/pwclient
index 791c5119a0757ee88772929ce0313a1b09c46139..b63db5362c21344e9082d2e0a36a49ac02e6cae7 100755 (executable)
--- a/patchwork/bin/pwclient
+++ b/patchwork/bin/pwclient
@@ -294,17 +294,10 @@ def action_get(rpc, patch_id):
         fname = "%s.%d" % (base_fname, i)
         i += 1
 
-    try:
-        f = open(fname, "w")
-    except:
-        sys.stderr.write("Unable to open %s for writing\n" % fname)
-        sys.exit(1)
-
-    try:
+    with open(fname, 'w') as f:
         f.write(unicode(s).encode("utf-8"))
-        f.close()
         print('Saved patch to %s' % fname)
-    except:
+    except IOError:
         sys.stderr.write("Failed to write to %s\n" % fname)
         sys.exit(1)
 

diff --git a/patchwork/models.py b/patchwork/models.py
index 808c37fd8a457ac5d7f9cf8673f516379440650f..66fb6ab60e8060173996962b5be281ba03f8958d 100644 (file)
--- a/patchwork/models.py
+++ b/patchwork/models.py
@@ -590,12 +590,14 @@ class Bundle(models.Model):
         # see if the patch is already in this bundle
         if BundlePatch.objects.filter(bundle=self,
                                       patch=patch).count():
-            raise Exception('patch is already in bundle')
+            return
 
         bp = BundlePatch.objects.create(bundle=self, patch=patch,
                                         order=max_order + 1)
         bp.save()
 
+        return bp
+
     def public_url(self):
         if not self.public:
             return None

diff --git a/patchwork/notifications.py b/patchwork/notifications.py
index 54204019e47a5a1ecc644a31bba753288b3a148a..16152ef9d19530d11aa1226354336a75ecf789b7 100644 (file)
--- a/patchwork/notifications.py
+++ b/patchwork/notifications.py
@@ -19,6 +19,7 @@
 
 import datetime
 import itertools
+import smtplib
 
 from django.conf import settings
 from django.contrib.auth.models import User
@@ -87,7 +88,7 @@ def send_notifications():
 
         try:
             message.send()
-        except Exception as ex:
+        except smtplib.SMTPException as ex:
             errors.append((recipient, ex))
             continue
 

diff --git a/patchwork/paginator.py b/patchwork/paginator.py
index e31c76c71296d9ed59ced90ee1bc0bcfd1ebeb75..3da2cd0f25adcf3b2d8c41db4a0f52966bf95353 100644 (file)
--- a/patchwork/paginator.py
+++ b/patchwork/paginator.py
@@ -55,9 +55,9 @@ class Paginator(paginator.Paginator):
         super(Paginator, self).__init__(objects, items_per_page)
 
         try:
-            page_no = int(request.GET.get('page'))
+            page_no = int(request.GET.get('page', 1))
             self.current_page = self.page(int(page_no))
-        except Exception:
+        except ValueError:
             page_no = 1
             self.current_page = self.page(page_no)
 

diff --git a/patchwork/templatetags/listurl.py b/patchwork/templatetags/listurl.py
index 3f28f719e590c3f82b299dd1ff77205bd54c55f8..cee6753b9c97d8c78d5d4ad7622edae2c2b189dd 100644 (file)
--- a/patchwork/templatetags/listurl.py
+++ b/patchwork/templatetags/listurl.py
@@ -63,9 +63,9 @@ class ListURLNode(template.defaulttags.URLNode):
 
         params = []
         try:
-            qs_var = template.Variable('list_view.params')
-            params = dict(qs_var.resolve(context))
-        except Exception:
+            qs_var = template.Variable('list_view.params').resolve(context)
+            params = dict(qs_var)
+        except (TypeError, template.VariableDoesNotExist):
             pass
 
         for (k, v) in self.params.items():

diff --git a/patchwork/views/__init__.py b/patchwork/views/__init__.py
index a7b70610e700cb96b6acc9455b15781b8f4e597e..c4cfd83c70acfaf0bef44a4c42fa7b139d2ad286 100644 (file)
--- a/patchwork/views/__init__.py
+++ b/patchwork/views/__init__.py
@@ -174,13 +174,13 @@ def set_bundle(request, project, action, data, patches, context):
             try:
                 bp = BundlePatch.objects.get(bundle=bundle, patch=patch)
                 bp.delete()
+            except BundlePatch.DoesNotExist:
+                pass
+            else:
                 messages.success(
                     request,
                     "Patch '%s' removed from bundle %s\n" % (patch.name,
                                                              bundle.name))
-            # TODO(stephenfin): Make this less broad
-            except Exception:
-                pass
 
     bundle.save()
 

diff --git a/patchwork/views/bundle.py b/patchwork/views/bundle.py
index ba569e26cc5632f1bbfc7a1b959bb3286f168e2b..e7174298dc02261ebff125d7578fccc9f31688ea 100644 (file)
--- a/patchwork/views/bundle.py
+++ b/patchwork/views/bundle.py
@@ -48,10 +48,7 @@ def setbundle(request):
             patch_id = request.POST.get('patch_id', None)
             if patch_id:
                 patch = get_object_or_404(Patch, id=patch_id)
-                try:
-                    bundle.append_patch(patch)
-                except Exception:
-                    pass
+                bundle.append_patch(patch)
             bundle.save()
         elif action == 'add':
             bundle = get_object_or_404(Bundle,
@@ -66,11 +63,8 @@ def setbundle(request):
                 patch_ids = get_patch_ids(request.POST)
 
             for patch_id in patch_ids:
-                try:
-                    patch = Patch.objects.get(id=patch_id)
-                    bundle.append_patch(patch)
-                except:
-                    pass
+                patch = Patch.objects.get(id=patch_id)
+                bundle.append_patch(patch)
 
             bundle.save()
         elif action == 'delete':
@@ -78,11 +72,10 @@ def setbundle(request):
                 bundle = Bundle.objects.get(owner=request.user,
                                             id=request.POST['id'])
                 bundle.delete()
-            except Exception:
+            except Bundle.DoesNotExist:
                 pass
 
             bundle = None
-
     else:
         bundle = get_object_or_404(Bundle, owner=request.user,
                                    id=request.POST['bundle_id'])

diff --git a/patchwork/views/mail.py b/patchwork/views/mail.py
index 3695e5b1f5ea7f7ccb379ce4e69497ab93979296..8744d79b59720b02514c1a702b0bc3d41b2e1deb 100644 (file)
--- a/patchwork/views/mail.py
+++ b/patchwork/views/mail.py
@@ -17,7 +17,7 @@
 # along with Patchwork; if not, write to the Free Software
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
-from __future__ import absolute_import
+import smtplib
 
 from django.conf import settings as conf_settings
 from django.core.mail import send_mail
@@ -114,7 +114,7 @@ def _optinout(request, action, description):
         send_mail('Patchwork %s confirmation' % description, mail,
                   conf_settings.DEFAULT_FROM_EMAIL, [email])
         context['email_sent'] = True
-    except Exception:
+    except smtplib.SMTPException:
         context['error'] = ('An error occurred during confirmation . '
                             'Please try again later.')
         context['admins'] = conf_settings.ADMINS

diff --git a/patchwork/views/patch.py b/patchwork/views/patch.py
index 41a2ec8365150418cccf699a3ed49a20cfbb89ed..244abe2e6d7e2ffca0aab7eda24646ddfa1c39cb 100644 (file)
--- a/patchwork/views/patch.py
+++ b/patchwork/views/patch.py
@@ -77,15 +77,15 @@ def patch(request, patch_id):
         elif action == 'addtobundle':
             bundle = get_object_or_404(
                 Bundle, id=request.POST.get('bundle_id'))
-            try:
-                bundle.append_patch(patch)
-                bundle.save()
+            if bundle.append_patch(patch):
                 messages.success(request,
-                                 'Patch added to bundle "%s"' % bundle.name)
-            except Exception as ex:
+                                 'Patch "%s" added to bundle "%s"' % (
+                                     patch.name, bundle.name))
+            else:
                 messages.error(request,
-                               "Couldn't add patch '%s' to bundle %s: %s"
-                               % (patch.name, bundle.name, ex.message))
+                               'Failed to add patch "%s" to bundle "%s": '
+                               'patch is already in bundle' % (
+                                   patch.name, bundle.name))
 
         # all other actions require edit privs
         elif not editable:

diff --git a/patchwork/views/user.py b/patchwork/views/user.py
index dfbfde81dd08ef4525178ca44d13a286f891cdfa..b82636f085facc3fb347d42f14ae9f98ad971bc3 100644 (file)
--- a/patchwork/views/user.py
+++ b/patchwork/views/user.py
@@ -17,7 +17,7 @@
 # along with Patchwork; if not, write to the Free Software
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
-from __future__ import absolute_import
+import smtplib
 
 from django.contrib import auth
 from django.contrib.auth.decorators import login_required
@@ -145,7 +145,7 @@ def link(request):
                                            context, request=request),
                           settings.DEFAULT_FROM_EMAIL,
                           [form.cleaned_data['email']])
-            except Exception:
+            except smtplib.SMTPException:
                 context['confirmation'] = None
                 context['error'] = ('An error occurred during confirmation. '
                                     'Please try again later')

diff --git a/patchwork/views/xmlrpc.py b/patchwork/views/xmlrpc.py
index 77e253380ee63e5c4a8fdb1537cc049f5f4acd62..1919ad920e4f10e5100e038e6687c87c0438cb20 100644 (file)
--- a/patchwork/views/xmlrpc.py
+++ b/patchwork/views/xmlrpc.py
@@ -94,7 +94,7 @@ class PatchworkXMLRPCDispatcher(SimpleXMLRPCDispatcher,
         try:
             decoded = base64.b64decode(header.encode('ascii')).decode('ascii')
             username, password = decoded.split(':', 1)
-        except:
+        except ValueError:
             raise Exception('Invalid authentication credentials')
 
         return authenticate(username=username, password=password)
@@ -124,7 +124,7 @@ class PatchworkXMLRPCDispatcher(SimpleXMLRPCDispatcher,
             response = self.dumps(response, methodresponse=1)
         except six.moves.xmlrpc_client.Fault as fault:
             response = self.dumps(fault)
-        except:
+        except Exception:  # noqa
             # report exception back to server
             response = self.dumps(
                 six.moves.xmlrpc_client.Fault(
@@ -149,7 +149,7 @@ def xmlrpc(request):
     if request.method == 'POST':
         try:
             ret = dispatcher._marshaled_dispatch(request)
-        except Exception:
+        except Exception:  # noqa
             return HttpResponseServerError()
     else:
         ret = dispatcher.generate_html_documentation()