From 3193afe7ca8f95e907b1513dbcd08a5745f42d4d Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Tue, 30 Jul 2024 11:33:20 +0800 Subject: [PATCH] FIX: Rescue and warn when error is encountered in `DiscourseIpInfo.mmdb_download` (#28134) Since switching to Maxmind permalinks to download the databases in 7079698cdfb6f0cd73cdfd305b15350634fcd56a, 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. --- lib/discourse_ip_info.rb | 38 +++++++++++++----------------- spec/lib/discourse_ip_info_spec.rb | 23 ++++++++++++++++++ 2 files changed, 40 insertions(+), 21 deletions(-) diff --git a/lib/discourse_ip_info.rb b/lib/discourse_ip_info.rb index e7e28fb0d8a..ab697db919b 100644 --- a/lib/discourse_ip_info.rb +++ b/lib/discourse_ip_info.rb @@ -55,31 +55,27 @@ class DiscourseIpInfo end end - begin - gz_file = - FileHelper.download( - url, - max_file_size: 100.megabytes, - tmp_file_name: "#{name}.gz", - validate_uri: false, - follow_redirect: true, - extra_headers:, - ) + gz_file = + FileHelper.download( + url, + max_file_size: 100.megabytes, + tmp_file_name: "#{name}.gz", + validate_uri: false, + follow_redirect: true, + extra_headers:, + ) - filename = File.basename(gz_file.path) + filename = File.basename(gz_file.path) - 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("tar", "-xzvf", "#{dir}/#{filename}", chdir: dir) - Discourse::Utils.execute_command("cp", gz_file.path, "#{dir}/#{filename}") - - Discourse::Utils.execute_command("tar", "-xzvf", "#{dir}/#{filename}", chdir: dir) - - Dir["#{dir}/**/*.mmdb"].each { |f| FileUtils.mv(f, mmdb_path(name)) } - rescue => e - Discourse.warn_exception(e, message: "MaxMind database download failed.") - end + Dir["#{dir}/**/*.mmdb"].each { |f| FileUtils.mv(f, mmdb_path(name)) } + rescue => e + Discourse.warn_exception(e, message: "MaxMind database #{name} download failed.") ensure FileUtils.rm_r(dir, force: true) if dir gz_file&.close! diff --git a/spec/lib/discourse_ip_info_spec.rb b/spec/lib/discourse_ip_info_spec.rb index dd65bb4b920..3525d9a0ff4 100644 --- a/spec/lib/discourse_ip_info_spec.rb +++ b/spec/lib/discourse_ip_info_spec.rb @@ -60,5 +60,28 @@ RSpec.describe DiscourseIpInfo do described_class.mmdb_download("GeoLite2-City") 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