commit 6902c6a34feabb08ab7b4a82c89b601b34fba28d
author David Pursehouse <david.pursehouse@sonymobile.com> Wed Mar 09 14:45:50 2016 +0900
committer David Pursehouse <david.pursehouse@sonymobile.com> Wed Mar 09 15:27:04 2016 +0900
tree 6491748ee389d261dc5c1ee3f3550f8ce9ec1157
parent 8e3d87d60c6e660e36cb830f0e44d6fe81e24db3

ChangeEventListener: Improve exception handling

If the configuration includes a username or email address that is
misspelled or has the incorrect case, the error log is filled up
with logs like:

Cannot resolve reviewer: User.Name
com.google.gerrit.extensions.restapi.UnprocessableEntityException: Group Not Found: User.Name
        at com.google.gerrit.server.group.GroupsCollection.parse(GroupsCollection.java:113)
        ...

This is misleading, because the problem is not actually caused by a
missing group.

Refactor the checks for account and group to handle exceptions
separately, and log appropriate warnings.

Change-Id: Ie29810e5edb9a5ebe9c603f288c17099aea875e9

src/main/java/com/googlesource/gerrit/plugins/reviewers/ChangeEventListener.java

diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewers/ChangeEventListener.java b/src/main/java/com/googlesource/gerrit/plugins/reviewers/ChangeEventListener.java
index b4aafaa..e019152 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewers/ChangeEventListener.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewers/ChangeEventListener.java
@@ -21,6 +21,8 @@
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Sets;
 import com.google.gerrit.common.EventListener;
+import com.google.gerrit.common.errors.NoSuchGroupException;
+import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.PatchSet;
@@ -36,6 +38,7 @@
 import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.git.WorkQueue;
 import com.google.gerrit.server.group.GroupsCollection;
+import com.google.gerrit.server.project.NoSuchProjectException;
 import com.google.gerrit.server.query.Predicate;
 import com.google.gerrit.server.query.QueryParseException;
 import com.google.gerrit.server.query.change.ChangeData;
@@ -243,15 +246,28 @@
           reviewers.add(account);
           continue;
         }
-        if (groupMembers == null) {
-          groupMembers =
-              groupMembersFactory.create(identifiedUserFactory.create(Iterables
-                  .getOnlyElement(byEmailCache.get(uploaderEMail))));
-        }
-        reviewers.addAll(groupMembers.listAccounts(groupsCollection.get()
-            .parse(r).getGroupUUID(), p));
-      } catch (Exception e) {
-        log.warn("Cannot resolve reviewer: " + r, e);
+      } catch (OrmException e) {
+        // If the account doesn't exist, find() will return null.  We only
+        // get here if something went wrong accessing the database
+        log.error("Failed to resolve account " + r, e);
+        continue;
+      }
+      if (groupMembers == null) {
+        groupMembers =
+            groupMembersFactory.create(identifiedUserFactory.create(Iterables
+                .getOnlyElement(byEmailCache.get(uploaderEMail))));
+      }
+      try {
+        reviewers.addAll(groupMembers.listAccounts(
+            groupsCollection.get().parse(r).getGroupUUID(), p));
+      } catch (UnprocessableEntityException | NoSuchGroupException e) {
+        log.warn(String.format(
+            "Reviewer %s is neither an account nor a group", r));
+      } catch (NoSuchProjectException e) {
+        log.warn(String.format(
+            "Failed to list accounts for group %s and project %s", r, p));
+      } catch (IOException | OrmException e) {
+        log.warn(String.format("Failed to list accounts for group %s", r), e);
       }
     }
     return reviewers;