diff --git a/app/controllers/test_requests_controller.rb b/app/controllers/test_requests_controller.rb new file mode 100644 index 00000000000..b8d8d755c90 --- /dev/null +++ b/app/controllers/test_requests_controller.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +# This controller's actions are only available in the test environment to help with more complex testing requirements +class TestRequestsController < ApplicationController + if Rails.env.test? + def test_net_http_timeouts + net_http = Net::HTTP.new("example.com") + + render json: { + open_timeout: net_http.open_timeout, + read_timeout: net_http.read_timeout, + write_timeout: net_http.write_timeout, + max_retries: net_http.max_retries, + } + end + end +end diff --git a/config/initializers/200-first_middlewares.rb b/config/initializers/200-first_middlewares.rb index 60073484b7e..329704e69c0 100644 --- a/config/initializers/200-first_middlewares.rb +++ b/config/initializers/200-first_middlewares.rb @@ -7,6 +7,8 @@ # We aren't manipulating the middleware stack directly because of # https://github.com/rails/rails/pull/27936 +require "middleware/processing_request" +Rails.configuration.middleware.unshift(Middleware::ProcessingRequest) Rails.configuration.middleware.unshift(MessageBus::Rack::Middleware) # no reason to track this in development, that is 300+ redis calls saved per diff --git a/config/routes.rb b/config/routes.rb index c2e2c47365e..f20a2394957 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1649,5 +1649,10 @@ Discourse::Application.routes.draw do get "/form-templates/:id" => "form_templates#show" get "/form-templates" => "form_templates#index" + + if Rails.env.test? + # Routes that are only used for testing + get "/test_net_http_timeouts" => "test_requests#test_net_http_timeouts" + end end end diff --git a/lib/freedom_patches/net_http.rb b/lib/freedom_patches/net_http.rb new file mode 100644 index 00000000000..9e1df1aa8c2 --- /dev/null +++ b/lib/freedom_patches/net_http.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +module NetHTTPPatch + OPEN_TIMEOUT = 5 + READ_TIMEOUT = 10 + WRITE_TIMEOUT = 5 + + # By default Net::HTTP will retry 1 time on idempotent requests but we can't afford that while processing a request + # so setting it to 0 + MAX_RETIRES = 0 + + def initialize(*args, &block) + super(*args, &block) + + ## START PATCH + if Thread.current[Middleware::ProcessingRequest::PROCESSING_REQUEST_THREAD_KEY] + self.open_timeout = OPEN_TIMEOUT + self.read_timeout = READ_TIMEOUT + self.write_timeout = WRITE_TIMEOUT + self.max_retries = 0 + end + ## END PATCH + end +end + +Net::HTTP.prepend(NetHTTPPatch) diff --git a/lib/middleware/processing_request.rb b/lib/middleware/processing_request.rb new file mode 100644 index 00000000000..fda6195bab1 --- /dev/null +++ b/lib/middleware/processing_request.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class Middleware::ProcessingRequest + PROCESSING_REQUEST_THREAD_KEY = "discourse.processing_request" + + def initialize(app) + @app = app + end + + def call(env) + Thread.current[PROCESSING_REQUEST_THREAD_KEY] = true + @app.call(env) + ensure + Thread.current[PROCESSING_REQUEST_THREAD_KEY] = nil + end +end diff --git a/spec/integrity/middleware_order_spec.rb b/spec/integrity/middleware_order_spec.rb index 9d7565e0303..1ae0e167f9b 100644 --- a/spec/integrity/middleware_order_spec.rb +++ b/spec/integrity/middleware_order_spec.rb @@ -8,6 +8,7 @@ RSpec.describe "Middleware order" do ActionDispatch::RemoteIp, Middleware::RequestTracker, MessageBus::Rack::Middleware, + Middleware::ProcessingRequest, Rack::Sendfile, ActionDispatch::Static, ActionDispatch::Executor, diff --git a/spec/requests/net_http_timeout_spec.rb b/spec/requests/net_http_timeout_spec.rb new file mode 100644 index 00000000000..aeace6e501d --- /dev/null +++ b/spec/requests/net_http_timeout_spec.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +RSpec.describe "Net::HTTP timeouts when processing a request" do + it "should set the right timeouts for any `Net::HTTP` instances intialized while processing a request" do + stub_const(NetHTTPPatch, :OPEN_TIMEOUT, 0.001) do + stub_const(NetHTTPPatch, :READ_TIMEOUT, 0.002) do + stub_const(NetHTTPPatch, :WRITE_TIMEOUT, 0.003) do + get "/test_net_http_timeouts.json" + + parsed = response.parsed_body + + expect(parsed["open_timeout"]).to eq(0.001) + expect(parsed["read_timeout"]).to eq(0.002) + expect(parsed["write_timeout"]).to eq(0.003) + expect(parsed["max_retries"]).to eq(0) + end + end + end + end +end