From 491cae6c604b4b9d94d3ff8277b747b87f2b599c Mon Sep 17 00:00:00 2001
From: evazion <noizave@gmail.com>
Date: Sat, 24 Aug 2019 22:55:36 -0500
Subject: [PATCH] app controller: standardize api limit error handling.

* Refactor api_check to use render_error_page so that api limit errors
  get logged to New Relic for analysis.

* Also standardize json error responses to return the error message in
  `message` instead of `reason`.
---
 app/controllers/application_controller.rb   | 40 ++++++++-------------
 app/views/static/too_many_requests.html.erb |  2 --
 2 files changed, 14 insertions(+), 28 deletions(-)
 delete mode 100644 app/views/static/too_many_requests.html.erb

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index c75bfebdd..b1831e66c 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -1,4 +1,6 @@
 class ApplicationController < ActionController::Base
+  class ApiLimitError < StandardError; end
+
   skip_forgery_protection if: -> { SessionLoader.new(request).has_api_authentication? }
   helper :pagination
   before_action :reset_current_user
@@ -44,35 +46,19 @@ class ApplicationController < ActionController::Base
   end
 
   def api_check
-    if !CurrentUser.is_anonymous? && !request.get? && !request.head?
-      if CurrentUser.user.token_bucket.nil?
-        TokenBucket.create_default(CurrentUser.user)
-        CurrentUser.user.reload
-      end
+    return if CurrentUser.is_anonymous? || request.get? || request.head?
 
-      throttled = CurrentUser.user.token_bucket.throttled?
-      headers["X-Api-Limit"] = CurrentUser.user.token_bucket.token_count.to_s
-
-      if throttled
-        respond_to do |format|
-          format.json do
-            render json: {success: false, reason: "too many requests"}.to_json, status: 429
-          end
-
-          format.xml do
-            render xml: {success: false, reason: "too many requests"}.to_xml(:root => "response"), status: 429
-          end
-
-          format.html do
-            render :template => "static/too_many_requests", :status => 429
-          end
-        end
-
-        return false
-      end
+    if CurrentUser.user.token_bucket.nil?
+      TokenBucket.create_default(CurrentUser.user)
+      CurrentUser.user.reload
     end
 
-    return true
+    throttled = CurrentUser.user.token_bucket.throttled?
+    headers["X-Api-Limit"] = CurrentUser.user.token_bucket.token_count.to_s
+
+    if throttled
+      raise ApiLimitError, "too many requests"
+    end
   end
 
   def rescue_exception(exception)
@@ -93,6 +79,8 @@ class ApplicationController < ActionController::Base
       render_error_page(410, exception)
     when Post::SearchError
       render_error_page(422, exception)
+    when ApiLimitError
+      render_error_page(429, exception)
     when NotImplementedError
       render_error_page(501, exception, message: "This feature isn't available: #{exception.message}")
     when PG::ConnectionBad
diff --git a/app/views/static/too_many_requests.html.erb b/app/views/static/too_many_requests.html.erb
deleted file mode 100644
index 4d9f56c85..000000000
--- a/app/views/static/too_many_requests.html.erb
+++ /dev/null
@@ -1,2 +0,0 @@
-<h1>Too Many Requests</h1>
-<p>Please rate limit yourself</p>
\ No newline at end of file