)]}' { "commit": "b53fc06a4b2238eb084af62490af188e2b93d36d", "tree": "dead0f5171317f764a13b40acf9202859e30b064", "parents": [ "76346b0810deec731737c9bb2bf54cc46a2c88cc" ], "author": { "name": "Masaya Suzuki", "email": "masayasuzuki@google.com", "time": "Mon Mar 25 13:35:07 2019 -0700" }, "committer": { "name": "Masaya Suzuki", "email": "masayasuzuki@google.com", "time": "Thu May 09 17:14:03 2019 -0700" }, "message": "Introduce GitilesRequestFailureException and DefaultErrorHandlingFilter\n\nThis change introduces an exception for failing a request. This isolates\nthe error checks and the error handling. For example, some part of\nGitiles application code checks if a request URL points to a valid\nobject. When it finds the URL pointing to a non-existing object, the\ncurrent code is also responsible for rendering an error for a user. With\nthis change, the application code becomes not responsible for rendering\nan error page, and it just needs to throw a\nGitilesRequestFailureException with an appropriate failure reason.\n\nThis isolation has multiple benefits:\n\n- Unified error handling code\n\n Because the error page rendering is done in multiple places, some\n code uses 400 Bad Request while others use 404 Not Found for the\n same object not found case. This behavior will be unified.\n\n- Customizable error handler\n\n A user who uses Gitiles as a Java library can customize error\n handlers. This lets them log an error in the server log.\n\n- Exception stack trace is preserved\n\n Previously, the exception stack trace is not used at all because\n Gitiles just renders an error page with\n HttpServletResponse#sendError. Now with a customized error handler,\n a Gitiles user have a chance to log exception stack trace in the\n server log.\n\n* Design Decisions\n\n** Subclass vs. enum\n\nGitilesRequestFailureException could have defined as a abstract base\nclass that each subclass represents a different situation. This is\nuseful for situation where there are two types of callers; the ones that\ndo not care about the subtypes and want to handle all subtypes of\nGitilesRequestFailureException in the same way, and the ones that want\nto handle some subtypes of GitilesRequestFailureException differently.\nFor example, EOFException is defined as a subtype of IOException. For\nthose that don\u0027t care about EOF or not can handle all exceptions thrown\nfrom IO with just `catch (IOException e)`, while those that want to\nhandle EOF as a special case can handle it separately.\n\nThis GitilesRequestFailureException is intended to be thrown when a\nrequest cannot be processed and should be returned immediately.\nConceptually, this is same as `System.exit` but for requests. This\nshould be handled only by DefaultErrorHandlingFilter or its equivalent.\nHandling this exception in other application code is a misuse of this\nexception and they should use a different exception. By making it\npossible to subclass GitilesRequestFailureException, it makes it easier\nto misuse this exception since the application code can partially handle\nthis exception easily. Based on Scott Meyers advice \"Make interfaces\neasy to use correctly and hard to use incorrectly\" [1], this class\ndoesn\u0027t take subclassing approach and use enum to represent an error\ntype. An enum serves enough for DefaultErrorHandlingFilter\u0027s purpose.\n\nAs a side effect by using an enum, we can now send a programmatically\nparsable error type that is finer than HTTP status.\nDefaultErrorHandlingFilter sets the enum to the HTTP response header.\nThis makes it easy for the Gitiles REST API users to handle or diagnose\nerrors.\n\n** Extensibility\n\nAs seen in the constructor of GitilesServlet, Gitiles can be extended by\na user. There can be a case that user code wants to terminate a request\nimmediately. For example, a user would want to set a quota on the number\nof requests, and they might want to response to a request immediately\nwhen a quota is depleted with 429 Too Many Requests.\nGitilesRequestFailureException and its FailureReason enum cannot be\nextended by a user without modifying the Gitiles source code. They\ncannot throw GitilesRequestFailureException in this case.\n\nOne approach for this problem is to define a class like\nServiceMayNotContinueException in JGit. This exception takes an HTTP\nstatus code and JGit custom hooks can throw this exception with\narbitrary status code.\n\nThis change takes a different approach. If a user wants to return a user\ndefined error, they can do so by defining a user defined exception and\nits corresponding error handler. For example, they can define their own\nRuntimeException that looks like GitilesRequestFailureException, and\ndefine a new error handling filter that catches it.\n\n public final class MyRequestFailureException extends RuntimeException {\n private final FailureReason reason;\n\n public MyRequestFailureException(FailureReason reason) {\n super();\n this.reason \u003d reason;\n }\n\n public FailureReason getReason() {\n return reason;\n }\n\n enum FailureReason {\n QUOTA_EXCEEDED(429);\n }\n\n private final int httpStatusCode;\n\n FailureReason(int httpStatusCode) {\n this.httpStatusCode \u003d httpStatusCode;\n }\n\n public int getHttpStatusCode() {\n return httpStatusCode;\n }\n }\n\n public class MyErrorHandlingFilter extends AbstractHttpFilter {\n private static final DefaultErrorHandlingFilter delegate \u003d\n new DefaultErrorHandlingFilter();\n\n @Override\n public void doFilter(HttpServletRequest req, HttpServletResponse res, FilterChain chain)\n throws IOException, ServletException {\n try {\n delegate(req, res, chain);\n } catch (MyRequestFailureException e) {\n res.setHeader(\n DefaultErrorHandlingFilter.GITILES_ERROR,\n e.getReason().toString());\n res.sendError(e.getReason().getHttpStatusCode());\n }\n }\n }\n\nCompared to ServiceMayNotContinueException approach, this gives a user\ngreater flexibility. ServiceMayNotContinueException lets a user specify\nonly HTTP status code and an error message. This custom exception\nhandler approach lets a user decide how an error should be surfaced\ncompletely. For example, a user can extend the exception and add the\ntime that an HTTP request caller should retry to recover the quota. The\ncustom error handler can show such data in the error page or in the HTTP\nheader for a REST API client.\n\n* Behavioral change\n\nWhen an object specified by the URL is not suitable for the view (e.g.\ntrying to show a blob as a tree), it used to show SC_NOT_FOUND. It now\nreturns SC_BAD_REQUEST.\n\n[1]: https://www.aristeia.com/Papers/IEEE_Software_JulAug_2004_revised.htm\n\nChange-Id: I436309324e24be5de6358b96237389ba93dbe9ff\n", "tree_diff": [ { "type": "add", "old_id": "0000000000000000000000000000000000000000", "old_mode": 0, "old_path": "/dev/null", "new_id": "958b8001de64e5ce34d9644430affdc4eee5bf13", "new_mode": 33188, "new_path": "java/com/google/gitiles/DefaultErrorHandlingFilter.java" }, { "type": "modify", "old_id": "59e8acd60ffc73d0b43e3a5a6635ce442f31b0c7", "old_mode": 33188, "old_path": "java/com/google/gitiles/GitilesFilter.java", "new_id": "2c810bba7b9bb7c1c6d6fffe0ce9751f7799e2ed", "new_mode": 33188, "new_path": "java/com/google/gitiles/GitilesFilter.java" }, { "type": "add", "old_id": "0000000000000000000000000000000000000000", "old_mode": 0, "old_path": "/dev/null", "new_id": "91f9cf8ec1f70632341d3ceb013da00e4b92e625", "new_mode": 33188, "new_path": "java/com/google/gitiles/GitilesRequestFailureException.java" }, { "type": "modify", "old_id": "56f4c6162c25e8ecf564150e2ca5f7985023e5c8", "old_mode": 33188, "old_path": "java/com/google/gitiles/GitilesServlet.java", "new_id": "6d10c0c238d4c252f58bf02aae72de1ef1b97601", "new_mode": 33188, "new_path": "java/com/google/gitiles/GitilesServlet.java" }, { "type": "add", "old_id": "0000000000000000000000000000000000000000", "old_mode": 0, "old_path": "/dev/null", "new_id": "68326582a5b4418b34bd15503eb1807e800929b6", "new_mode": 33188, "new_path": "javatests/com/google/gitiles/DefaultErrorHandlingFilterTest.java" } ] }