Extract ErrorResponses from application controller (#38105)

app/controllers/application_controller.rb

@@ -9,6 +9,7 @@ class ApplicationController < ActionController::Base
   include UserTrackingConcern
   include SessionTrackingConcern
   include CacheConcern
+  include ErrorResponses
   include PreloadingConcern
   include DomainControlHelper
   include DatabaseHelper
@@ -23,21 +24,6 @@ class ApplicationController < ActionController::Base
   helper_method :limited_federation_mode?
   helper_method :skip_csrf_meta_tags?
 
-  rescue_from ActionController::ParameterMissing, Paperclip::AdapterRegistry::NoHandlerError, with: :bad_request
-  rescue_from Mastodon::NotPermittedError, with: :forbidden
-  rescue_from ActionController::RoutingError, ActiveRecord::RecordNotFound, with: :not_found
-  rescue_from ActionController::UnknownFormat, with: :not_acceptable
-  rescue_from ActionController::InvalidAuthenticityToken, with: :unprocessable_content
-  rescue_from Mastodon::RateLimitExceededError, with: :too_many_requests
-
-  rescue_from(*Mastodon::HTTP_CONNECTION_ERRORS, with: :internal_server_error)
-  rescue_from Mastodon::RaceConditionError, Stoplight::Error::RedLight, ActiveRecord::SerializationFailure, with: :service_unavailable
-
-  rescue_from Seahorse::Client::NetworkingError do |e|
-    Rails.logger.warn "Storage server error: #{e}"
-    service_unavailable
-  end
-
   before_action :check_self_destruct!
 
   before_action :store_referrer, except: :raise_not_found, if: :devise_controller?
@@ -118,42 +104,6 @@ class ApplicationController < ActionController::Base
     ActiveModel::Type::Boolean.new.cast(params[key])
   end
 
-  def forbidden
-    respond_with_error(403)
-  end
-
-  def not_found
-    respond_with_error(404)
-  end
-
-  def gone
-    respond_with_error(410)
-  end
-
-  def unprocessable_content
-    respond_with_error(422)
-  end
-
-  def not_acceptable
-    respond_with_error(406)
-  end
-
-  def bad_request
-    respond_with_error(400)
-  end
-
-  def internal_server_error
-    respond_with_error(500)
-  end
-
-  def service_unavailable
-    respond_with_error(503)
-  end
-
-  def too_many_requests
-    respond_with_error(429)
-  end
-
   def single_user_mode?
     @single_user_mode ||= Rails.configuration.x.single_user_mode && Account.without_internal.exists?
   end
@@ -178,13 +128,6 @@ class ApplicationController < ActionController::Base
     @current_session = SessionActivation.find_by(session_id: cookies.signed['_session_id']) if cookies.signed['_session_id'].present?
   end
 
-  def respond_with_error(code)
-    respond_to do |format|
-      format.any  { render "errors/#{code}", layout: 'error', status: code, formats: [:html] }
-      format.json { render json: { error: Rack::Utils::HTTP_STATUS_CODES[code] }, status: code }
-    end
-  end
-
   def check_self_destruct!
     return unless self_destruct?
 

app/controllers/concerns/error_responses.rb

@@ -0,0 +1,68 @@
+# frozen_string_literal: true
+
+module ErrorResponses
+  extend ActiveSupport::Concern
+
+  included do
+    rescue_from ActionController::InvalidAuthenticityToken, with: :unprocessable_content
+    rescue_from ActionController::ParameterMissing, Paperclip::AdapterRegistry::NoHandlerError, with: :bad_request
+    rescue_from ActionController::RoutingError, ActiveRecord::RecordNotFound, with: :not_found
+    rescue_from ActionController::UnknownFormat, with: :not_acceptable
+    rescue_from Mastodon::NotPermittedError, with: :forbidden
+    rescue_from Mastodon::RaceConditionError, Stoplight::Error::RedLight, ActiveRecord::SerializationFailure, with: :service_unavailable
+    rescue_from Mastodon::RateLimitExceededError, with: :too_many_requests
+    rescue_from(*Mastodon::HTTP_CONNECTION_ERRORS, with: :internal_server_error)
+
+    rescue_from Seahorse::Client::NetworkingError do |e|
+      Rails.logger.warn "Storage server error: #{e}"
+      service_unavailable
+    end
+  end
+
+  protected
+
+  def bad_request
+    respond_with_error(400)
+  end
+
+  def forbidden
+    respond_with_error(403)
+  end
+
+  def gone
+    respond_with_error(410)
+  end
+
+  def internal_server_error
+    respond_with_error(500)
+  end
+
+  def not_acceptable
+    respond_with_error(406)
+  end
+
+  def not_found
+    respond_with_error(404)
+  end
+
+  def service_unavailable
+    respond_with_error(503)
+  end
+
+  def too_many_requests
+    respond_with_error(429)
+  end
+
+  def unprocessable_content
+    respond_with_error(422)
+  end
+
+  private
+
+  def respond_with_error(code)
+    respond_to do |format|
+      format.any  { render "errors/#{code}", layout: 'error', formats: [:html], status: code }
+      format.json { render json: { error: Rack::Utils::HTTP_STATUS_CODES[code] }, status: code }
+    end
+  end
+end

spec/controllers/application_controller_spec.rb

@@ -6,52 +6,21 @@ RSpec.describe ApplicationController do
   render_views
 
   controller do
-    def success
+    def success = head(200)
-      head 200
-    end
-
-    def routing_error
-      raise ActionController::RoutingError, ''
-    end
-
-    def record_not_found
-      raise ActiveRecord::RecordNotFound, ''
-    end
-
-    def invalid_authenticity_token
-      raise ActionController::InvalidAuthenticityToken, ''
-    end
-  end
-
-  shared_examples 'error response' do |code|
-    it "returns http #{code} for http and renders template" do
-      subject
-
-      expect(response)
-        .to have_http_status(code)
-      expect(response.parsed_body)
-        .to have_css('body[class=error]')
-      expect(response.parsed_body.css('h1').to_s)
-        .to include(error_content(code))
-    end
-
-    def error_content(code)
-      if code == 422
-        I18n.t('errors.422.content')
-      else
-        I18n.t("errors.#{code}")
-      end
-    end
   end
 
   context 'with a forgery' do
-    subject do
+    before do
       ActionController::Base.allow_forgery_protection = true
       routes.draw { post 'success' => 'anonymous#success' }
-      post 'success'
     end
 
-    it_behaves_like 'error response', 422
+    it 'responds with 422 and error page' do
+      post 'success'
+
+      expect(response)
+        .to have_http_status(422)
+    end
   end
 
   describe 'helper_method :current_account' do
@@ -85,33 +54,6 @@ RSpec.describe ApplicationController do
     end
   end
 
-  context 'with ActionController::RoutingError' do
-    subject do
-      routes.draw { get 'routing_error' => 'anonymous#routing_error' }
-      get 'routing_error'
-    end
-
-    it_behaves_like 'error response', 404
-  end
-
-  context 'with ActiveRecord::RecordNotFound' do
-    subject do
-      routes.draw { get 'record_not_found' => 'anonymous#record_not_found' }
-      get 'record_not_found'
-    end
-
-    it_behaves_like 'error response', 404
-  end
-
-  context 'with ActionController::InvalidAuthenticityToken' do
-    subject do
-      routes.draw { get 'invalid_authenticity_token' => 'anonymous#invalid_authenticity_token' }
-      get 'invalid_authenticity_token'
-    end
-
-    it_behaves_like 'error response', 422
-  end
-
   describe 'before_action :check_suspension' do
     before do
       routes.draw { get 'success' => 'anonymous#success' }
@@ -141,64 +83,4 @@ RSpec.describe ApplicationController do
       expect { controller.raise_not_found }.to raise_error(ActionController::RoutingError, 'No route matches unmatched')
     end
   end
-
-  describe 'forbidden' do
-    controller do
-      def route_forbidden
-        forbidden
-      end
-    end
-
-    subject do
-      routes.draw { get 'route_forbidden' => 'anonymous#route_forbidden' }
-      get 'route_forbidden'
-    end
-
-    it_behaves_like 'error response', 403
-  end
-
-  describe 'not_found' do
-    controller do
-      def route_not_found
-        not_found
-      end
-    end
-
-    subject do
-      routes.draw { get 'route_not_found' => 'anonymous#route_not_found' }
-      get 'route_not_found'
-    end
-
-    it_behaves_like 'error response', 404
-  end
-
-  describe 'gone' do
-    controller do
-      def route_gone
-        gone
-      end
-    end
-
-    subject do
-      routes.draw { get 'route_gone' => 'anonymous#route_gone' }
-      get 'route_gone'
-    end
-
-    it_behaves_like 'error response', 410
-  end
-
-  describe 'unprocessable_content' do
-    controller do
-      def route_unprocessable_content
-        unprocessable_content
-      end
-    end
-
-    subject do
-      routes.draw { get 'route_unprocessable_content' => 'anonymous#route_unprocessable_content' }
-      get 'route_unprocessable_content'
-    end
-
-    it_behaves_like 'error response', 422
-  end
 end

spec/controllers/concerns/error_responses_spec.rb

@@ -0,0 +1,128 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe ErrorResponses do
+  render_views
+
+  shared_examples 'error response' do |code|
+    before { routes.draw { get 'show' => 'anonymous#show' } }
+
+    it "returns http #{code} and renders error template" do
+      get 'show'
+
+      expect(response)
+        .to have_http_status(code)
+      expect(response.parsed_body)
+        .to have_css('body[class=error]')
+        .and have_css('h1', text: error_content(code))
+    end
+
+    def error_content(code)
+      I18n.t("errors.#{code}")
+        .then { |value| I18n.t("errors.#{code}.content") if value.is_a?(Hash) }
+    end
+  end
+
+  describe 'bad_request' do
+    controller(ApplicationController) do
+      def show = bad_request
+    end
+
+    it_behaves_like 'error response', 400
+  end
+
+  describe 'forbidden' do
+    controller(ApplicationController) do
+      def show = forbidden
+    end
+
+    it_behaves_like 'error response', 403
+  end
+
+  describe 'gone' do
+    controller(ApplicationController) do
+      def show = gone
+    end
+
+    it_behaves_like 'error response', 410
+  end
+
+  describe 'internal_server_error' do
+    controller(ApplicationController) do
+      def show = internal_server_error
+    end
+
+    it_behaves_like 'error response', 500
+  end
+
+  describe 'not_acceptable' do
+    controller(ApplicationController) do
+      def show = not_acceptable
+    end
+
+    it_behaves_like 'error response', 406
+  end
+
+  describe 'not_found' do
+    controller(ApplicationController) do
+      def show = not_found
+    end
+
+    it_behaves_like 'error response', 404
+  end
+
+  describe 'service_unavailable' do
+    controller(ApplicationController) do
+      def show = service_unavailable
+    end
+
+    it_behaves_like 'error response', 503
+  end
+
+  describe 'too_many_requests' do
+    controller(ApplicationController) do
+      def show = too_many_requests
+    end
+
+    it_behaves_like 'error response', 429
+  end
+
+  describe 'unprocessable_content' do
+    controller(ApplicationController) do
+      def show = unprocessable_content
+    end
+
+    it_behaves_like 'error response', 422
+  end
+
+  context 'with ActionController::RoutingError' do
+    controller(ApplicationController) do
+      def show
+        raise ActionController::RoutingError, ''
+      end
+    end
+
+    it_behaves_like 'error response', 404
+  end
+
+  context 'with ActiveRecord::RecordNotFound' do
+    controller(ApplicationController) do
+      def show
+        raise ActiveRecord::RecordNotFound, ''
+      end
+    end
+
+    it_behaves_like 'error response', 404
+  end
+
+  context 'with ActionController::InvalidAuthenticityToken' do
+    controller(ApplicationController) do
+      def show
+        raise ActionController::InvalidAuthenticityToken, ''
+      end
+    end
+
+    it_behaves_like 'error response', 422
+  end
+end