dsmiley commented on code in PR #1780:
URL: https://github.com/apache/solr/pull/1780#discussion_r1266203328


##########
solr/core/src/java/org/apache/solr/servlet/ServletUtils.java:
##########
@@ -197,35 +197,30 @@ static void configExcludes(PathExcluder excluder, String 
patternConfig) {
    * @param request The request to limit
    * @param response The associated response
    * @param limitedExecution code that will be traced
-   * @param trace a boolean that turns tracing on or off
    */
   static void rateLimitRequest(
+      RateLimitManager rateLimitManager,
       HttpServletRequest request,
       HttpServletResponse response,
-      Runnable limitedExecution,
-      boolean trace)
+      Runnable limitedExecution)
       throws ServletException, IOException {
     boolean accepted = false;
-    RateLimitManager rateLimitManager = getRateLimitManager(request);
     try {
-      try {
-        accepted = rateLimitManager.handleRequest(request);
-      } catch (InterruptedException e) {
-        Thread.currentThread().interrupt();
-        throw new SolrException(ErrorCode.SERVER_ERROR, e.getMessage());
-      }
-
+      accepted = rateLimitManager.handleRequest(request);
       if (!accepted) {
-        String errorMessage =
-            "Too many requests for this request type."
-                + "Please try after some time or increase the quota for this 
request type";
-
-        response.sendError(429, errorMessage);
+        response.sendError(ErrorCode.TOO_MANY_REQUESTS.code, 
RateLimitManager.ERROR_MESSAGE);
+        return;
       }
       // todo: this shouldn't be required, tracing and rate limiting should be 
independently
       // composable
-      traceHttpRequestExecution2(request, response, limitedExecution, trace);
+      traceHttpRequestExecution2(request, response, limitedExecution);
+    } catch (InterruptedException e) {
+      Thread.currentThread().interrupt();
+      throw new SolrException(ErrorCode.SERVER_ERROR, e.getMessage());
     } finally {
+      consumeInputFully(request, response);

Review Comment:
   Lets move these three lines somewhere else as this doesn't have to do with 
reate limiting (this method).  Perhaps generic doFilter.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org

Reply via email to