Implements custom 404 and 500 pages for handling custom 404 and error…

… pages (#1958)

Co-authored-by: Bess Sadler <[email protected]>

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

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?

app/views/errors/internal_server_error.html.erb

@@ -0,0 +1,2 @@
+<h1 class="welcome-headers">There was an error encountered handling your request!</h1>
+<p>For assistance, please e-mail <a href="mailto:[email protected]">[email protected]</a> for support.</p>

app/views/errors/not_found.html.erb

@@ -0,0 +1,2 @@
+<h1 class="welcome-headers">The page you were looking for doesn't exist.</h1>
+<p>For assistance, please e-mail <a href="mailto:[email protected]">[email protected]</a> for support.</p>

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

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.

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
 

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

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

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
 

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