Do not drop the exception in the error handler
When an HTML rendering fails, the handler drops the exception without
saying anything. Add the rendering exception as a suppressed exception
and rethrow the original exception. This helps debugging an issue that
the handler is trying to change the status code for the committed
response.
Change-Id: I9e2e73222cd71df6bd6ebef7b12d385abb6531af
Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>
diff --git a/java/com/google/gitiles/DefaultErrorHandlingFilter.java b/java/com/google/gitiles/DefaultErrorHandlingFilter.java
index f558c0d..459347c 100644
--- a/java/com/google/gitiles/DefaultErrorHandlingFilter.java
+++ b/java/com/google/gitiles/DefaultErrorHandlingFilter.java
@@ -45,32 +45,68 @@
@Override
public void doFilter(HttpServletRequest req, HttpServletResponse res, FilterChain chain)
throws IOException, ServletException {
- int status = -1;
- String message = null;
try {
chain.doFilter(req, res);
} catch (GitilesRequestFailureException e) {
- res.setHeader(GITILES_ERROR, e.getReason().toString());
- status = e.getReason().getHttpStatusCode();
- message = e.getPublicErrorMessage();
+ try {
+ res.setHeader(GITILES_ERROR, e.getReason().toString());
+ renderHtml(req, res, e.getReason().getHttpStatusCode(), e.getPublicErrorMessage());
+ } catch (IOException e2) {
+ e.addSuppressed(e2);
+ throw e;
+ }
} catch (RepositoryNotFoundException e) {
- status = FailureReason.REPOSITORY_NOT_FOUND.getHttpStatusCode();
- message = FailureReason.REPOSITORY_NOT_FOUND.getMessage();
+ try {
+ res.setHeader(GITILES_ERROR, FailureReason.REPOSITORY_NOT_FOUND.toString());
+ renderHtml(
+ req,
+ res,
+ FailureReason.REPOSITORY_NOT_FOUND.getHttpStatusCode(),
+ FailureReason.REPOSITORY_NOT_FOUND.getMessage());
+ } catch (IOException e2) {
+ e.addSuppressed(e2);
+ throw e;
+ }
} catch (AmbiguousObjectException e) {
- status = FailureReason.AMBIGUOUS_OBJECT.getHttpStatusCode();
- message = FailureReason.AMBIGUOUS_OBJECT.getMessage();
+ try {
+ res.setHeader(GITILES_ERROR, FailureReason.AMBIGUOUS_OBJECT.toString());
+ renderHtml(
+ req,
+ res,
+ FailureReason.AMBIGUOUS_OBJECT.getHttpStatusCode(),
+ FailureReason.AMBIGUOUS_OBJECT.getMessage());
+ } catch (IOException e2) {
+ e.addSuppressed(e2);
+ throw e;
+ }
} catch (ServiceMayNotContinueException e) {
- status = e.getStatusCode();
- message = e.getMessage();
- } catch (IOException | ServletException err) {
- log.warn("Internal server error", err);
- status = FailureReason.INTERNAL_SERVER_ERROR.getHttpStatusCode();
- message = FailureReason.INTERNAL_SERVER_ERROR.getMessage();
+ try {
+ renderHtml(req, res, e.getStatusCode(), e.getMessage());
+ } catch (IOException e2) {
+ e.addSuppressed(e2);
+ throw e;
+ }
+ } catch (IOException | ServletException e) {
+ try {
+ log.warn("Internal server error", e);
+ res.setHeader(GITILES_ERROR, FailureReason.INTERNAL_SERVER_ERROR.toString());
+ renderHtml(
+ req,
+ res,
+ FailureReason.INTERNAL_SERVER_ERROR.getHttpStatusCode(),
+ FailureReason.INTERNAL_SERVER_ERROR.getMessage());
+ } catch (IOException e2) {
+ e.addSuppressed(e2);
+ throw e;
+ }
}
- if (status != -1) {
- res.setStatus(status);
- renderHtml(req, res, "gitiles.error", ImmutableMap.of("title", message));
- }
+ }
+
+ private void renderHtml(
+ HttpServletRequest req, HttpServletResponse res, int status, String message)
+ throws IOException {
+ res.setStatus(status);
+ renderHtml(req, res, "gitiles.error", ImmutableMap.of("title", message));
}
protected void renderHtml(
