diff --git a/app/controllers/errors_controller.rb b/app/controllers/errors_controller.rb new file mode 100644 index 000000000..394bbd89e --- /dev/null +++ b/app/controllers/errors_controller.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true +class ErrorsController < ApplicationController + skip_before_action :authenticate_user! + + def not_found + render(status: :not_found) + end + + def internal_server_error + render(status: :internal_server_error) + end +end diff --git a/app/controllers/works_controller.rb b/app/controllers/works_controller.rb index 9803fe013..fc504ea9d 100644 --- a/app/controllers/works_controller.rb +++ b/app/controllers/works_controller.rb @@ -164,8 +164,10 @@ def assign_curator render json: {} end rescue => ex + # This is necessary for JSON responses Rails.logger.error("Error changing curator for work: #{work.id}. Exception: #{ex.message}") - render json: { errors: ["Cannot save dataset"] }, status: :bad_request + Honeybadger.notify("Error changing curator for work: #{work.id}. Exception: #{ex.message}") + render(json: { errors: ["Cannot save dataset"] }, status: :bad_request) end def add_message @@ -314,10 +316,15 @@ def rescue_aasm_error if action_name == "create" handle_error_for_create(generic_error) else - redirect_to root_url, notice: "We apologize, an error was encountered: #{generic_error.message}. Please contact the PDC Describe administrators." + redirect_to error_url, notice: "We apologize, an error was encountered: #{generic_error.message}. Please contact the PDC Describe administrators." end end + rescue_from StandardError do |generic_error| + Honeybadger.notify("We apologize, an error was encountered: #{generic_error.message}.") + redirect_to error_url, notice: "We apologize, an error was encountered: #{generic_error.message}. Please contact the PDC Describe administrators." + end + # @note No testing coverage but not a route, not called def handle_error_for_create(generic_error) if @work.persisted? diff --git a/app/views/errors/internal_server_error.html.erb b/app/views/errors/internal_server_error.html.erb new file mode 100644 index 000000000..4ed53f95a --- /dev/null +++ b/app/views/errors/internal_server_error.html.erb @@ -0,0 +1,2 @@ +

There was an error encountered handling your request!

+

For assistance, please e-mail prds@princeton.edu for support.

diff --git a/app/views/errors/not_found.html.erb b/app/views/errors/not_found.html.erb new file mode 100644 index 000000000..b4703ae27 --- /dev/null +++ b/app/views/errors/not_found.html.erb @@ -0,0 +1,2 @@ +

The page you were looking for doesn't exist.

+

For assistance, please e-mail prds@princeton.edu for support.

diff --git a/config/application.rb b/config/application.rb index 3fec3d391..d706ad4b6 100644 --- a/config/application.rb +++ b/config/application.rb @@ -33,5 +33,7 @@ class Application < Rails::Application # Explicitly set timezome rather than relying on system, # which may be different in CI environment. config.time_zone = "America/New_York" + + config.exceptions_app = routes end end diff --git a/config/environments/development.rb b/config/environments/development.rb index 5232f6e48..7c6216c04 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -16,7 +16,7 @@ config.sass.cache = false # Show full error reports. - config.consider_all_requests_local = true + config.consider_all_requests_local = false # Enable/disable caching. By default caching is disabled. # Run rails dev:cache to toggle caching. diff --git a/config/environments/test.rb b/config/environments/test.rb index d15f17917..38c3f9a0c 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -29,7 +29,7 @@ } # Show full error reports and disable caching. - config.consider_all_requests_local = true + config.consider_all_requests_local = false config.action_controller.perform_caching = false config.cache_store = :null_store diff --git a/config/routes.rb b/config/routes.rb index ddaaa1f18..4a48f4f51 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -96,4 +96,7 @@ # Anything still unmatched by the end of the routes file should go to the not_found page # match '*a', to: redirect('/404'), via: :get + + match "/404", to: "errors#not_found", via: :all, as: :not_found + match "/500", to: "errors#internal_server_error", via: :all, as: :error end diff --git a/public/404.html b/public/404.html deleted file mode 100644 index 2be3af26f..000000000 --- a/public/404.html +++ /dev/null @@ -1,67 +0,0 @@ - - - - The page you were looking for doesn't exist (404) - - - - - - -
-
-

The page you were looking for doesn't exist.

-

You may have mistyped the address or the page may have moved.

-
-

If you are the application owner check the logs for more information.

-
- - diff --git a/public/500.html b/public/500.html deleted file mode 100644 index 78a030af2..000000000 --- a/public/500.html +++ /dev/null @@ -1,66 +0,0 @@ - - - - We're sorry, but something went wrong (500) - - - - - - -
-
-

We're sorry, but something went wrong.

-
-

If you are the application owner check the logs for more information.

-
- - diff --git a/spec/controllers/works_controller_spec.rb b/spec/controllers/works_controller_spec.rb index ad1dde88f..b557c4d4f 100644 --- a/spec/controllers/works_controller_spec.rb +++ b/spec/controllers/works_controller_spec.rb @@ -634,9 +634,9 @@ it "does not redirect to the Work show view if not exact (missing slash)" do stub_s3 - expect do - get :resolve_doi, params: { doi: work.doi.gsub("10.34770", "") } - end.to raise_error(ActiveRecord::RecordNotFound) + doi_param = work.doi.gsub("10.34770", "") + get(:resolve_doi, params: { doi: doi_param }) + expect(response).to redirect_to(error_url) end end end @@ -667,9 +667,8 @@ it "does not redirect to the Work show view if not exact (missing slash)" do stub_s3 - expect do - get :resolve_ark, params: { ark: work.ark.gsub("ark:", "") } - end.to raise_error(ActiveRecord::RecordNotFound) + get :resolve_ark, params: { ark: work.ark.gsub("ark:", "") } + expect(response).to redirect_to(error_url) end end end diff --git a/spec/requests/works_spec.rb b/spec/requests/works_spec.rb index c30ac44b5..b93d078c6 100644 --- a/spec/requests/works_spec.rb +++ b/spec/requests/works_spec.rb @@ -49,7 +49,8 @@ end it "will raise an error" do - expect { get work_url(work) }.to raise_error(Work::InvalidGroupError, "The Work test-id does not belong to any Group") + get work_url(work) + expect(response).to redirect_to(error_url) end end diff --git a/spec/system/root_spec.rb b/spec/system/root_spec.rb index b86c46d8e..b79458dda 100644 --- a/spec/system/root_spec.rb +++ b/spec/system/root_spec.rb @@ -8,4 +8,31 @@ expect(page).to have_content "Welcome" end + + context "when requesting a URL which does not exist", type: :system do + it "renders the custom 404 page" do + visit "/invalid" + + expect(page.status_code).to eq(404) + expect(page).to have_content "The page you were looking for doesn't exist." + end + end + + context "when an error occurs while requesting a URL", type: :system do + let(:user) { FactoryBot.create(:super_admin_user) } + let(:work) { FactoryBot.create(:draft_work) } + + before do + sign_in(user) + work + allow(Work).to receive(:find).and_raise(StandardError, "test") + end + + it "renders the custom 500 page" do + visit "/works/#{work.id}" + + expect(page.status_code).to eq(500) + expect(page).to have_content("We apologize, an error was encountered: test") + end + end end