DEV: Block all incoming requests before resetting Capybara session (#25692)

Why this change?

We have been debugging flaky system tests and noticed in https://github.com/discourse/discourse/actions/runs/7911902047/job/21596791343?pr=25690
that ActiveRecord connection checkout timeouts are encountered because
the Capybara server thread is processing requests even after
`Capybara.reset_session!` and ActiveRecord's `teardown_fixtures` have already been call.
The theory here is that an inflight request can still hit the Capybara
server even after `Capybara.reset_session!` has been called and end up
eating up an ActiveRecord connection for too long and also messing with
the database outside of a transaction.

What does this change do?

This change adds a `BlockRequestsMiddleware` middleware in the test
environment which is enabled to reject all incoming requests at the end
of each system test and before `Capybara.reset_session!` is called. At
the start of each RSpec test, the middleware is disabled again.
This commit is contained in:
Alan Guo Xiang Tan 2024-02-15 16:36:12 +08:00 committed by GitHub
parent 4346abe260
commit c30aeafd9d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 32 additions and 0 deletions

View File

@ -28,8 +28,35 @@ if Rails.env.test?
super(env) super(env)
end end
end end
Rails.configuration.middleware.unshift TestMultisiteMiddleware, Rails.configuration.middleware.unshift TestMultisiteMiddleware,
RailsMultisite::DiscoursePatches.config RailsMultisite::DiscoursePatches.config
class BlockRequestsMiddleware
@@block_requests = false
def self.block_requests!
@@block_requests = true
end
def self.allow_requests!
@@block_requests = false
end
def initialize(app)
@app = app
end
def call(env)
if @@block_requests
[503, { "Content-Type" => "text/plain" }, ["Blocked by BlockRequestsMiddleware"]]
else
@app.call(env)
end
end
end
Rails.configuration.middleware.unshift BlockRequestsMiddleware
elsif Rails.configuration.multisite elsif Rails.configuration.multisite
assets_hostnames = GlobalSetting.cdn_hostnames assets_hostnames = GlobalSetting.cdn_hostnames

View File

@ -608,6 +608,7 @@ RSpec.configure do |config|
driven_by driver.join("_").to_sym driven_by driver.join("_").to_sym
setup_system_test setup_system_test
BlockRequestsMiddleware.allow_requests!
end end
config.after(:each, type: :system) do |example| config.after(:each, type: :system) do |example|
@ -663,6 +664,10 @@ RSpec.configure do |config|
end end
page.execute_script("if (typeof MessageBus !== 'undefined') { MessageBus.stop(); }") page.execute_script("if (typeof MessageBus !== 'undefined') { MessageBus.stop(); }")
# Block all incoming requests before resetting Capybara session which will wait for all requests to finish
BlockRequestsMiddleware.block_requests!
Capybara.reset_session! Capybara.reset_session!
MessageBus.backend_instance.reset! # Clears all existing backlog from memory backend MessageBus.backend_instance.reset! # Clears all existing backlog from memory backend
Discourse.redis.flushdb Discourse.redis.flushdb