ChecksSubmitRule: Catch all RuntimeExceptions
This used to catch the checked OrmException, and was converted
mechanically to StorageException without considering the fact that
StorageException is now unchecked. In fact, the intent of this catch
block is to catch any type of unexpected exception.
These are the only instances of catching StorageException in the plugin:
$ find . -name \*.java | xargs grep -P '(catch \(|\|) *StorageException'
./java/com/google/gerrit/plugins/checks/rules/ChecksSubmitRule.java: } catch (StorageException e) {
./java/com/google/gerrit/plugins/checks/rules/ChecksSubmitRule.java: } catch (StorageException e) {
Change-Id: I257382adaee7d03ba6d8a3ea8e0550ed0f6f1ee1
diff --git a/java/com/google/gerrit/plugins/checks/rules/ChecksSubmitRule.java b/java/com/google/gerrit/plugins/checks/rules/ChecksSubmitRule.java
index b1bfd63..3d10ef7 100644
--- a/java/com/google/gerrit/plugins/checks/rules/ChecksSubmitRule.java
+++ b/java/com/google/gerrit/plugins/checks/rules/ChecksSubmitRule.java
@@ -20,7 +20,6 @@
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.common.data.SubmitRecord.Status;
import com.google.gerrit.common.data.SubmitRequirement;
-import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.annotations.Exports;
import com.google.gerrit.extensions.config.FactoryModule;
import com.google.gerrit.plugins.checks.CombinedCheckStateCache;
@@ -69,7 +68,7 @@
PatchSet.Id currentPathSetId;
try {
currentPathSetId = changeData.currentPatchSet().getId();
- } catch (StorageException e) {
+ } catch (RuntimeException e) {
String errorMessage =
String.format("failed to load the current patch set of change %s", changeId);
logger.atSevere().withCause(e).log(errorMessage);
@@ -80,7 +79,7 @@
try {
// Reload value in cache to fix up inconsistencies between cache and actual state.
combinedCheckState = combinedCheckStateCache.reload(project, currentPathSetId);
- } catch (StorageException e) {
+ } catch (RuntimeException e) {
String errorMessage =
String.format("failed to evaluate check states for change %s", changeId);
logger.atSevere().withCause(e).log(errorMessage);
diff --git a/javatests/com/google/gerrit/plugins/checks/rules/ChecksSubmitRuleTest.java b/javatests/com/google/gerrit/plugins/checks/rules/ChecksSubmitRuleTest.java
index 4a3bab7..7101a0f 100644
--- a/javatests/com/google/gerrit/plugins/checks/rules/ChecksSubmitRuleTest.java
+++ b/javatests/com/google/gerrit/plugins/checks/rules/ChecksSubmitRuleTest.java
@@ -21,7 +21,6 @@
import com.google.common.collect.Iterables;
import com.google.gerrit.common.data.SubmitRecord;
-import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.plugins.checks.CombinedCheckStateCache;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
@@ -43,7 +42,7 @@
ChangeData cd = EasyMock.createStrictMock(ChangeData.class);
expect(cd.project()).andReturn(Project.nameKey("My-Project"));
expect(cd.getId()).andReturn(Change.id(1));
- expect(cd.currentPatchSet()).andThrow(new StorageException("Fail for test"));
+ expect(cd.currentPatchSet()).andThrow(new IllegalStateException("Fail for test"));
replay(cd);
Collection<SubmitRecord> submitRecords =
@@ -54,7 +53,8 @@
@Test
public void getCombinedCheckStateFails() throws Exception {
CombinedCheckStateCache cache = EasyMock.createStrictMock(CombinedCheckStateCache.class);
- expect(cache.reload(anyObject(), anyObject())).andThrow(new StorageException("Fail for test"));
+ expect(cache.reload(anyObject(), anyObject()))
+ .andThrow(new IllegalStateException("Fail for test"));
replay(cache);
ChecksSubmitRule checksSubmitRule = new ChecksSubmitRule(cache);
