ExceptionHook: Add method that allows to control the status code of the response
If during the execution of a REST request an exception occurs we return
a 500 Internal Server Error response to the user, unless the exception
is sepcifically caught and handled in RestApiServlet.
Now, with the new method implementors of the ExceptionHook extension
point can control the REST response for arbitrary exceptions. The new
getStatusCode method allows them to decide on the HTTP status code and
the already existing getUserMessage method allows them to define the
message in the response body.
This is useful if environment-specific exceptions should be handled.
E.g. at Google we have a specific exception that is thrown if a client
disconnects. At the moment this exception results in an internal server
error, which affects our SLO metrics. A disconnected client is not an
error and we would rather return 200 OK in this case.
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: I04b1e2cfc002289ca8b405ce11449d3c445165c5
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index 0535397..9c6637b 100644
--- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -1670,18 +1670,37 @@
if (!res.isCommitted()) {
res.reset();
traceContext.getTraceId().ifPresent(traceId -> res.addHeader(X_GERRIT_TRACE, traceId));
- StringBuilder msg = new StringBuilder("Internal server error");
ImmutableList<String> userMessages =
globals.exceptionHooks.stream()
.map(h -> h.getUserMessage(err))
.filter(Optional::isPresent)
.map(Optional::get)
.collect(toImmutableList());
+
+ Optional<Integer> statusCode =
+ globals.exceptionHooks.stream()
+ .map(h -> h.getStatusCode(err))
+ .filter(Optional::isPresent)
+ .map(Optional::get)
+ .findFirst();
+ if (statusCode.isPresent() && statusCode.get() < 400) {
+ StringBuilder msg = new StringBuilder();
+ if (userMessages.size() == 1) {
+ msg.append(userMessages.get(0));
+ } else {
+ userMessages.forEach(m -> msg.append("\n* ").append(m));
+ }
+
+ res.setStatus(statusCode.get());
+ logger.atFinest().withCause(err).log("REST call finished: %d", statusCode);
+ return replyText(req, res, true, msg.toString());
+ }
+
+ StringBuilder msg = new StringBuilder("Internal server error");
if (!userMessages.isEmpty()) {
- msg.append("\n");
userMessages.forEach(m -> msg.append("\n* ").append(m));
}
- return replyError(req, res, SC_INTERNAL_SERVER_ERROR, msg.toString(), err);
+ return replyError(req, res, statusCode.orElse(SC_INTERNAL_SERVER_ERROR), msg.toString(), err);
}
return 0;
}
diff --git a/java/com/google/gerrit/server/ExceptionHook.java b/java/com/google/gerrit/server/ExceptionHook.java
index db44b4b..a2bade0 100644
--- a/java/com/google/gerrit/server/ExceptionHook.java
+++ b/java/com/google/gerrit/server/ExceptionHook.java
@@ -55,7 +55,9 @@
}
/**
- * Returns an error message that should be returned to the user.
+ * Returns a message that should be returned to the user.
+ *
+ * <p>This message is included into the HTTP response that is sent to the user.
*
* @param throwable throwable that was thrown while executing an operation
* @return error message that should be returned to the user, {@link Optional#empty()} if no
@@ -64,4 +66,25 @@
default Optional<String> getUserMessage(Throwable throwable) {
return Optional.empty();
}
+
+ /**
+ * Returns the HTTP status code that should be returned to the user.
+ *
+ * <p>If no value is returned ({@link Optional#empty()}) the HTTP status code defaults to {@code
+ * 500 Internal Server Error}.
+ *
+ * <p>{@link #getUserMessage(Throwable)} allows to define which message should be included into
+ * the body of the HTTP response.
+ *
+ * <p>Implementors may use this method to change the status code for certain exceptions (e.g.
+ * using this method it would be possible to return {@code 409 Conflict} for {@link
+ * com.google.gerrit.git.LockFailureException}s instead of {@code 500 Internal Server Error}).
+ *
+ * @param throwable throwable that was thrown while executing an operation
+ * @return HTTP status code that should be returned to the user, {@link Optional#empty()} if the
+ * exception should result in {@code 500 Internal Server Error}
+ */
+ default Optional<Integer> getStatusCode(Throwable throwable) {
+ return Optional.empty();
+ }
}
