From f9100743ecec65cdeeabdaa29ed94afc4b2e3aa5 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Thu, 14 Mar 2024 05:09:47 -0400 Subject: [PATCH] Add `Api::ErrorHandling` concern for api/base controller (#29574) --- app/controllers/api/base_controller.rb | 46 +--------------- .../concerns/api/error_handling.rb | 52 +++++++++++++++++++ spec/controllers/api/base_controller_spec.rb | 36 ------------- .../concerns/api/error_handling_spec.rb | 51 ++++++++++++++++++ 4 files changed, 104 insertions(+), 81 deletions(-) create mode 100644 app/controllers/concerns/api/error_handling.rb create mode 100644 spec/controllers/concerns/api/error_handling_spec.rb diff --git a/app/controllers/api/base_controller.rb b/app/controllers/api/base_controller.rb index 8bf9da2cf..bf7deac5c 100644 --- a/app/controllers/api/base_controller.rb +++ b/app/controllers/api/base_controller.rb @@ -8,6 +8,7 @@ class Api::BaseController < ApplicationController include Api::AccessTokenTrackingConcern include Api::CachingConcern include Api::ContentSecurityPolicy + include Api::ErrorHandling skip_before_action :require_functional!, unless: :limited_federation_mode? @@ -18,51 +19,6 @@ class Api::BaseController < ApplicationController protect_from_forgery with: :null_session - rescue_from ActiveRecord::RecordInvalid, Mastodon::ValidationError do |e| - render json: { error: e.to_s }, status: 422 - end - - rescue_from ActiveRecord::RecordNotUnique do - render json: { error: 'Duplicate record' }, status: 422 - end - - rescue_from Date::Error do - render json: { error: 'Invalid date supplied' }, status: 422 - end - - rescue_from ActiveRecord::RecordNotFound do - render json: { error: 'Record not found' }, status: 404 - end - - rescue_from HTTP::Error, Mastodon::UnexpectedResponseError do - render json: { error: 'Remote data could not be fetched' }, status: 503 - end - - rescue_from OpenSSL::SSL::SSLError do - render json: { error: 'Remote SSL certificate could not be verified' }, status: 503 - end - - rescue_from Mastodon::NotPermittedError do - render json: { error: 'This action is not allowed' }, status: 403 - end - - rescue_from Seahorse::Client::NetworkingError do |e| - Rails.logger.warn "Storage server error: #{e}" - render json: { error: 'There was a temporary problem serving your request, please try again' }, status: 503 - end - - rescue_from Mastodon::RaceConditionError, Stoplight::Error::RedLight do - render json: { error: 'There was a temporary problem serving your request, please try again' }, status: 503 - end - - rescue_from Mastodon::RateLimitExceededError do - render json: { error: I18n.t('errors.429') }, status: 429 - end - - rescue_from ActionController::ParameterMissing, Mastodon::InvalidParameterError do |e| - render json: { error: e.to_s }, status: 400 - end - def doorkeeper_unauthorized_render_options(error: nil) { json: { error: error.try(:description) || 'Not authorized' } } end diff --git a/app/controllers/concerns/api/error_handling.rb b/app/controllers/concerns/api/error_handling.rb new file mode 100644 index 000000000..ad559fe2d --- /dev/null +++ b/app/controllers/concerns/api/error_handling.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +module Api::ErrorHandling + extend ActiveSupport::Concern + + included do + rescue_from ActiveRecord::RecordInvalid, Mastodon::ValidationError do |e| + render json: { error: e.to_s }, status: 422 + end + + rescue_from ActiveRecord::RecordNotUnique do + render json: { error: 'Duplicate record' }, status: 422 + end + + rescue_from Date::Error do + render json: { error: 'Invalid date supplied' }, status: 422 + end + + rescue_from ActiveRecord::RecordNotFound do + render json: { error: 'Record not found' }, status: 404 + end + + rescue_from HTTP::Error, Mastodon::UnexpectedResponseError do + render json: { error: 'Remote data could not be fetched' }, status: 503 + end + + rescue_from OpenSSL::SSL::SSLError do + render json: { error: 'Remote SSL certificate could not be verified' }, status: 503 + end + + rescue_from Mastodon::NotPermittedError do + render json: { error: 'This action is not allowed' }, status: 403 + end + + rescue_from Seahorse::Client::NetworkingError do |e| + Rails.logger.warn "Storage server error: #{e}" + render json: { error: 'There was a temporary problem serving your request, please try again' }, status: 503 + end + + rescue_from Mastodon::RaceConditionError, Stoplight::Error::RedLight do + render json: { error: 'There was a temporary problem serving your request, please try again' }, status: 503 + end + + rescue_from Mastodon::RateLimitExceededError do + render json: { error: I18n.t('errors.429') }, status: 429 + end + + rescue_from ActionController::ParameterMissing, Mastodon::InvalidParameterError do |e| + render json: { error: e.to_s }, status: 400 + end + end +end diff --git a/spec/controllers/api/base_controller_spec.rb b/spec/controllers/api/base_controller_spec.rb index f8e014be2..659d55f80 100644 --- a/spec/controllers/api/base_controller_spec.rb +++ b/spec/controllers/api/base_controller_spec.rb @@ -3,10 +3,6 @@ require 'rails_helper' describe Api::BaseController do - before do - stub_const('FakeService', Class.new) - end - controller do def success head 200 @@ -72,36 +68,4 @@ describe Api::BaseController do expect(response).to have_http_status(403) end end - - describe 'error handling' do - before do - routes.draw { get 'failure' => 'api/base#failure' } - end - - { - ActiveRecord::RecordInvalid => 422, - ActiveRecord::RecordNotFound => 404, - ActiveRecord::RecordNotUnique => 422, - Date::Error => 422, - HTTP::Error => 503, - Mastodon::InvalidParameterError => 400, - Mastodon::NotPermittedError => 403, - Mastodon::RaceConditionError => 503, - Mastodon::RateLimitExceededError => 429, - Mastodon::UnexpectedResponseError => 503, - Mastodon::ValidationError => 422, - OpenSSL::SSL::SSLError => 503, - Seahorse::Client::NetworkingError => 503, - Stoplight::Error::RedLight => 503, - }.each do |error, code| - it "Handles error class of #{error}" do - allow(FakeService).to receive(:new).and_raise(error) - - get :failure - - expect(response).to have_http_status(code) - expect(FakeService).to have_received(:new) - end - end - end end diff --git a/spec/controllers/concerns/api/error_handling_spec.rb b/spec/controllers/concerns/api/error_handling_spec.rb new file mode 100644 index 000000000..9b36fc20a --- /dev/null +++ b/spec/controllers/concerns/api/error_handling_spec.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe Api::ErrorHandling do + before do + stub_const('FakeService', Class.new) + end + + controller(Api::BaseController) do + def failure + FakeService.new + end + end + + describe 'error handling' do + before do + routes.draw { get 'failure' => 'api/base#failure' } + end + + { + ActiveRecord::RecordInvalid => 422, + ActiveRecord::RecordNotFound => 404, + ActiveRecord::RecordNotUnique => 422, + Date::Error => 422, + HTTP::Error => 503, + Mastodon::InvalidParameterError => 400, + Mastodon::NotPermittedError => 403, + Mastodon::RaceConditionError => 503, + Mastodon::RateLimitExceededError => 429, + Mastodon::UnexpectedResponseError => 503, + Mastodon::ValidationError => 422, + OpenSSL::SSL::SSLError => 503, + Seahorse::Client::NetworkingError => 503, + Stoplight::Error::RedLight => 503, + }.each do |error, code| + it "Handles error class of #{error}" do + allow(FakeService) + .to receive(:new) + .and_raise(error) + + get :failure + + expect(response) + .to have_http_status(code) + expect(FakeService) + .to have_received(:new) + end + end + end +end