Fix: Wrong exception mapping in ReceiveCommmits

When IOException is thrown in insertPatchSet() or
insertChange(), e.g. ClosedByInterruptException thrown
from AutoCommitWriter.manualFlush(), it will be
mapped to OrmException with error message
"Error updating database" by ORM_EXCEPTION, this becomes
misleading when the administrator analyzes the error log.

Fixed. Replaced the ORM_EXCEPTION with INSERT_EXCEPTION.

Change-Id: I49a6dfea7e49df72db775989a3a0501f60db76d2
This commit is contained in:
Bruce Zu
2014-04-16 18:34:02 +08:00
committed by David Pursehouse
parent 4b77e60348
commit 977072b8ca
2 changed files with 49 additions and 14 deletions

View File

@@ -0,0 +1,30 @@
//Copyright (C) 2014 The Android Open Source Project
//
//Licensed under the Apache License, Version 2.0 (the "License");
//you may not use this file except in compliance with the License.
//You may obtain a copy of the License at
//
//http://www.apache.org/licenses/LICENSE-2.0
//
//Unless required by applicable law or agreed to in writing, software
//distributed under the License is distributed on an "AS IS" BASIS,
//WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
//See the License for the specific language governing permissions and
//limitations under the License.
package com.google.gerrit.server.git;
/**
* Thrown in inserting change or patchset, e.g. OrmException or IOException.
*/
public class InsertException extends Exception {
private static final long serialVersionUID = 1L;
InsertException(final String msg) {
super(msg, null);
}
InsertException(final String msg, final Throwable why) {
super(msg, why);
}
}

View File

@@ -236,14 +236,17 @@ public class ReceiveCommits {
}
}
private static final Function<Exception, OrmException> ORM_EXCEPTION =
new Function<Exception, OrmException>() {
private static final Function<Exception, InsertException> INSERT_EXCEPTION =
new Function<Exception, InsertException>() {
@Override
public OrmException apply(Exception input) {
public InsertException apply(Exception input) {
if (input instanceof OrmException) {
return (OrmException) input;
return new InsertException("ORM error", input);
}
return new OrmException("Error updating database", input);
if (input instanceof IOException) {
return new InsertException("IO error", input);
}
return new InsertException("Error inserting change/patchset", input);
}
};
@@ -647,7 +650,7 @@ public class ReceiveCommits {
log.error(String.format(
"Cannot add patch set to %d of %s",
e.getKey().get(), project.getName()), err);
} catch (OrmException err) {
} catch (InsertException err) {
reject(replace.inputCommand, "internal server error");
log.error(String.format(
"Cannot add patch set to %d of %s",
@@ -681,7 +684,7 @@ public class ReceiveCommits {
}
try {
List<CheckedFuture<?, OrmException>> futures = Lists.newArrayList();
List<CheckedFuture<?, InsertException>> futures = Lists.newArrayList();
for (ReplaceRequest replace : replaceByChange.values()) {
if (magicBranch != null && replace.inputCommand == magicBranch.cmd) {
futures.add(replace.insertPatchSet());
@@ -692,12 +695,12 @@ public class ReceiveCommits {
futures.add(create.insertChange());
}
for (CheckedFuture<?, OrmException> f : futures) {
for (CheckedFuture<?, InsertException> f : futures) {
f.checkedGet();
}
magicBranch.cmd.setResult(OK);
} catch (OrmException err) {
log.error("Can't insert changes for " + project.getName(), err);
} catch (InsertException err) {
log.error("Can't insert change/patchset for " + project.getName(), err);
reject(magicBranch.cmd, "internal server error");
} catch (IOException err) {
log.error("Can't read commits for " + project.getName(), err);
@@ -1462,7 +1465,7 @@ public class ReceiveCommits {
ins.getPatchSet().getRefName());
}
CheckedFuture<Void, OrmException> insertChange() throws IOException {
CheckedFuture<Void, InsertException> insertChange() throws IOException {
rp.getRevWalk().parseBody(commit);
final Thread caller = Thread.currentThread();
@@ -1486,7 +1489,7 @@ public class ReceiveCommits {
return null;
}
}));
return Futures.makeChecked(future, ORM_EXCEPTION);
return Futures.makeChecked(future, INSERT_EXCEPTION);
}
private void insertChange(ReviewDb db) throws OrmException, IOException {
@@ -1790,7 +1793,7 @@ public class ReceiveCommits {
return true;
}
CheckedFuture<PatchSet.Id, OrmException> insertPatchSet()
CheckedFuture<PatchSet.Id, InsertException> insertPatchSet()
throws IOException {
rp.getRevWalk().parseBody(newCommit);
@@ -1817,7 +1820,7 @@ public class ReceiveCommits {
}
}
}));
return Futures.makeChecked(future, ORM_EXCEPTION);
return Futures.makeChecked(future, INSERT_EXCEPTION);
}
PatchSet.Id insertPatchSet(ReviewDb db) throws OrmException, IOException {
@@ -2183,6 +2186,8 @@ public class ReceiveCommits {
codeReviewCommit, rw, repo, project, new ArrayList<Change>(),
new HashMap<Change.Id, CodeReviewCommit>());
subOp.update();
} catch (InsertException e) {
log.error("Can't insert patchset", e);
} catch (IOException e) {
log.error("Can't scan for changes to close", e);
} catch (OrmException e) {