Suppress boring parts of the stack trace if something fails
Change-Id: Ib000d3de3d76cd9d9bb84839fd5d6e44a6ebdc49
diff --git a/src/main/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestRefUpdatedListener.java b/src/main/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestRefUpdatedListener.java
index 0d95224..19ad258 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestRefUpdatedListener.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestRefUpdatedListener.java
@@ -17,7 +17,6 @@
import static com.google.gerrit.reviewdb.client.RefNames.REFS_HEADS;
import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Preconditions;
import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
import com.google.gerrit.extensions.events.LifecycleListener;
@@ -37,12 +36,13 @@
import java.io.InputStream;
import java.net.URI;
import java.net.URISyntaxException;
+import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
+import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
-import org.apache.http.client.utils.URIBuilder;
import org.eclipse.jgit.api.errors.GitAPIException;
import org.eclipse.jgit.api.errors.InvalidRemoteException;
import org.eclipse.jgit.api.errors.RefNotFoundException;
@@ -159,7 +159,8 @@
srcRef = "";
} else {
if (!Repository.isValidRefName(destRef)) {
- throw new ConfigInvalidException(String.format("destination branch '%s' invalid", destRef));
+ throw new ConfigInvalidException(
+ String.format("destination branch '%s' invalid", destRef));
}
srcRef = cfg.getString(SECTION_NAME, name, "srcRef");
@@ -184,7 +185,6 @@
recordSubmoduleLabels = cfg.getBoolean(SECTION_NAME, name, "recordSubmoduleLabels", false);
-
try {
// http://foo/platform/manifest => http://foo/platform/
baseUri = new URI(srcRepoKey.toString()).resolve("");
@@ -268,7 +268,6 @@
wildcardDestinations.add(configEntry.destRepoKey.get());
}
-
sources.add(configEntry.srcRepoKey.get());
destinations.add(configEntry.destRepoKey.get());
@@ -290,7 +289,9 @@
public void stop() {}
@Override
- public void start() { updateConfiguration(); }
+ public void start() {
+ updateConfiguration();
+ }
/** for debugging. */
private String configurationToString() {
@@ -345,6 +346,11 @@
try {
update(c, event.getRefName());
} catch (IOException | GitAPIException e) {
+ // We only want the trace up to here. We could recurse into the exception, but this at least
+ // trims the very common jgit.gitrepo.RepoCommand.RemoteUnavailableException.
+ StackTraceElement here = Thread.currentThread().getStackTrace()[1];
+ e.setStackTrace(trimStack(e.getStackTrace(), here));
+
// We are in an asynchronously called listener, so there is no user action to give
// feedback to. We log the error, but it would be nice if we could surface these logs
// somewhere. Perhaps we could store these as commits in some special branch (but in
@@ -355,6 +361,24 @@
}
}
+ /**
+ * Remove boring stack frames. This retains the innermost frames up to and including the
+ * {@code class#method} passed in {@code ref}.
+ */
+ @VisibleForTesting
+ static StackTraceElement[] trimStack(StackTraceElement[] trace, StackTraceElement ref) {
+ List<StackTraceElement> trimmed = new ArrayList<>();
+ for (StackTraceElement e : trace) {
+ trimmed.add(e);
+ if (e.getClassName().equals(ref.getClassName())
+ && e.getMethodName().equals(ref.getMethodName())) {
+ break;
+ }
+ }
+
+ return trimmed.toArray(new StackTraceElement[trimmed.size()]);
+ }
+
private static class GerritIncludeReader implements ManifestParser.IncludedFileReader {
private final Repository repo;
private final String ref;
@@ -489,5 +513,4 @@
}
return name;
}
-
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestIT.java b/src/test/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestIT.java
index 7a19516..d9dc131 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestIT.java
@@ -27,6 +27,7 @@
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import java.net.URI;
+import java.util.Arrays;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.BlobBasedConfig;
@@ -165,6 +166,33 @@
assertThat(branch.file("project3").getContentType()).isEqualTo("x-git/gitlink; charset=UTF-8");
}
+ private void outer() {
+ inner();
+ }
+
+ private void inner() {
+ throw new IllegalStateException();
+ }
+
+ private void innerTest() {
+ try {
+ outer();
+ fail("should throw");
+ } catch (IllegalStateException e) {
+ StackTraceElement[] trimmed =
+ SuperManifestRefUpdatedListener.trimStack(
+ e.getStackTrace(), Thread.currentThread().getStackTrace()[1]);
+ String str = Arrays.toString(trimmed);
+ assertThat(str).doesNotContain("trimStackTrace");
+ assertThat(str).contains("innerTest");
+ }
+ }
+
+ @Test
+ public void trimStackTrace() throws Exception {
+ innerTest();
+ }
+
@Test
public void wildcardDestBranchWorks() throws Exception {
setupTestRepos("project");
@@ -365,13 +393,15 @@
public void testToRepoKey() {
URI base = URI.create("https://gerrit-review.googlesource.com");
assertThat(
- SuperManifestRefUpdatedListener.urlToRepoKey(base,
- "https://gerrit-review.googlesource.com/repo")).isEqualTo("repo");
+ SuperManifestRefUpdatedListener.urlToRepoKey(
+ base, "https://gerrit-review.googlesource.com/repo"))
+ .isEqualTo("repo");
assertThat(SuperManifestRefUpdatedListener.urlToRepoKey(base, "repo")).isEqualTo("repo");
assertThat(
- SuperManifestRefUpdatedListener.urlToRepoKey(
- URI.create("https://gerrit-review.googlesource.com/"),
- "https://gerrit-review.googlesource.com/repo")).isEqualTo("repo");
+ SuperManifestRefUpdatedListener.urlToRepoKey(
+ URI.create("https://gerrit-review.googlesource.com/"),
+ "https://gerrit-review.googlesource.com/repo"))
+ .isEqualTo("repo");
}
// TODO - should add tests for all the error handling in configuration parsing?
