gerrit approve: Cleanup error reporting for missing objects

If a database object is not present, or the change is closed, try
to use a consistent formatting for this error message.  We catch the
two major "not found" types up in the top level of the work thread,
making it easier for the action code to throw directly out of the
control objects.

Change-Id: I6e731ad648738245b6b15840f40711ce677eb957
Signed-off-by: Shawn O. Pearce <sop@google.com>

2 files changed, 16 insertions, 7 deletions

diff --git a/src/main/java/com/google/gerrit/server/ssh/BaseCommand.java b/src/main/java/com/google/gerrit/server/ssh/BaseCommand.java
index 9a3af3772c..fe6ecedf2d 100644
--- a/src/main/java/com/google/gerrit/server/ssh/BaseCommand.java
+++ b/src/main/java/com/google/gerrit/server/ssh/BaseCommand.java
@@ -17,6 +17,8 @@ package com.google.gerrit.server.ssh;
 import com.google.gerrit.client.reviewdb.Account;
 import com.google.gerrit.pgm.CmdLineParser;
 import com.google.gerrit.server.RequestCleanup;
+import com.google.gerrit.server.project.NoSuchChangeException;
+import com.google.gerrit.server.project.NoSuchProjectException;
 import com.google.gerrit.server.ssh.SshScopes.Context;
 import com.google.inject.Inject;
 
@@ -243,7 +245,13 @@ public abstract class BaseCommand implements Command {
             active.add(cmd);
           }
           SshScopes.current.set(context);
-          thunk.run();
+          try {
+            thunk.run();
+          } catch (NoSuchProjectException e) {
+            throw new UnloggedFailure(1, e.getMessage() + " no such project");
+          } catch (NoSuchChangeException e) {
+            throw new UnloggedFailure(1, e.getMessage() + " no such change");
+          }
           out.flush();
           err.flush();
         } catch (Throwable e) {
diff --git a/src/main/java/com/google/gerrit/server/ssh/commands/ApproveCommand.java b/src/main/java/com/google/gerrit/server/ssh/commands/ApproveCommand.java
index c87cfe70c8..9019da716f 100644
--- a/src/main/java/com/google/gerrit/server/ssh/commands/ApproveCommand.java
+++ b/src/main/java/com/google/gerrit/server/ssh/commands/ApproveCommand.java
@@ -55,8 +55,6 @@ public class ApproveCommand extends BaseCommand {
     return parser;
   }
 
-  private static final int CMD_ERR = 3;
-
   @Argument(index = 0, required = true, metaVar = "CHANGE,PATCHSET", usage = "Patch set to approve")
   private PatchSet.Id patchSetId;
 
@@ -99,7 +97,7 @@ public class ApproveCommand extends BaseCommand {
         final PatchSet ps = db.patchSets().get(patchSetId);
 
         if (ps == null) {
-          throw new UnloggedFailure(CMD_ERR, "Invalid patchset id");
+          throw error("" + patchSetId + " no such patch set");
         }
 
         final Change.Id cid = ps.getId().getParentKey();
@@ -107,7 +105,7 @@ public class ApproveCommand extends BaseCommand {
         final Change c = control.getChange();
 
         if (c.getStatus().isClosed()) {
-          throw new UnloggedFailure(CMD_ERR, "Change is closed.");
+          throw error("change " + cid + " is closed");
         }
 
         StringBuffer sb = new StringBuffer();
@@ -193,8 +191,7 @@ public class ApproveCommand extends BaseCommand {
     psa.setValue(score);
     fs.normalize(approvalTypes.getApprovalType(psa.getCategoryId()), psa);
     if (score != psa.getValue()) {
-      throw new UnloggedFailure(CMD_ERR, co.name() + "=" + co.value()
-          + " not permitted");
+      throw error(co.name() + "=" + co.value() + " not permitted");
     }
 
     psa.setGranted();
@@ -226,4 +223,8 @@ public class ApproveCommand extends BaseCommand {
           category.getName()));
     }
   }
+
+  private static UnloggedFailure error(final String msg) {
+    return new UnloggedFailure(1, msg);
+  }
 }