From d2f44d4ce6c247981c550944267891579e76ee40 Mon Sep 17 00:00:00 2001 From: Mason Ballengee Date: Fri, 19 Apr 2024 13:08:20 -0400 Subject: [PATCH 1/3] Add option to rescue Solr/Fedora connection errors This commit uses a static error page for the Solr connection errors. The nav bar header rendered as part of the regular Avalon layout checks user permissions, which directly calls out to Solr. This triggers an error that is not caught by rescue methods in application_controller. Because the nav bar is rendered on almost every page of the application, this results in most of the application being inaccessible. As such, serving a static error page/message should be sufficient. Using the Rails `config.action_dispatch.rescue_response` handling pathway was not logging the proper error in testing and did not prevent other partials from attempting to render which resulted in extra errors and noise being present in the logs. Serving this file without the Avalon layout and logging the Solr error as part of a standard `rescue_from` in the application_controller feels more in line with our needs. --- app/controllers/application_controller.rb | 27 +++++++ app/jobs/application_job.rb | 11 +++ app/views/errors/fedora_connection.html.erb | 22 ++++++ app/views/errors/solr_connection.html.erb | 70 +++++++++++++++++++ config/settings.yml | 2 + .../application_controller_spec.rb | 42 +++++++++++ spec/jobs/application_job_spec.rb | 33 +++++++++ 7 files changed, 207 insertions(+) create mode 100644 app/views/errors/fedora_connection.html.erb create mode 100644 app/views/errors/solr_connection.html.erb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 2dd6966e8b..6bd5f56676 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -34,6 +34,11 @@ class ApplicationController < ActionController::Base before_action :set_no_cache_headers, if: proc{|c| request.xhr? } prepend_before_action :remove_zero_width_chars + rescue_from RSolr::Error::ConnectionRefused, :with => :handle_solr_connection_error + rescue_from RSolr::Error::Timeout, :with => :handle_solr_connection_error + rescue_from Blacklight::Exceptions::ECONNREFUSED, :with => :handle_solr_connection_error + rescue_from Faraday::ConnectionFailed, :with => :handle_fedora_connection_error + def set_no_cache_headers response.headers["Cache-Control"] = "no-cache, no-store" response.headers["Pragma"] = "no-cache" @@ -245,4 +250,26 @@ def strip_zero_width_chars!(obj) def should_store_return_url? !(request.xhr? || request.format != "html" || request.path.start_with?("/users/") || request.path.end_with?("poster") || request.path.end_with?("thumbnail")) end + + def handle_solr_connection_error(exception) + raise if Settings.solr_and_fedora.raise_on_connection_error + Rails.logger.error(exception.class.to_s + ': ' + exception.message + '\n' + exception.backtrace.join('\n')) + + if request.format == :json + render json: {errors: [exception.message]}, status: 503 + else + render '/errors/solr_connection', layout: false, status: 503 + end + end + + def handle_fedora_connection_error(exception) + raise if Settings.solr_and_fedora.raise_on_connection_error + Rails.logger.error(exception.class.to_s + ': ' + exception.message + '\n' + exception.backtrace.join('\n')) + + if request.format == :json + render json: {errors: [exception.message]}, status: 503 + else + render '/errors/fedora_connection', status: 503 + end + end end diff --git a/app/jobs/application_job.rb b/app/jobs/application_job.rb index ab3523a35d..6175bd1f44 100644 --- a/app/jobs/application_job.rb +++ b/app/jobs/application_job.rb @@ -13,7 +13,18 @@ # --- END LICENSE_HEADER BLOCK --- class ApplicationJob < ActiveJob::Base + rescue_from RSolr::Error::ConnectionRefused, :with => :handle_connection_error + rescue_from RSolr::Error::Timeout, :with => :handle_connection_error + rescue_from Blacklight::Exceptions::ECONNREFUSED, :with => :handle_connection_error + rescue_from Faraday::ConnectionFailed, :with => :handle_connection_error + rescue_from Ldp::Gone do |exception| Rails.logger.error(exception.message + '\n' + exception.backtrace.join('\n')) end + + private + def handle_connection_error(exception) + raise if Settings.solr_and_fedora.raise_on_connection_error + Rails.logger.error(exception.class.to_s + ': ' + exception.message + '\n' + exception.backtrace.join('\n')) + end end diff --git a/app/views/errors/fedora_connection.html.erb b/app/views/errors/fedora_connection.html.erb new file mode 100644 index 0000000000..43a1944828 --- /dev/null +++ b/app/views/errors/fedora_connection.html.erb @@ -0,0 +1,22 @@ +<%# +Copyright 2011-2024, The Trustees of Indiana University and Northwestern + University. Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software distributed + under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR + CONDITIONS OF ANY KIND, either express or implied. See the License for the + specific language governing permissions and limitations under the License. +--- END LICENSE_HEADER BLOCK --- +%> +
+
+

