opendev/gerrit
Code Issues Proposed changes

RelatedChangesSorter#sort: Improve error message

Browse Source 
The current error message is too verbose and also doesn't contain the
relevant information (which patch sets / SHA1s were found) [1].

We now use requireNonNull instead of checkArgument, this means if the
condition is hit we now throw a NullPointerException instead of an
IllegalArgumentException.

[1]
java.lang.IllegalArgumentException: PatchSet{id=104005,1, commitId=AnyObjectId[4ec1003448f6a4f6402a74ee66500b1569681a74], uploader=1004393, createdOn=2019-11-26 11:16:28.0, groups=[ef52a3e0d897e5a810445afcf84346fa88a9e742], pushCertificate=Optional.empty, description=Optional.empty} not found in [ChangeData{Change{103757 (Ibd4a013b37badfc14711557e094ae520e3cbb416), dest=test-infra,refs/heads/master, status=n}}]
        at com.google.common.base.Preconditions.checkArgument(Preconditions.java:441)
        at com.google.gerrit.server.restapi.change.RelatedChangesSorter.sort(RelatedChangesSorter.java:79)
        at com.google.gerrit.server.restapi.change.GetRelated.getRelated(GetRelated.java:103)
        at com.google.gerrit.server.restapi.change.GetRelated.apply(GetRelated.java:76)
        at com.google.gerrit.server.restapi.change.GetRelated.apply(GetRelated.java:52)
        at com.google.gerrit.httpd.restapi.RestApiServlet.lambda$invokeRestReadViewWithRetry$3(RestApiServlet.java:723)
        at com.github.rholder.retry.AttemptTimeLimiters$NoAttemptTimeLimit.call(AttemptTimeLimiters.java:78)
        at com.github.rholder.retry.Retryer.call(Retryer.java:160)
        at com.google.gerrit.server.update.RetryHelper.executeWithTimeoutCount(RetryHelper.java:420)
        at com.google.gerrit.server.update.RetryHelper.executeWithAttemptAndTimeoutCount(RetryHelper.java:368)
        at com.google.gerrit.server.update.RetryHelper.execute(RetryHelper.java:271)
        at com.google.gerrit.httpd.restapi.RestApiServlet.invokeRestEndpointWithRetry(RestApiServlet.java:820)
        at com.google.gerrit.httpd.restapi.RestApiServlet.invokeRestReadViewWithRetry(RestApiServlet.java:718)
        at com.google.gerrit.httpd.restapi.RestApiServlet.service(RestApiServlet.java:501)
        at javax.servlet.http.HttpServlet.service(HttpServlet.java:717)
        ...

Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: I1e8e1fb02b8e698c722a3fee406feaaf4e195a3c
This commit is contained in:
Edwin Kempin
2019-11-27 09:25:55 +01:00
parent 28c0ce2449
commit 192d05a071
1 changed files with 10 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
java/com/google/gerrit/server/restapi/change/RelatedChangesSorter.java
View File

@@ -16,6 +16,7 @@ package com.google.gerrit.server.restapi.change;
import static com.google.common.base.Preconditions.checkArgument;
import static java.util.Objects.requireNonNull;
import static java.util.stream.Collectors.toMap;
import com.google.auto.value.AutoValue;
import com.google.common.annotations.VisibleForTesting;
@@ -76,7 +77,15 @@ class RelatedChangesSorter {
// Map of all patch sets, keyed by commit SHA-1.
Map<ObjectId, PatchSetData> byId = collectById(in);
PatchSetData start = byId.get(startPs.commitId());
checkArgument(start != null, "%s not found in %s", startPs, in);
requireNonNull(
start,
() ->
String.format(
"commit %s of patch set %s not found in %s",
startPs.commitId().name(),
startPs.id(),
byId.entrySet().stream()
.collect(toMap(e -> e.getKey().name(), e -> e.getValue().patchSet().id()))));
// Map of patch set -> immediate parent.
ListMultimap<PatchSetData, PatchSetData> parents =