Throw GitilesRequestFailureException
There are many cases that `return null` should be replaced with `throw
new GitilesRequestFailureException`. However, its correctness is hard to
check at scale. This change focuses on replacing `sendError` calls with
GitilesRequestFailureException. Later, we can change `return null` and
dropping exception cases to use GitilesRequestFailureException.
Change-Id: Ic502b82030f7cdd5db6cea7aaedd07c9b6ff6725
diff --git a/java/com/google/gitiles/ArchiveServlet.java b/java/com/google/gitiles/ArchiveServlet.java
index 3f03455..ed9fedd 100644
--- a/java/com/google/gitiles/ArchiveServlet.java
+++ b/java/com/google/gitiles/ArchiveServlet.java
@@ -14,10 +14,10 @@
package com.google.gitiles;
-import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND;
import static javax.servlet.http.HttpServletResponse.SC_OK;
import com.google.common.base.Strings;
+import com.google.gitiles.GitilesRequestFailureException.FailureReason;
import java.io.IOException;
import java.util.Optional;
import javax.servlet.ServletException;
@@ -50,15 +50,13 @@
ObjectId treeId = getTree(view, repo, rev);
if (treeId.equals(ObjectId.zeroId())) {
- res.sendError(SC_NOT_FOUND);
- return;
+ throw new GitilesRequestFailureException(FailureReason.INCORRECT_OBJECT_TYPE);
}
Optional<ArchiveFormat> format =
ArchiveFormat.byExtension(view.getExtension(), getAccess(req).getConfig());
if (!format.isPresent()) {
- res.setStatus(SC_NOT_FOUND);
- return;
+ throw new GitilesRequestFailureException(FailureReason.UNSUPPORTED_RESPONSE_FORMAT);
}
String filename = getFilename(view, rev, view.getExtension());
setDownloadHeaders(req, res, filename, format.get().getMimeType());
diff --git a/java/com/google/gitiles/BaseServlet.java b/java/com/google/gitiles/BaseServlet.java
index 4b8a139..02a3f1a 100644
--- a/java/com/google/gitiles/BaseServlet.java
+++ b/java/com/google/gitiles/BaseServlet.java
@@ -30,6 +30,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps;
import com.google.common.net.HttpHeaders;
+import com.google.gitiles.GitilesRequestFailureException.FailureReason;
import com.google.gson.FieldNamingPolicy;
import com.google.gson.GsonBuilder;
import java.io.BufferedWriter;
@@ -119,8 +120,7 @@
break;
case DEFAULT:
default:
- res.sendError(SC_BAD_REQUEST);
- break;
+ throw new GitilesRequestFailureException(FailureReason.UNSUPPORTED_RESPONSE_FORMAT);
}
}
@@ -147,7 +147,7 @@
* @param res in-progress response.
*/
protected void doGetHtml(HttpServletRequest req, HttpServletResponse res) throws IOException {
- res.sendError(SC_BAD_REQUEST);
+ throw new GitilesRequestFailureException(FailureReason.UNSUPPORTED_RESPONSE_FORMAT);
}
/**
@@ -157,7 +157,7 @@
* @param res in-progress response.
*/
protected void doGetText(HttpServletRequest req, HttpServletResponse res) throws IOException {
- res.sendError(SC_BAD_REQUEST);
+ throw new GitilesRequestFailureException(FailureReason.UNSUPPORTED_RESPONSE_FORMAT);
}
/**
@@ -167,7 +167,7 @@
* @param res in-progress response.
*/
protected void doGetJson(HttpServletRequest req, HttpServletResponse res) throws IOException {
- res.sendError(SC_BAD_REQUEST);
+ throw new GitilesRequestFailureException(FailureReason.UNSUPPORTED_RESPONSE_FORMAT);
}
protected static Map<String, Object> getData(HttpServletRequest req) {
diff --git a/java/com/google/gitiles/DescribeServlet.java b/java/com/google/gitiles/DescribeServlet.java
index 785d452..ebd2ea4 100644
--- a/java/com/google/gitiles/DescribeServlet.java
+++ b/java/com/google/gitiles/DescribeServlet.java
@@ -14,11 +14,9 @@
package com.google.gitiles;
-import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
-import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND;
-
import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableMap;
+import com.google.gitiles.GitilesRequestFailureException.FailureReason;
import com.google.gson.reflect.TypeToken;
import java.io.IOException;
import java.io.Writer;
@@ -84,21 +82,14 @@
try {
return repo.resolve(rev);
} catch (RevisionSyntaxException e) {
- renderTextError(
- req,
- res,
- SC_BAD_REQUEST,
- "Invalid revision syntax: " + RefServlet.sanitizeRefForText(rev));
- return null;
+ throw new GitilesRequestFailureException(FailureReason.INCORECT_PARAMETER)
+ .withPublicErrorMessage(
+ "Invalid revision syntax: %s", RefServlet.sanitizeRefForText(rev));
} catch (AmbiguousObjectException e) {
- renderTextError(
- req,
- res,
- SC_BAD_REQUEST,
- String.format(
+ throw new GitilesRequestFailureException(FailureReason.AMBIGUOUS_OBJECT)
+ .withPublicErrorMessage(
"Ambiguous short SHA-1 %s (%s)",
- e.getAbbreviatedObjectId(), Joiner.on(", ").join(e.getCandidates())));
- return null;
+ e.getAbbreviatedObjectId(), Joiner.on(", ").join(e.getCandidates()));
}
}
@@ -106,8 +97,7 @@
Repository repo, GitilesView view, HttpServletRequest req, HttpServletResponse res)
throws IOException {
if (!getBooleanParam(view, CONTAINS_PARAM)) {
- res.setStatus(SC_BAD_REQUEST);
- return null;
+ throw new GitilesRequestFailureException(FailureReason.INCORECT_PARAMETER);
}
ObjectId id = resolve(repo, view, req, res);
if (id == null) {
@@ -124,8 +114,7 @@
throw new IOException(e);
}
if (name == null) {
- res.setStatus(SC_NOT_FOUND);
- return null;
+ throw new GitilesRequestFailureException(FailureReason.OBJECT_NOT_FOUND);
}
return name;
}
@@ -137,8 +126,8 @@
boolean all = getBooleanParam(view, ALL_PARAM);
boolean tags = getBooleanParam(view, TAGS_PARAM);
if (all && tags) {
- renderTextError(req, res, SC_BAD_REQUEST, "Cannot specify both \"all\" and \"tags\"");
- return null;
+ throw new GitilesRequestFailureException(FailureReason.UNSUPPORTED_REVISION_NAMES)
+ .withPublicErrorMessage("Cannot specify both \"all\" and \"tags\"");
}
if (all) {
cmd.addPrefix(Constants.R_REFS);
diff --git a/java/com/google/gitiles/DiffServlet.java b/java/com/google/gitiles/DiffServlet.java
index d0e4409..5b1801d 100644
--- a/java/com/google/gitiles/DiffServlet.java
+++ b/java/com/google/gitiles/DiffServlet.java
@@ -15,11 +15,11 @@
package com.google.gitiles;
import static com.google.common.base.Preconditions.checkNotNull;
-import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND;
import com.google.common.io.BaseEncoding;
import com.google.gitiles.CommitData.Field;
import com.google.gitiles.DateFormatter.Format;
+import com.google.gitiles.GitilesRequestFailureException.FailureReason;
import java.io.IOException;
import java.io.OutputStream;
import java.io.Writer;
@@ -67,8 +67,7 @@
AbstractTreeIterator newTree;
try {
if (tw == null && !view.getPathPart().isEmpty()) {
- res.setStatus(SC_NOT_FOUND);
- return;
+ throw new GitilesRequestFailureException(FailureReason.OBJECT_NOT_FOUND);
}
isFile = tw != null && isFile(tw);
@@ -77,9 +76,10 @@
showCommit = isParentOf(walk, view.getOldRevision(), view.getRevision());
oldTree = getTreeIterator(walk, view.getOldRevision().getId());
newTree = getTreeIterator(walk, view.getRevision().getId());
- } catch (MissingObjectException | IncorrectObjectTypeException e) {
- res.setStatus(SC_NOT_FOUND);
- return;
+ } catch (MissingObjectException e) {
+ throw new GitilesRequestFailureException(FailureReason.OBJECT_NOT_FOUND, e);
+ } catch (IncorrectObjectTypeException e) {
+ throw new GitilesRequestFailureException(FailureReason.INCORRECT_OBJECT_TYPE, e);
}
Map<String, Object> data = getData(req);
@@ -124,9 +124,10 @@
try {
oldTree = getTreeIterator(walk, view.getOldRevision().getId());
newTree = getTreeIterator(walk, view.getRevision().getId());
- } catch (MissingObjectException | IncorrectObjectTypeException e) {
- res.setStatus(SC_NOT_FOUND);
- return;
+ } catch (MissingObjectException e) {
+ throw new GitilesRequestFailureException(FailureReason.OBJECT_NOT_FOUND, e);
+ } catch (IncorrectObjectTypeException e) {
+ throw new GitilesRequestFailureException(FailureReason.INCORRECT_OBJECT_TYPE, e);
}
try (Writer writer = startRenderText(req, res);
diff --git a/java/com/google/gitiles/GitwebRedirectFilter.java b/java/com/google/gitiles/GitwebRedirectFilter.java
index f1e01bc..324af72 100644
--- a/java/com/google/gitiles/GitwebRedirectFilter.java
+++ b/java/com/google/gitiles/GitwebRedirectFilter.java
@@ -18,7 +18,6 @@
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.net.HttpHeaders.LOCATION;
import static java.nio.charset.StandardCharsets.UTF_8;
-import static javax.servlet.http.HttpServletResponse.SC_GONE;
import static javax.servlet.http.HttpServletResponse.SC_MOVED_PERMANENTLY;
import com.google.common.base.Splitter;
@@ -27,6 +26,7 @@
import com.google.common.collect.LinkedListMultimap;
import com.google.common.collect.ListMultimap;
import com.google.gitiles.GitilesView.InvalidViewException;
+import com.google.gitiles.GitilesRequestFailureException.FailureReason;
import java.io.IOException;
import java.io.UnsupportedEncodingException;
import java.net.URLDecoder;
@@ -117,8 +117,7 @@
} else {
// Gitiles does not provide an RSS feed (a=rss,atom,opml)
// Any other URL is out of date and not valid anymore.
- res.sendError(SC_GONE);
- return;
+ throw new GitilesRequestFailureException(FailureReason.UNSUPPORTED_GITWEB_URL);
}
if (!Strings.isNullOrEmpty(project)) {
@@ -132,8 +131,7 @@
.setServletPath(gitwebView.getServletPath())
.toUrl();
} catch (InvalidViewException e) {
- res.setStatus(SC_GONE);
- return;
+ throw new GitilesRequestFailureException(FailureReason.UNSUPPORTED_GITWEB_URL);
}
res.setStatus(SC_MOVED_PERMANENTLY);
res.setHeader(LOCATION, url);
diff --git a/java/com/google/gitiles/HostIndexServlet.java b/java/com/google/gitiles/HostIndexServlet.java
index d23aa3c..ce2ec6b 100644
--- a/java/com/google/gitiles/HostIndexServlet.java
+++ b/java/com/google/gitiles/HostIndexServlet.java
@@ -15,16 +15,11 @@
package com.google.gitiles;
import static com.google.common.base.Preconditions.checkNotNull;
-import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
-import static javax.servlet.http.HttpServletResponse.SC_FORBIDDEN;
-import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND;
-import static javax.servlet.http.HttpServletResponse.SC_SERVICE_UNAVAILABLE;
-import static javax.servlet.http.HttpServletResponse.SC_UNAUTHORIZED;
-import static org.eclipse.jgit.http.server.GitSmartHttpTools.sendError;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Sets;
+import com.google.gitiles.GitilesRequestFailureException.FailureReason;
import com.google.gson.reflect.TypeToken;
import com.google.template.soy.data.SoyListData;
import com.google.template.soy.data.SoyMapData;
@@ -40,8 +35,6 @@
import javax.annotation.Nullable;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
-import org.eclipse.jgit.errors.RepositoryNotFoundException;
-import org.eclipse.jgit.transport.ServiceMayNotContinueException;
import org.eclipse.jgit.transport.resolver.ServiceNotAuthorizedException;
import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException;
import org.slf4j.Logger;
@@ -66,27 +59,13 @@
Map<String, RepositoryDescription> descs;
try {
descs = getAccess(req).listRepositories(prefix, branches);
- } catch (RepositoryNotFoundException e) {
- res.sendError(SC_NOT_FOUND);
- return null;
} catch (ServiceNotEnabledException e) {
- res.sendError(SC_FORBIDDEN);
- return null;
+ throw new GitilesRequestFailureException(FailureReason.SERVICE_NOT_ENABLED, e);
} catch (ServiceNotAuthorizedException e) {
- res.sendError(SC_UNAUTHORIZED);
- return null;
- } catch (ServiceMayNotContinueException e) {
- sendError(req, res, e.getStatusCode(), e.getMessage());
- return null;
- } catch (IOException err) {
- String name = urls.getHostName(req);
- log.warn("Cannot scan repositories" + (name != null ? " for " + name : ""), err);
- res.sendError(SC_SERVICE_UNAVAILABLE);
- return null;
+ throw new GitilesRequestFailureException(FailureReason.NOT_AUTHORIZED, e);
}
if (prefix != null && descs.isEmpty()) {
- res.sendError(SC_NOT_FOUND);
- return null;
+ throw new GitilesRequestFailureException(FailureReason.REPOSITORY_NOT_FOUND);
}
return descs;
}
@@ -103,15 +82,14 @@
protected void doHead(HttpServletRequest req, HttpServletResponse res) throws IOException {
Optional<FormatType> format = getFormat(req);
if (!format.isPresent()) {
- res.sendError(SC_BAD_REQUEST);
- return;
+ throw new GitilesRequestFailureException(FailureReason.UNSUPPORTED_RESPONSE_FORMAT);
}
GitilesView view = ViewFilter.getView(req);
String prefix = view.getRepositoryPrefix();
if (prefix != null) {
Map<String, RepositoryDescription> descs =
- list(req, res, prefix, Collections.<String>emptySet());
+ list(req, res, prefix, Collections.emptySet());
if (descs == null) {
return;
}
@@ -125,8 +103,7 @@
break;
case DEFAULT:
default:
- res.sendError(SC_BAD_REQUEST);
- break;
+ throw new GitilesRequestFailureException(FailureReason.UNSUPPORTED_RESPONSE_FORMAT);
}
}
diff --git a/java/com/google/gitiles/LogServlet.java b/java/com/google/gitiles/LogServlet.java
index f1bdd95..3523846 100644
--- a/java/com/google/gitiles/LogServlet.java
+++ b/java/com/google/gitiles/LogServlet.java
@@ -15,8 +15,6 @@
package com.google.gitiles;
import static com.google.common.base.Preconditions.checkNotNull;
-import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
-import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND;
import com.google.common.base.Strings;
import com.google.common.collect.Iterables;
@@ -27,6 +25,7 @@
import com.google.common.primitives.Longs;
import com.google.gitiles.CommitData.Field;
import com.google.gitiles.DateFormatter.Format;
+import com.google.gitiles.GitilesRequestFailureException.FailureReason;
import com.google.gson.reflect.TypeToken;
import java.io.IOException;
import java.io.OutputStream;
@@ -42,7 +41,6 @@
import org.eclipse.jgit.diff.DiffConfig;
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
import org.eclipse.jgit.errors.MissingObjectException;
-import org.eclipse.jgit.errors.RevWalkException;
import org.eclipse.jgit.http.server.ServletUtils;
import org.eclipse.jgit.lib.AbbreviatedObjectId;
import org.eclipse.jgit.lib.Constants;
@@ -97,8 +95,7 @@
GitilesAccess access = getAccess(req);
paginator = newPaginator(repo, view, access);
if (paginator == null) {
- res.setStatus(SC_NOT_FOUND);
- return;
+ throw new GitilesRequestFailureException(FailureReason.OBJECT_NOT_FOUND);
}
DateFormatter df = new DateFormatter(access, Format.DEFAULT);
@@ -133,10 +130,6 @@
.renderStreaming(paginator, null, renderer, w, df, LogSoyData.FooterBehavior.NEXT);
w.flush();
}
- } catch (RevWalkException e) {
- log.warn("Error in rev walk", e);
- res.setStatus(SC_INTERNAL_SERVER_ERROR);
- return;
} finally {
if (paginator != null) {
paginator.getWalk().close();
@@ -160,8 +153,7 @@
GitilesAccess access = getAccess(req);
paginator = newPaginator(repo, view, access);
if (paginator == null) {
- res.setStatus(SC_NOT_FOUND);
- return;
+ throw new GitilesRequestFailureException(FailureReason.OBJECT_NOT_FOUND);
}
DateFormatter df = new DateFormatter(access, Format.DEFAULT);
CommitJsonData.Log result = new CommitJsonData.Log();
diff --git a/java/com/google/gitiles/PathServlet.java b/java/com/google/gitiles/PathServlet.java
index 0ecb875..dac38d3 100644
--- a/java/com/google/gitiles/PathServlet.java
+++ b/java/com/google/gitiles/PathServlet.java
@@ -16,8 +16,6 @@
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.gitiles.TreeSoyData.resolveTargetUrl;
-import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
-import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND;
import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
import static org.eclipse.jgit.lib.Constants.OBJ_COMMIT;
import static org.eclipse.jgit.lib.Constants.OBJ_TREE;
@@ -29,6 +27,7 @@
import com.google.common.collect.Maps;
import com.google.common.io.BaseEncoding;
import com.google.common.primitives.Bytes;
+import com.google.gitiles.GitilesRequestFailureException.FailureReason;
import java.io.IOException;
import java.io.OutputStream;
import java.io.Writer;
@@ -131,8 +130,7 @@
try (RevWalk rw = new RevWalk(repo);
WalkResult wr = WalkResult.forPath(rw, view, false)) {
if (wr == null) {
- res.setStatus(SC_NOT_FOUND);
- return;
+ throw new GitilesRequestFailureException(FailureReason.OBJECT_NOT_FOUND);
}
switch (wr.type) {
case TREE:
@@ -149,12 +147,10 @@
showGitlink(req, res, wr);
break;
default:
- log.error("Bad file type: {}", wr.type);
- res.setStatus(SC_NOT_FOUND);
- break;
+ throw new GitilesRequestFailureException(FailureReason.UNSUPPORTED_OBJECT_TYPE);
}
} catch (LargeObjectException e) {
- res.setStatus(SC_INTERNAL_SERVER_ERROR);
+ throw new GitilesRequestFailureException(FailureReason.OBJECT_TOO_LARGE, e);
}
}
@@ -166,8 +162,7 @@
try (RevWalk rw = new RevWalk(repo);
WalkResult wr = WalkResult.forPath(rw, view, false)) {
if (wr == null) {
- res.setStatus(SC_NOT_FOUND);
- return;
+ throw new GitilesRequestFailureException(FailureReason.OBJECT_NOT_FOUND);
}
// Write base64 as plain text without modifying any other headers, under
@@ -185,11 +180,10 @@
break;
case GITLINK:
default:
- renderTextError(req, res, SC_NOT_FOUND, "Not a file");
- break;
+ throw new GitilesRequestFailureException(FailureReason.UNSUPPORTED_OBJECT_TYPE);
}
} catch (LargeObjectException e) {
- res.setStatus(SC_INTERNAL_SERVER_ERROR);
+ throw new GitilesRequestFailureException(FailureReason.OBJECT_TOO_LARGE, e);
}
}
@@ -252,8 +246,7 @@
try (RevWalk rw = new RevWalk(repo);
WalkResult wr = WalkResult.forPath(rw, view, recursive)) {
if (wr == null) {
- res.setStatus(SC_NOT_FOUND);
- return;
+ throw new GitilesRequestFailureException(FailureReason.OBJECT_NOT_FOUND);
}
switch (wr.type) {
case REGULAR_FILE:
@@ -275,11 +268,10 @@
case GITLINK:
case SYMLINK:
default:
- res.setStatus(SC_NOT_FOUND);
- break;
+ throw new GitilesRequestFailureException(FailureReason.UNSUPPORTED_OBJECT_TYPE);
}
} catch (LargeObjectException e) {
- res.setStatus(SC_INTERNAL_SERVER_ERROR);
+ throw new GitilesRequestFailureException(FailureReason.OBJECT_TOO_LARGE, e);
}
}
diff --git a/java/com/google/gitiles/RefServlet.java b/java/com/google/gitiles/RefServlet.java
index 2b16ef5..fd93126 100644
--- a/java/com/google/gitiles/RefServlet.java
+++ b/java/com/google/gitiles/RefServlet.java
@@ -16,7 +16,6 @@
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
-import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
@@ -25,6 +24,7 @@
import com.google.common.collect.Ordering;
import com.google.common.primitives.Ints;
import com.google.common.util.concurrent.UncheckedExecutionException;
+import com.google.gitiles.GitilesRequestFailureException.FailureReason;
import com.google.gson.reflect.TypeToken;
import java.io.IOException;
import java.io.Writer;
@@ -59,8 +59,7 @@
@Override
protected void doGetHtml(HttpServletRequest req, HttpServletResponse res) throws IOException {
if (!ViewFilter.getView(req).getPathPart().isEmpty()) {
- res.setStatus(SC_NOT_FOUND);
- return;
+ throw new GitilesRequestFailureException(FailureReason.INCORECT_PARAMETER);
}
List<Map<String, Object>> tags;
try (RevWalk walk = new RevWalk(ServletUtils.getRepository(req))) {
diff --git a/java/com/google/gitiles/RepositoryFilter.java b/java/com/google/gitiles/RepositoryFilter.java
index fa9d601..a25a8de 100644
--- a/java/com/google/gitiles/RepositoryFilter.java
+++ b/java/com/google/gitiles/RepositoryFilter.java
@@ -16,11 +16,9 @@
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.gitiles.ViewFilter.getRegexGroup;
-import static javax.servlet.http.HttpServletResponse.SC_FORBIDDEN;
-import static javax.servlet.http.HttpServletResponse.SC_UNAUTHORIZED;
-import static org.eclipse.jgit.http.server.GitSmartHttpTools.sendError;
import static org.eclipse.jgit.http.server.ServletUtils.ATTRIBUTE_REPOSITORY;
+import com.google.gitiles.GitilesRequestFailureException.FailureReason;
import java.io.IOException;
import javax.servlet.FilterChain;
import javax.servlet.ServletException;
@@ -28,12 +26,12 @@
import javax.servlet.http.HttpServletResponse;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.transport.ServiceMayNotContinueException;
import org.eclipse.jgit.transport.resolver.RepositoryResolver;
import org.eclipse.jgit.transport.resolver.ServiceNotAuthorizedException;
import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException;
class RepositoryFilter extends AbstractHttpFilter {
+
private final RepositoryResolver<HttpServletRequest> resolver;
RepositoryFilter(RepositoryResolver<HttpServletRequest> resolver) {
@@ -53,15 +51,13 @@
// to HostIndexServlet which will attempt to list repositories
// or send SC_NOT_FOUND there.
chain.doFilter(req, res);
- } catch (ServiceMayNotContinueException e) {
- sendError(req, res, e.getStatusCode(), e.getMessage());
} finally {
req.removeAttribute(ATTRIBUTE_REPOSITORY);
}
} catch (ServiceNotEnabledException e) {
- sendError(req, res, SC_FORBIDDEN);
+ throw new GitilesRequestFailureException(FailureReason.SERVICE_NOT_ENABLED, e);
} catch (ServiceNotAuthorizedException e) {
- res.sendError(SC_UNAUTHORIZED);
+ throw new GitilesRequestFailureException(FailureReason.NOT_AUTHORIZED, e);
}
}
}
diff --git a/java/com/google/gitiles/RepositoryIndexServlet.java b/java/com/google/gitiles/RepositoryIndexServlet.java
index 7d7ac36..334f6fe 100644
--- a/java/com/google/gitiles/RepositoryIndexServlet.java
+++ b/java/com/google/gitiles/RepositoryIndexServlet.java
@@ -15,13 +15,13 @@
package com.google.gitiles;
import static com.google.common.base.Preconditions.checkNotNull;
-import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps;
import com.google.gitiles.DateFormatter.Format;
+import com.google.gitiles.GitilesRequestFailureException.FailureReason;
import com.google.gitiles.doc.MarkdownConfig;
import com.google.gson.reflect.TypeToken;
import com.google.template.soy.data.SanitizedContent;
@@ -65,8 +65,7 @@
// If the repository didn't exist a prior filter would have 404 replied.
Optional<FormatType> format = getFormat(req);
if (!format.isPresent()) {
- res.sendError(SC_BAD_REQUEST);
- return;
+ throw new GitilesRequestFailureException(FailureReason.UNSUPPORTED_RESPONSE_FORMAT);
}
switch (format.get()) {
case HTML:
@@ -77,8 +76,7 @@
case TEXT:
case DEFAULT:
default:
- res.sendError(SC_BAD_REQUEST);
- break;
+ throw new GitilesRequestFailureException(FailureReason.UNSUPPORTED_RESPONSE_FORMAT);
}
}
diff --git a/java/com/google/gitiles/RevisionServlet.java b/java/com/google/gitiles/RevisionServlet.java
index 597c053..8bf661f 100644
--- a/java/com/google/gitiles/RevisionServlet.java
+++ b/java/com/google/gitiles/RevisionServlet.java
@@ -15,7 +15,6 @@
package com.google.gitiles;
import static com.google.common.base.Preconditions.checkNotNull;
-import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND;
import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
import static org.eclipse.jgit.lib.Constants.OBJ_COMMIT;
import static org.eclipse.jgit.lib.Constants.OBJ_TAG;
@@ -28,6 +27,7 @@
import com.google.gitiles.CommitData.Field;
import com.google.gitiles.CommitJsonData.Commit;
import com.google.gitiles.DateFormatter.Format;
+import com.google.gitiles.GitilesRequestFailureException.FailureReason;
import java.io.IOException;
import java.io.OutputStream;
import java.io.Writer;
@@ -125,18 +125,12 @@
new TagSoyData(linkifier, req).toSoyData(walk, (RevTag) obj, df)));
break;
default:
- log.warn("Bad object type for {}: {}", ObjectId.toString(obj.getId()), obj.getType());
- res.setStatus(SC_NOT_FOUND);
- return;
+ throw new GitilesRequestFailureException(FailureReason.UNSUPPORTED_OBJECT_TYPE);
}
} catch (MissingObjectException e) {
- log.warn("Missing object " + ObjectId.toString(obj.getId()), e);
- res.setStatus(SC_NOT_FOUND);
- return;
+ throw new GitilesRequestFailureException(FailureReason.OBJECT_NOT_FOUND, e);
} catch (IncorrectObjectTypeException e) {
- log.warn("Incorrect object type for " + ObjectId.toString(obj.getId()), e);
- res.setStatus(SC_NOT_FOUND);
- return;
+ throw new GitilesRequestFailureException(FailureReason.INCORRECT_OBJECT_TYPE, e);
}
}
@@ -159,7 +153,7 @@
try (ObjectReader reader = repo.newObjectReader()) {
ObjectLoader loader = reader.open(view.getRevision().getId());
if (loader.getType() != OBJ_COMMIT) {
- res.setStatus(SC_NOT_FOUND);
+ throw new GitilesRequestFailureException(FailureReason.UNSUPPORTED_OBJECT_TYPE);
} else {
PathServlet.setTypeHeader(res, loader.getType());
try (Writer writer = startRenderText(req, res);
@@ -188,8 +182,7 @@
break;
default:
// TODO(dborowitz): Support showing other types.
- res.setStatus(SC_NOT_FOUND);
- break;
+ throw new GitilesRequestFailureException(FailureReason.UNSUPPORTED_OBJECT_TYPE);
}
}
}
diff --git a/java/com/google/gitiles/RootedDocServlet.java b/java/com/google/gitiles/RootedDocServlet.java
index 96329b1..bf72b32 100644
--- a/java/com/google/gitiles/RootedDocServlet.java
+++ b/java/com/google/gitiles/RootedDocServlet.java
@@ -16,6 +16,7 @@
import static org.eclipse.jgit.http.server.ServletUtils.ATTRIBUTE_REPOSITORY;
+import com.google.gitiles.GitilesRequestFailureException.FailureReason;
import com.google.gitiles.doc.DocServlet;
import com.google.gitiles.doc.HtmlSanitizer;
import java.io.IOException;
@@ -24,7 +25,6 @@
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
-import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
@@ -74,14 +74,12 @@
RevWalk rw = new RevWalk(repo)) {
ObjectId id = repo.resolve(BRANCH);
if (id == null) {
- res.sendError(HttpServletResponse.SC_NOT_FOUND);
- return;
+ throw new GitilesRequestFailureException(FailureReason.OBJECT_NOT_FOUND);
}
RevObject obj = rw.peel(rw.parseAny(id));
if (!(obj instanceof RevCommit)) {
- res.sendError(HttpServletResponse.SC_NOT_FOUND);
- return;
+ throw new GitilesRequestFailureException(FailureReason.INCORRECT_OBJECT_TYPE);
}
req.setAttribute(ATTRIBUTE_REPOSITORY, repo);
@@ -95,11 +93,10 @@
.build());
docServlet.service(req, res);
- } catch (RepositoryNotFoundException
- | ServiceNotAuthorizedException
- | ServiceNotEnabledException e) {
- log.error(String.format("cannot open repository for %s", req.getServerName()), e);
- res.sendError(HttpServletResponse.SC_NOT_FOUND);
+ } catch (ServiceNotAuthorizedException e) {
+ throw new GitilesRequestFailureException(FailureReason.NOT_AUTHORIZED, e);
+ } catch (ServiceNotEnabledException e) {
+ throw new GitilesRequestFailureException(FailureReason.SERVICE_NOT_ENABLED, e);
} finally {
ViewFilter.removeView(req);
req.removeAttribute(ATTRIBUTE_REPOSITORY);
diff --git a/java/com/google/gitiles/ViewFilter.java b/java/com/google/gitiles/ViewFilter.java
index 03e8d9a..590e583 100644
--- a/java/com/google/gitiles/ViewFilter.java
+++ b/java/com/google/gitiles/ViewFilter.java
@@ -16,12 +16,10 @@
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
-import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND;
-import static javax.servlet.http.HttpServletResponse.SC_SERVICE_UNAVAILABLE;
-import static org.eclipse.jgit.http.server.GitSmartHttpTools.sendError;
import static org.eclipse.jgit.http.server.ServletUtils.ATTRIBUTE_REPOSITORY;
import com.google.common.base.Strings;
+import com.google.gitiles.GitilesRequestFailureException.FailureReason;
import java.io.IOException;
import java.util.Map;
import javax.servlet.FilterChain;
@@ -30,7 +28,6 @@
import javax.servlet.http.HttpServletResponse;
import org.eclipse.jgit.http.server.ServletUtils;
import org.eclipse.jgit.http.server.glue.WrappedRequest;
-import org.eclipse.jgit.transport.ServiceMayNotContinueException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -102,21 +99,9 @@
@Override
public void doFilter(HttpServletRequest req, HttpServletResponse res, FilterChain chain)
throws IOException, ServletException {
- GitilesView.Builder view;
- try {
- view = parse(req);
- } catch (ServiceMayNotContinueException e) {
- sendError(req, res, e.getStatusCode(), e.getMessage());
- return;
- } catch (IOException err) {
- String name = urls.getHostName(req);
- log.warn("Cannot parse view" + (name != null ? " for " + name : ""), err);
- res.setStatus(SC_SERVICE_UNAVAILABLE);
- return;
- }
+ GitilesView.Builder view = parse(req);
if (view == null) {
- res.setStatus(SC_NOT_FOUND);
- return;
+ throw new GitilesRequestFailureException(FailureReason.CANNOT_PARSE_GITILES_VIEW);
}
@SuppressWarnings("unchecked")
diff --git a/java/com/google/gitiles/blame/BlameServlet.java b/java/com/google/gitiles/blame/BlameServlet.java
index 605ac6f..f1b9976 100644
--- a/java/com/google/gitiles/blame/BlameServlet.java
+++ b/java/com/google/gitiles/blame/BlameServlet.java
@@ -15,7 +15,6 @@
package com.google.gitiles.blame;
import static com.google.common.base.Preconditions.checkNotNull;
-import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
@@ -29,6 +28,8 @@
import com.google.gitiles.GitilesAccess;
import com.google.gitiles.GitilesView;
import com.google.gitiles.Renderer;
+import com.google.gitiles.GitilesRequestFailureException;
+import com.google.gitiles.GitilesRequestFailureException.FailureReason;
import com.google.gitiles.ViewFilter;
import com.google.gitiles.blame.cache.BlameCache;
import com.google.gitiles.blame.cache.Region;
@@ -159,8 +160,7 @@
RevCommit currCommit = rw.parseCommit(view.getRevision().getId());
ObjectId currCommitBlobId = resolveBlob(view, rw, currCommit);
if (currCommitBlobId == null) {
- res.setStatus(SC_NOT_FOUND);
- return null;
+ throw new GitilesRequestFailureException(FailureReason.OBJECT_NOT_FOUND);
}
ObjectId lastCommit = cache.findLastCommit(repo, currCommit, view.getPathPart());
@@ -182,8 +182,7 @@
List<Region> regions = cache.get(repo, lastCommit, view.getPathPart());
if (regions.isEmpty()) {
- res.setStatus(SC_NOT_FOUND);
- return null;
+ throw new GitilesRequestFailureException(FailureReason.BLAME_REGION_NOT_FOUND);
}
return new RegionResult(regions, lastCommitBlobId);
}
diff --git a/java/com/google/gitiles/doc/DocServlet.java b/java/com/google/gitiles/doc/DocServlet.java
index 24f8d4b..509f460 100644
--- a/java/com/google/gitiles/doc/DocServlet.java
+++ b/java/com/google/gitiles/doc/DocServlet.java
@@ -32,6 +32,8 @@
import com.google.gitiles.GitilesAccess;
import com.google.gitiles.GitilesView;
import com.google.gitiles.Renderer;
+import com.google.gitiles.GitilesRequestFailureException.FailureReason;
+import com.google.gitiles.GitilesRequestFailureException;
import com.google.gitiles.ViewFilter;
import com.google.gitiles.doc.html.StreamHtmlBuilder;
import java.io.IOException;
@@ -86,8 +88,7 @@
protected void doGetHtml(HttpServletRequest req, HttpServletResponse res) throws IOException {
MarkdownConfig cfg = MarkdownConfig.get(getAccess(req).getConfig());
if (!cfg.render) {
- res.setStatus(SC_NOT_FOUND);
- return;
+ throw new GitilesRequestFailureException(FailureReason.MARKDOWN_NOT_ENABLED);
}
GitilesView view = ViewFilter.getView(req);
@@ -99,14 +100,12 @@
try {
root = rw.parseTree(view.getRevision().getId());
} catch (IncorrectObjectTypeException e) {
- res.setStatus(SC_NOT_FOUND);
- return;
+ throw new GitilesRequestFailureException(FailureReason.INCORRECT_OBJECT_TYPE);
}
MarkdownFile srcmd = findFile(rw, root, path);
if (srcmd == null) {
- res.setStatus(SC_NOT_FOUND);
- return;
+ throw new GitilesRequestFailureException(FailureReason.OBJECT_NOT_FOUND);
}
MarkdownFile navmd = findNavbar(rw, root, path);
@@ -125,9 +124,6 @@
} catch (LargeObjectException.ExceedsLimit errBig) {
fileTooBig(res, view, errBig);
return;
- } catch (IOException err) {
- readError(res, view, err);
- return;
}
MarkdownToHtml.Builder fmt =
@@ -280,29 +276,12 @@
HttpServletResponse res, GitilesView view, LargeObjectException.ExceedsLimit errBig)
throws IOException {
if (view.getType() == GitilesView.Type.ROOTED_DOC) {
- log.error(
- String.format(
- "markdown too large: %s/%s %s %s: %s",
- view.getHostName(),
- view.getRepositoryName(),
- view.getRevision(),
- view.getPathPart(),
- errBig.getMessage()));
- res.setStatus(SC_INTERNAL_SERVER_ERROR);
+ throw new GitilesRequestFailureException(FailureReason.OBJECT_TOO_LARGE);
} else {
res.sendRedirect(GitilesView.show().copyFrom(view).toUrl());
}
}
- private static void readError(HttpServletResponse res, GitilesView view, IOException err) {
- log.error(
- String.format(
- "cannot load markdown %s/%s %s %s",
- view.getHostName(), view.getRepositoryName(), view.getRevision(), view.getPathPart()),
- err);
- res.setStatus(SC_INTERNAL_SERVER_ERROR);
- }
-
private static class MarkdownFile {
final String path;
final ObjectId id;
diff --git a/javatests/com/google/gitiles/MoreAssert.java b/javatests/com/google/gitiles/MoreAssert.java
new file mode 100644
index 0000000..bd80d69
--- /dev/null
+++ b/javatests/com/google/gitiles/MoreAssert.java
@@ -0,0 +1,38 @@
+// Copyright 2019 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.gitiles;
+
+/** Assertion methods for Gitiles. */
+public class MoreAssert {
+ private MoreAssert() {}
+
+ /** Simple version of assertThrows that will be introduced in JUnit 4.13. */
+ public static <T extends Throwable> T assertThrows(Class<T> expected, ThrowingRunnable r) {
+ try {
+ r.run();
+ throw new AssertionError("Expected " + expected.getSimpleName() + " to be thrown");
+ } catch (Throwable actual) {
+ if (expected.isAssignableFrom(actual.getClass())) {
+ return (T) actual;
+ }
+ throw new AssertionError(
+ "Expected " + expected.getSimpleName() + ", but got " + actual.getClass().getSimpleName(),
+ actual);
+ }
+ }
+
+ public interface ThrowingRunnable {
+ void run() throws Throwable;
+ }
+}
diff --git a/javatests/com/google/gitiles/ViewFilterTest.java b/javatests/com/google/gitiles/ViewFilterTest.java
index 2a8cb65..d25b6cf 100644
--- a/javatests/com/google/gitiles/ViewFilterTest.java
+++ b/javatests/com/google/gitiles/ViewFilterTest.java
@@ -16,6 +16,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
+import static com.google.gitiles.MoreAssert.assertThrows;
import com.google.common.net.HttpHeaders;
import com.google.gitiles.GitilesView.Type;
@@ -45,8 +46,8 @@
public void noCommand() throws Exception {
assertThat(getView("/").getType()).isEqualTo(Type.HOST_INDEX);
assertThat(getView("/repo").getType()).isEqualTo(Type.REPOSITORY_INDEX);
- assertThat(getView("/repo/+")).isNull();
- assertThat(getView("/repo/+/")).isNull();
+ assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+"));
+ assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+/"));
}
@Test
@@ -136,8 +137,8 @@
public void describe() throws Exception {
GitilesView view;
- assertThat(getView("/repo/+describe")).isNull();
- assertThat(getView("/repo/+describe/")).isNull();
+ assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+describe"));
+ assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+describe/"));
view = getView("/repo/+describe/deadbeef");
assertThat(view.getType()).isEqualTo(Type.DESCRIBE);
@@ -184,7 +185,7 @@
assertThat(view.getRevision().getId()).isEqualTo(stable);
assertThat(view.getPathPart()).isNull();
- assertThat(getView("/repo/+show/stable..master")).isNull();
+ assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+show/stable..master"));
}
@Test
@@ -250,7 +251,7 @@
assertThat(view.getRevision().getId()).isEqualTo(master);
assertThat(view.getPathPart()).isEqualTo("foo/bar");
- assertThat(getView("/repo/+show/stable..master/foo")).isNull();
+ assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+show/stable..master/foo"));
}
@Test
@@ -279,7 +280,7 @@
assertThat(view.getRevision().getId()).isEqualTo(master);
assertThat(view.getPathPart()).isEqualTo("foo/bar.md");
- assertThat(getView("/repo/+doc/stable..master/foo")).isNull();
+ assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+doc/stable..master/foo"));
}
@Test
@@ -288,10 +289,10 @@
assertThat(getView("//").getType()).isEqualTo(Type.HOST_INDEX);
assertThat(getView("//repo").getType()).isEqualTo(Type.REPOSITORY_INDEX);
assertThat(getView("//repo//").getType()).isEqualTo(Type.REPOSITORY_INDEX);
- assertThat(getView("/repo/+//master")).isNull();
- assertThat(getView("/repo/+/refs//heads//master")).isNull();
- assertThat(getView("/repo/+//master//")).isNull();
- assertThat(getView("/repo/+//master/foo//bar")).isNull();
+ assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+//master"));
+ assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+/refs//heads//master"));
+ assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+//master//"));
+ assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+//master/foo//bar"));
}
@Test
@@ -420,13 +421,13 @@
repo.branch("refs/heads/branch").commit().create();
GitilesView view;
- assertThat(getView("/repo/+archive")).isNull();
- assertThat(getView("/repo/+archive/")).isNull();
- assertThat(getView("/repo/+archive/master..branch")).isNull();
- assertThat(getView("/repo/+archive/master.foo")).isNull();
- assertThat(getView("/repo/+archive/master.zip")).isNull();
- assertThat(getView("/repo/+archive/master/.tar.gz")).isNull();
- assertThat(getView("/repo/+archive/master/foo/.tar.gz")).isNull();
+ assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+archive"));
+ assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+archive/"));
+ assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+archive/master..branch"));
+ assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+archive/master.foo"));
+ assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+archive/master.zip"));
+ assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+archive/master/.tar.gz"));
+ assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+archive/master/foo/.tar.gz"));
view = getView("/repo/+archive/master.tar.gz");
assertThat(view.getType()).isEqualTo(Type.ARCHIVE);
@@ -462,10 +463,10 @@
repo.branch("refs/heads/branch").commit().create();
GitilesView view;
- assertThat(getView("/repo/+blame")).isNull();
- assertThat(getView("/repo/+blame/")).isNull();
- assertThat(getView("/repo/+blame/master")).isNull();
- assertThat(getView("/repo/+blame/master..branch")).isNull();
+ assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+blame"));
+ assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+blame/"));
+ assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+blame/master"));
+ assertThrows(GitilesRequestFailureException.class, () -> getView("/repo/+blame/master..branch"));
view = getView("/repo/+blame/master/foo/bar");
assertThat(view.getType()).isEqualTo(Type.BLAME);