From de88a08ebd2bb516a04c942353f8e526295670ed Mon Sep 17 00:00:00 2001
From: Molly Struve <mollylbs@gmail.com>
Date: Wed, 22 Apr 2020 08:55:26 -0500
Subject: [PATCH] [deploy] handle rate limit errors gracefully (#7422)

---
 app/controllers/api/v0/api_controller.rb |  6 ++++++
 app/labor/rate_limit_checker.rb          |  1 +
 app/services/articles/creator.rb         |  2 +-
 config/initializers/honeybadger.rb       |  1 +
 spec/requests/api/v0/articles_spec.rb    |  9 +++++++++
 spec/requests/articles/articles_spec.rb  | 13 +++++++++++++
 6 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/app/controllers/api/v0/api_controller.rb b/app/controllers/api/v0/api_controller.rb
index 59d7fd99e..53334c6a6 100644
--- a/app/controllers/api/v0/api_controller.rb
+++ b/app/controllers/api/v0/api_controller.rb
@@ -21,6 +21,8 @@ class Api::V0::ApiController < ApplicationController
     error_unauthorized
   end
 
+  rescue_from RateLimitChecker::LimitReached, with: :too_many_requests
+
   protected
 
   def error_unprocessable_entity(message)
@@ -35,6 +37,10 @@ class Api::V0::ApiController < ApplicationController
     render json: { error: "not found", status: 404 }, status: :not_found
   end
 
+  def too_many_requests
+    render json: { error: "too many requests", status: 429 }, status: :too_many_requests
+  end
+
   def authenticate!
     if doorkeeper_token
       @user = User.find(doorkeeper_token.resource_owner_id)
diff --git a/app/labor/rate_limit_checker.rb b/app/labor/rate_limit_checker.rb
index 0698a11f0..5836c4a68 100644
--- a/app/labor/rate_limit_checker.rb
+++ b/app/labor/rate_limit_checker.rb
@@ -7,6 +7,7 @@ class RateLimitChecker
 
   class UploadRateLimitReached < StandardError; end
   class DailyFollowAccountLimitReached < StandardError; end
+  class LimitReached < StandardError; end
 
   def limit_by_action(action)
     check_method = "check_#{action}_limit"
diff --git a/app/services/articles/creator.rb b/app/services/articles/creator.rb
index 4e42cdf4a..c84605e24 100644
--- a/app/services/articles/creator.rb
+++ b/app/services/articles/creator.rb
@@ -11,7 +11,7 @@ module Articles
     end
 
     def call
-      raise if RateLimitChecker.new(user).limit_by_action("published_article_creation")
+      raise RateLimitChecker::LimitReached if RateLimitChecker.new(user).limit_by_action("published_article_creation")
 
       article = save_article
 
diff --git a/config/initializers/honeybadger.rb b/config/initializers/honeybadger.rb
index 06d1eed56..3fe695f45 100644
--- a/config/initializers/honeybadger.rb
+++ b/config/initializers/honeybadger.rb
@@ -19,6 +19,7 @@ Honeybadger.configure do |config|
     Pundit::NotAuthorizedError,
     ActiveRecord::RecordNotFound,
     ActiveRecord::QueryCanceled,
+    RateLimitChecker::LimitReached,
   ]
   config.request.filter_keys += %w[authorization]
   config.sidekiq.attempt_threshold = 10
diff --git a/spec/requests/api/v0/articles_spec.rb b/spec/requests/api/v0/articles_spec.rb
index b8a2a21e4..a3e3cd29a 100644
--- a/spec/requests/api/v0/articles_spec.rb
+++ b/spec/requests/api/v0/articles_spec.rb
@@ -445,6 +445,15 @@ RSpec.describe "Api::V0::Articles", type: :request do
     let_it_be(:api_secret) { create(:api_secret) }
     let_it_be(:user) { api_secret.user }
 
+    context "when creation limit is reached" do
+      it "returns a 429 status code and error" do
+        allow(Articles::Creator).to receive(:call).and_raise(RateLimitChecker::LimitReached)
+        headers = { "api-key" => api_secret.secret, "content-type" => "application/json" }
+        post api_articles_path, params: { article: { body_markdown: "" } }.to_json, headers: headers
+        expect(response).to have_http_status(:too_many_requests)
+      end
+    end
+
     context "when unauthorized" do
       it "fails with no api key" do
         post api_articles_path, headers: { "content-type" => "application/json" }
diff --git a/spec/requests/articles/articles_spec.rb b/spec/requests/articles/articles_spec.rb
index 4d1502f7d..6b10006ad 100644
--- a/spec/requests/articles/articles_spec.rb
+++ b/spec/requests/articles/articles_spec.rb
@@ -218,4 +218,17 @@ RSpec.describe "Articles", type: :request do
       end
     end
   end
+
+  describe "POST /create" do
+    before { sign_in user }
+
+    context "when creation limit is reached" do
+      it "raises a rate limit reached error" do
+        allow(Articles::Creator).to receive(:call).and_raise(RateLimitChecker::LimitReached)
+        expect do
+          post articles_path, params: { article: { markdown: "123" } }
+        end.to raise_error(RateLimitChecker::LimitReached)
+      end
+    end
+  end
 end