SuperManifestRefUpdatedListener: Do not return 500 via Response.withStatusCode REST endpoints should throw an exception to indicate an internal server error. RestApiServlet takes care to catch the exception and to return a '500 Internal Server Error' response for it. Returning 500 via Response.withStatusCode also works but messes up the error metric. If no exception is thrown, then we can't log an exception as the cause in the error metric and we fall back to using "_none" as the cause (hard-coded in RestApiServlet). "_none" is not helpful, hence fix the metric by throwing an exception, rather than returning a 500 response code with Response.withStatusCode. Change-Id: Ic479a221985bf5d936fc88def24837d343c73dd5 Signed-off-by: Edwin Kempin <ekempin@google.com>
diff --git a/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestRefUpdatedListener.java b/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestRefUpdatedListener.java index b232214..efa5577 100644 --- a/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestRefUpdatedListener.java +++ b/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestRefUpdatedListener.java
@@ -292,7 +292,11 @@ @Override public Response<?> apply(BranchResource resource, BranchInput input) - throws AuthException, PermissionBackendException, PreconditionFailedException { + throws AuthException, + PermissionBackendException, + PreconditionFailedException, + GitAPIException, + IOException { permissionBackend.currentUser().check(GlobalPermission.ADMINISTRATE_SERVER); String manifestProject = resource.getBranchKey().project().get(); String manifestBranch = resource.getBranchKey().branch(); @@ -341,7 +345,7 @@ throw new PreconditionFailedException(e.getMessage()); } catch (GitAPIException | IOException e) { errorWithCause(e, "Internal error processing %s:%s", manifestProject, manifestBranch); - return Response.withStatusCode(500, "Internal error: " + e.getMessage()); + throw e; } } return Response.ok();