Fedora Connection Error

+

The application was unable to connect to the Fedora database. Please try again in a few minutes.

+

If the problem persists contact your support staff.

+
+
\ No newline at end of file diff --git a/app/views/errors/solr_connection.html.erb b/app/views/errors/solr_connection.html.erb new file mode 100644 index 0000000000..f4c9475730 --- /dev/null +++ b/app/views/errors/solr_connection.html.erb @@ -0,0 +1,70 @@ + + + + Solr Connection Error (503) + + + + + +
+
+

Solr Connection Error

+
+

+ The application was unable to connect to the Solr database. +
Please try again in a few minutes. +
+
If the problem persists contact your support staff. +

+
+ + \ No newline at end of file diff --git a/config/settings.yml b/config/settings.yml index 1b0fd6e7bd..2cd7bc3c9f 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -44,6 +44,8 @@ solr: rule: shards: snitch: +solr_and_fedora: + raise_on_connection_error: true zookeeper: connection_str: "localhost:9983/configs" streaming: diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 87385656a5..6989424f44 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -80,6 +80,48 @@ def show get :show, params: { id: 'deleted-id' } expect(response).to render_template("errors/deleted_pid") end + + context 'raise_on_connection_error disabled' do + let(:request_context) { { body: "request_context" } } + let(:e) { { body: "error_response" } } + + [RSolr::Error::ConnectionRefused, RSolr::Error::Timeout, Blacklight::Exceptions::ECONNREFUSED, Faraday::ConnectionFailed].each do |error_code| + it "rescues #{error_code} errors" do + raised_error = error_code == RSolr::Error::Timeout ? error_code.new(request_context, e) : error_code + allow(Settings.solr_and_fedora).to receive(:raise_on_connection_error).and_return(false) + allow(controller).to receive(:show).and_raise(raised_error) + allow_any_instance_of(Exception).to receive(:backtrace).and_return(["Test trace"]) + allow_any_instance_of(Exception).to receive(:message).and_return('Connection reset by peer') + expect(Rails.logger).to receive(:error).with(error_code.to_s + ': Connection reset by peer\nTest trace') + expect { get :show, params: { id: 'abc1234' } }.to_not raise_error + end + + it "renders error template for #{error_code} errors" do + error_template = error_code == Faraday::ConnectionFailed ? 'errors/fedora_connection' : 'errors/solr_connection' + raised_error = error_code == RSolr::Error::Timeout ? error_code.new(request_context, e) : error_code + allow(Settings.solr_and_fedora).to receive(:raise_on_connection_error).and_return(false) + allow(controller).to receive(:show).and_raise(raised_error) + get :show, params: { id: 'abc1234' } + expect(response).to render_template(error_template) + end + end + end + + context 'raise_on_connection_error enabled' do + let(:request_context) { { body: "request_context" } } + let(:e) { { body: "error_response" } } + + [RSolr::Error::ConnectionRefused, RSolr::Error::Timeout, Blacklight::Exceptions::ECONNREFUSED, Faraday::ConnectionFailed].each do |error_code| + it "raises #{error_code} errors" do + raised_error = error_code == RSolr::Error::Timeout ? error_code.new(request_context, e) : error_code + allow(Settings.solr_and_fedora).to receive(:raise_on_connection_error).and_return(true) + allow(controller).to receive(:show).and_raise(raised_error) + allow_any_instance_of(Exception).to receive(:backtrace).and_return(["Test trace"]) + allow_any_instance_of(Exception).to receive(:message).and_return('Connection reset by peer') + expect { get :show, params: { id: 'abc1234' } }.to raise_error(error_code, 'Connection reset by peer') + end + end + end end describe "rewrite_v4_ids" do diff --git a/spec/jobs/application_job_spec.rb b/spec/jobs/application_job_spec.rb index 0a4178e373..183aafe623 100644 --- a/spec/jobs/application_job_spec.rb +++ b/spec/jobs/application_job_spec.rb @@ -22,5 +22,38 @@ expect(Rails.logger).to receive(:error).with('Ldp::Gone\nTest trace') expect { described_class.perform_now }.to_not raise_error end + + context 'raise_on_connection_error disabled' do + let(:request_context) { { body: "request_context" } } + let(:e) { { body: "error_response" } } + + [RSolr::Error::ConnectionRefused, RSolr::Error::Timeout, Blacklight::Exceptions::ECONNREFUSED, Faraday::ConnectionFailed].each do |error_code| + it "rescues #{error_code} errors" do + raised_error = error_code == RSolr::Error::Timeout ? error_code.new(request_context, e) : error_code + allow(Settings.solr_and_fedora).to receive(:raise_on_connection_error).and_return(false) + allow_any_instance_of(described_class).to receive(:perform).and_raise(raised_error) + allow_any_instance_of(Exception).to receive(:backtrace).and_return(["Test trace"]) + allow_any_instance_of(Exception).to receive(:message).and_return('Connection reset by peer') + expect(Rails.logger).to receive(:error).with(error_code.to_s + ': Connection reset by peer\nTest trace') + expect { described_class.perform_now }.to_not raise_error + end + end + end + + context 'raise_on_connection_error enabled' do + let(:request_context) { { body: "request_context" } } + let(:e) { { body: "error_response" } } + + [RSolr::Error::ConnectionRefused, RSolr::Error::Timeout, Blacklight::Exceptions::ECONNREFUSED, Faraday::ConnectionFailed].each do |error_code| + it "raises #{error_code} errors" do + raised_error = error_code == RSolr::Error::Timeout ? error_code.new(request_context, e) : error_code + allow(Settings.solr_and_fedora).to receive(:raise_on_connection_error).and_return(true) + allow_any_instance_of(described_class).to receive(:perform).and_raise(raised_error) + allow_any_instance_of(Exception).to receive(:backtrace).and_return(["Test trace"]) + allow_any_instance_of(Exception).to receive(:message).and_return('Connection reset by peer') + expect { described_class.perform_now }.to raise_error(error_code, 'Connection reset by peer') + end + end + end end end From d6552a5e89941bc92d7c9272408a344b97be7175 Mon Sep 17 00:00:00 2001 From: Mason Ballengee Date: Fri, 3 May 2024 16:51:47 -0400 Subject: [PATCH 2/3] Provide separate options for controller and job error raising --- app/controllers/application_controller.rb | 4 ++-- app/jobs/application_job.rb | 2 +- config/settings.yml | 8 ++++++-- spec/controllers/application_controller_spec.rb | 10 ++++++---- spec/jobs/application_job_spec.rb | 4 ++-- 5 files changed, 17 insertions(+), 11 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 6bd5f56676..387711224b 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -252,7 +252,7 @@ def should_store_return_url? end def handle_solr_connection_error(exception) - raise if Settings.solr_and_fedora.raise_on_connection_error + raise if Settings.app_controller.solr_and_fedora.raise_on_connection_error Rails.logger.error(exception.class.to_s + ': ' + exception.message + '\n' + exception.backtrace.join('\n')) if request.format == :json @@ -263,7 +263,7 @@ def handle_solr_connection_error(exception) end def handle_fedora_connection_error(exception) - raise if Settings.solr_and_fedora.raise_on_connection_error + raise if Settings.app_controller.solr_and_fedora.raise_on_connection_error Rails.logger.error(exception.class.to_s + ': ' + exception.message + '\n' + exception.backtrace.join('\n')) if request.format == :json diff --git a/app/jobs/application_job.rb b/app/jobs/application_job.rb index 6175bd1f44..5e3b850885 100644 --- a/app/jobs/application_job.rb +++ b/app/jobs/application_job.rb @@ -24,7 +24,7 @@ class ApplicationJob < ActiveJob::Base private def handle_connection_error(exception) - raise if Settings.solr_and_fedora.raise_on_connection_error + raise if Settings.app_job.solr_and_fedora.raise_on_connection_error Rails.logger.error(exception.class.to_s + ': ' + exception.message + '\n' + exception.backtrace.join('\n')) end end diff --git a/config/settings.yml b/config/settings.yml index 2cd7bc3c9f..a26318f7e7 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -44,8 +44,12 @@ solr: rule: shards: snitch: -solr_and_fedora: - raise_on_connection_error: true +app_controller: + solr_and_fedora: + raise_on_connection_error: true +app_job: + solr_and_fedora: + raise_on_connection_error: true zookeeper: connection_str: "localhost:9983/configs" streaming: diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 6989424f44..b312d1e4b0 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -85,10 +85,13 @@ def show let(:request_context) { { body: "request_context" } } let(:e) { { body: "error_response" } } + before :each do + allow(Settings.app_controller.solr_and_fedora).to receive(:raise_on_connection_error).and_return(false) + end + [RSolr::Error::ConnectionRefused, RSolr::Error::Timeout, Blacklight::Exceptions::ECONNREFUSED, Faraday::ConnectionFailed].each do |error_code| it "rescues #{error_code} errors" do - raised_error = error_code == RSolr::Error::Timeout ? error_code.new(request_context, e) : error_code - allow(Settings.solr_and_fedora).to receive(:raise_on_connection_error).and_return(false) + raised_error = error_code == RSolr::Error::Timeout ? error_code.new(request_context, e) : error_code allow(controller).to receive(:show).and_raise(raised_error) allow_any_instance_of(Exception).to receive(:backtrace).and_return(["Test trace"]) allow_any_instance_of(Exception).to receive(:message).and_return('Connection reset by peer') @@ -99,7 +102,6 @@ def show it "renders error template for #{error_code} errors" do error_template = error_code == Faraday::ConnectionFailed ? 'errors/fedora_connection' : 'errors/solr_connection' raised_error = error_code == RSolr::Error::Timeout ? error_code.new(request_context, e) : error_code - allow(Settings.solr_and_fedora).to receive(:raise_on_connection_error).and_return(false) allow(controller).to receive(:show).and_raise(raised_error) get :show, params: { id: 'abc1234' } expect(response).to render_template(error_template) @@ -114,7 +116,7 @@ def show [RSolr::Error::ConnectionRefused, RSolr::Error::Timeout, Blacklight::Exceptions::ECONNREFUSED, Faraday::ConnectionFailed].each do |error_code| it "raises #{error_code} errors" do raised_error = error_code == RSolr::Error::Timeout ? error_code.new(request_context, e) : error_code - allow(Settings.solr_and_fedora).to receive(:raise_on_connection_error).and_return(true) + allow(Settings.app_controller.solr_and_fedora).to receive(:raise_on_connection_error).and_return(true) allow(controller).to receive(:show).and_raise(raised_error) allow_any_instance_of(Exception).to receive(:backtrace).and_return(["Test trace"]) allow_any_instance_of(Exception).to receive(:message).and_return('Connection reset by peer') diff --git a/spec/jobs/application_job_spec.rb b/spec/jobs/application_job_spec.rb index 183aafe623..687415db33 100644 --- a/spec/jobs/application_job_spec.rb +++ b/spec/jobs/application_job_spec.rb @@ -30,7 +30,7 @@ [RSolr::Error::ConnectionRefused, RSolr::Error::Timeout, Blacklight::Exceptions::ECONNREFUSED, Faraday::ConnectionFailed].each do |error_code| it "rescues #{error_code} errors" do raised_error = error_code == RSolr::Error::Timeout ? error_code.new(request_context, e) : error_code - allow(Settings.solr_and_fedora).to receive(:raise_on_connection_error).and_return(false) + allow(Settings.app_job.solr_and_fedora).to receive(:raise_on_connection_error).and_return(false) allow_any_instance_of(described_class).to receive(:perform).and_raise(raised_error) allow_any_instance_of(Exception).to receive(:backtrace).and_return(["Test trace"]) allow_any_instance_of(Exception).to receive(:message).and_return('Connection reset by peer') @@ -47,7 +47,7 @@ [RSolr::Error::ConnectionRefused, RSolr::Error::Timeout, Blacklight::Exceptions::ECONNREFUSED, Faraday::ConnectionFailed].each do |error_code| it "raises #{error_code} errors" do raised_error = error_code == RSolr::Error::Timeout ? error_code.new(request_context, e) : error_code - allow(Settings.solr_and_fedora).to receive(:raise_on_connection_error).and_return(true) + allow(Settings.app_job.solr_and_fedora).to receive(:raise_on_connection_error).and_return(true) allow_any_instance_of(described_class).to receive(:perform).and_raise(raised_error) allow_any_instance_of(Exception).to receive(:backtrace).and_return(["Test trace"]) allow_any_instance_of(Exception).to receive(:message).and_return('Connection reset by peer') From 17609156b8895af56624efa8faa94a94eb659907 Mon Sep 17 00:00:00 2001 From: Mason Ballengee <68433277+masaball@users.noreply.github.com> Date: Wed, 8 May 2024 09:19:13 -0400 Subject: [PATCH 3/3] Default app_controller.raise_on_connection_error to false Co-authored-by: Chris Colvard --- config/settings.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/settings.yml b/config/settings.yml index a26318f7e7..315d171cda 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -46,7 +46,7 @@ solr: snitch: app_controller: solr_and_fedora: - raise_on_connection_error: true + raise_on_connection_error: false app_job: solr_and_fedora: raise_on_connection_error: true