From 4cc8dd41eca45e79ad107dc554bfdfdddbdce958 Mon Sep 17 00:00:00 2001 From: evazion Date: Sun, 12 Sep 2021 07:01:35 -0500 Subject: [PATCH] puma: add rack-timeout gem. Unlike Unicorn, Puma doesn't have a builtin HTTP request timeout mechanism, so we have to use Rack::Timeout instead. See the caveats in the Rack::Timeout documentation [1]. In Unicorn, a timeout would send a SIGKILL to the worker, immediately killing it. This would result in a dropped connection and a Cloudflare 502 error to the user. In Puma, it raises an exception, which we can catch and return a better error to the user. On the other hand, raising an exception can potentially corrupt application state if it's sent at the wrong time, or be delayed indefinitely if the app is stuck in IO or C extension code. The default request timeout is 65 seconds. 65 seconds is to give things like HTTP requests on a 60 second timeout enough time to complete. Set the RACK_REQUEST_TIMEOUT environment variable to change the timeout. 1: https://github.com/sharpstone/rack-timeout#further-documentation --- Gemfile | 1 + Gemfile.lock | 2 ++ app/controllers/application_controller.rb | 2 ++ config/initializers/rack_timeout.rb | 13 +++++++++++++ 4 files changed, 18 insertions(+) create mode 100644 config/initializers/rack_timeout.rb diff --git a/Gemfile b/Gemfile index 0ecf45756..cb2ef8986 100644 --- a/Gemfile +++ b/Gemfile @@ -55,6 +55,7 @@ gem 'newrelic_rpm', require: false gem 'clockwork' gem 'puma-metrics' gem 'puma_worker_killer' +gem "rack-timeout", require: "rack/timeout/base" group :production, :staging do gem 'unicorn', :platforms => :ruby diff --git a/Gemfile.lock b/Gemfile.lock index d5fd2aab6..453514843 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -344,6 +344,7 @@ GEM rack rack-test (1.1.0) rack (>= 1.0, < 3) + rack-timeout (0.6.0) rails (6.1.4.1) actioncable (= 6.1.4.1) actionmailbox (= 6.1.4.1) @@ -568,6 +569,7 @@ DEPENDENCIES puma_worker_killer pundit rack-mini-profiler + rack-timeout rails (~> 6.0) rake rakismet diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 13bf7f557..22273a652 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -106,6 +106,8 @@ class ApplicationController < ActionController::Base render_error_page(422, exception, template: "static/tag_limit_error", message: "You cannot search for more than #{CurrentUser.tag_query_limit} tags at a time.") when RateLimiter::RateLimitError render_error_page(429, exception) + when Rack::Timeout::RequestTimeoutException + render_error_page(500, exception, message: "Your request took too long to complete and was canceled.") when NotImplementedError render_error_page(501, exception, message: "This feature isn't available: #{exception.message}") when PG::ConnectionBad diff --git a/config/initializers/rack_timeout.rb b/config/initializers/rack_timeout.rb new file mode 100644 index 000000000..e0116aebf --- /dev/null +++ b/config/initializers/rack_timeout.rb @@ -0,0 +1,13 @@ +# https://github.com/sharpstone/rack-timeout#configuring +options = { + service_timeout: ENV.fetch("RACK_REQUEST_TIMEOUT", 65).to_i, + wait_timeout: false, + wait_overtime: false, + service_past_wait: false, + term_on_timeout: false +} + +Rack::Timeout::Logger.logger = Rails.logger.dup +Rack::Timeout::Logger.logger.level = :error + +Rails.application.config.middleware.insert_before Rack::Runtime, Rack::Timeout, **options