diff --git a/.env.development b/.env.development index 8f352f7e5f3..15e09487f28 100644 --- a/.env.development +++ b/.env.development @@ -11,3 +11,9 @@ OFN_REDIS_URL="redis://localhost:6379/1" OFN_REDIS_JOBS_URL="redis://localhost:6379/2" SITE_URL="0.0.0.0:3000" + +# Deactivate rack-timeout in development. +# https://github.com/zombocom/rack-timeout#configuring +RACK_TIMEOUT_SERVICE_TIMEOUT="0" +RACK_TIMEOUT_WAIT_TIMEOUT="0" +RACK_TIMEOUT_WAIT_OVERTIME="0" diff --git a/Gemfile b/Gemfile index f3e2c8ca408..9b2e9ec0073 100644 --- a/Gemfile +++ b/Gemfile @@ -92,6 +92,7 @@ gem 'gmaps4rails' gem 'mimemagic', '> 0.3.5' gem 'paper_trail', '~> 12.1' gem 'rack-rewrite' +gem 'rack-timeout' gem 'roadie-rails' gem 'hiredis' @@ -141,7 +142,6 @@ gem "private_address_check" group :production, :staging do gem 'ddtrace' - gem 'rack-timeout' gem 'sd_notify' # For better Systemd process management. Used by Puma. end diff --git a/app/controllers/admin/reports_controller.rb b/app/controllers/admin/reports_controller.rb index 2dc18e40928..9e75900b8de 100644 --- a/app/controllers/admin/reports_controller.rb +++ b/app/controllers/admin/reports_controller.rb @@ -26,6 +26,8 @@ def show else show_report end + rescue Timeout::Error + render_timeout_error end private @@ -36,6 +38,7 @@ def export_report def show_report assign_view_data + @table = render_report_as(:html) if render_data? render "show" end @@ -45,7 +48,6 @@ def assign_view_data @report_subtype = report_subtype @report_title = report_title @rendering_options = rendering_options - @table = render_report_as(:html) if render_data? @data = Reporting::FrontendData.new(spree_current_user) end @@ -58,7 +60,9 @@ def render_report_as(format) job = ReportJob.perform_later( report_class, spree_current_user, params, format ) - sleep 1 until job.done? + Timeout.timeout(max_wait_time) do + sleep 1 until job.done? + end # This result has been rendered by Rails in safe mode already. job.result.html_safe # rubocop:disable Rails/OutputSafety @@ -66,5 +70,23 @@ def render_report_as(format) @report.render_as(format) end end + + def render_timeout_error + assign_view_data + @error = ".report_taking_longer" + render "show" + end + + def max_wait_time + # This value is used by rack-timeout and nginx, usually 30 seconds in + # staging and production: + server_timeout = ENV.fetch("RACK_TIMEOUT_SERVICE_TIMEOUT", "15").to_f + + # Zero disables the timeout: + return 0 if server_timeout.zero? + + # We want to time out earlier than nginx: + server_timeout - 2.seconds + end end end diff --git a/app/views/admin/reports/show.html.haml b/app/views/admin/reports/show.html.haml index 4c93fc9977d..0493dfd6406 100644 --- a/app/views/admin/reports/show.html.haml +++ b/app/views/admin/reports/show.html.haml @@ -18,7 +18,8 @@ .report__header.print-hidden - if @report.message.present? %p.report__message= @report.message - - if request.post? + - if request.post? && !@error %button.btn-print.icon-print{ onclick: "window.print()"}= t(:report_print) += t(@error) if @error = @table diff --git a/config/locales/en.yml b/config/locales/en.yml index eb520908a0b..ba116e5e54c 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1438,6 +1438,11 @@ en: pack_by_customer: Pack By Customer pack_by_supplier: Pack By Supplier pack_by_product: Pack By Product + show: + report_taking_longer: > + Sorry, this report took too long to process. + It may contain a lot of data or we are busy with other reports. + You can try again later. revenues_by_hub: name: Revenues By Hub description: Revenues by hub diff --git a/spec/system/admin/reports_spec.rb b/spec/system/admin/reports_spec.rb index 33a9cae1528..0e255bbdb1e 100644 --- a/spec/system/admin/reports_spec.rb +++ b/spec/system/admin/reports_spec.rb @@ -44,6 +44,19 @@ click_button "Go" expect(page).to have_content "EMAIL FIRST NAME" end + + it "displays a friendly timeout message" do + ActiveJob::Base.queue_adapter.perform_enqueued_jobs = false + login_as_admin_and_visit admin_report_path( + report_type: :customers, report_subtype: :mailing_list + ) + expect(ENV).to receive(:fetch).with("RACK_TIMEOUT_SERVICE_TIMEOUT", "15") + .and_return("-1") # Negative values time out immediately. + + click_button "Go" + + expect(page).to have_content "this report took too long" + end end describe "Can access Customers reports and generate customers report" do