FIX: Rescue and warn when error is encountered in DiscourseIpInfo.mmdb_download (#28134)

Since switching to Maxmind permalinks to download the databases in
7079698cdf, we have received multiple
reports about rebuilds failing as `maxminddb:refresh` runs during
the rebuilds and failing to download the databases cases the rebuilds to
fail.

Downloading Maxmind databases should not sit in the critical rebuild
path but since we are close to the Discourse 3.3 release, we have opted
to just rescue all errors encountered when downloading the databases.

In the near future after the Discourse 3.3 release, we will be looking
at moving the downloading of maxmind databases out of the rebuild path.
This commit is contained in:
Alan Guo Xiang Tan 2024-07-30 11:33:20 +08:00 committed by GitHub
parent 1f5cbb9a44
commit 3193afe7ca
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 40 additions and 21 deletions

View File

@ -55,7 +55,6 @@ class DiscourseIpInfo
end end
end end
begin
gz_file = gz_file =
FileHelper.download( FileHelper.download(
url, url,
@ -71,15 +70,12 @@ class DiscourseIpInfo
dir = "#{Dir.tmpdir}/#{SecureRandom.hex}" dir = "#{Dir.tmpdir}/#{SecureRandom.hex}"
Discourse::Utils.execute_command("mkdir", "-p", dir) Discourse::Utils.execute_command("mkdir", "-p", dir)
Discourse::Utils.execute_command("cp", gz_file.path, "#{dir}/#{filename}") Discourse::Utils.execute_command("cp", gz_file.path, "#{dir}/#{filename}")
Discourse::Utils.execute_command("tar", "-xzvf", "#{dir}/#{filename}", chdir: dir) Discourse::Utils.execute_command("tar", "-xzvf", "#{dir}/#{filename}", chdir: dir)
Dir["#{dir}/**/*.mmdb"].each { |f| FileUtils.mv(f, mmdb_path(name)) } Dir["#{dir}/**/*.mmdb"].each { |f| FileUtils.mv(f, mmdb_path(name)) }
rescue => e rescue => e
Discourse.warn_exception(e, message: "MaxMind database download failed.") Discourse.warn_exception(e, message: "MaxMind database #{name} download failed.")
end
ensure ensure
FileUtils.rm_r(dir, force: true) if dir FileUtils.rm_r(dir, force: true) if dir
gz_file&.close! gz_file&.close!

View File

@ -60,5 +60,28 @@ RSpec.describe DiscourseIpInfo do
described_class.mmdb_download("GeoLite2-City") described_class.mmdb_download("GeoLite2-City")
end end
it "should not throw an error and instead log the exception when database file fails to download" do
original_logger = Rails.logger
Rails.logger = fake_logger = FakeLogger.new
global_setting :maxmind_license_key, "license_key"
global_setting :maxmind_account_id, "account_id"
stub_request(
:get,
"https://download.maxmind.com/geoip/databases/GeoLite2-City/download?suffix=tar.gz",
).with(basic_auth: %w[account_id license_key]).to_return(status: 500, body: nil, headers: {})
expect do described_class.mmdb_download("GeoLite2-City") end.not_to raise_error
expect(fake_logger.warnings.length).to eq(1)
expect(fake_logger.warnings.first).to include(
"MaxMind database GeoLite2-City download failed. 500 Error",
)
ensure
Rails.logger = original_logger
end
end end
end end