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();
