diff --git a/app/models/shipit/user.rb b/app/models/shipit/user.rb index 6de8fe8d6..a61c931c9 100644 --- a/app/models/shipit/user.rb +++ b/app/models/shipit/user.rb @@ -93,6 +93,13 @@ def stacks_contributed_to Commit.where('author_id = :id or committer_id = :id', id:).distinct.pluck(:stack_id) end + TRANSIENT_GITHUB_REFRESH_ERRORS = [ + Faraday::ConnectionFailed, + Faraday::TimeoutError, + Net::OpenTimeout, + Net::ReadTimeout, + ].freeze + def refresh_from_github! # Users are global, any app can be used # This will not work for users that only exist in an Enterprise install @@ -101,6 +108,9 @@ def refresh_from_github! identify_renamed_user! rescue Octokit::Forbidden Rails.logger.info("User #{name}, github_id #{github_id} has forbidden access to their GitHub, likely deleted.") + rescue *TRANSIENT_GITHUB_REFRESH_ERRORS => error + instrument_transient_github_refresh_error(error) + raise end def github_user=(github_user) @@ -155,6 +165,24 @@ def identify_renamed_user! false end + def instrument_transient_github_refresh_error(error) + payload = { + user_id: id, + github_id: github_id, + github_login: login, + error_class: error.class.name, + error_message: error.message, + operation: "refresh_github_user", + } + + ActiveSupport::Notifications.instrument("transient_github_refresh_error.shipit", payload) + Rails.logger.warn( + "Transient GitHub user refresh error " \ + "user_id=#{id} github_id=#{github_id} github_login=#{login.inspect} " \ + "error_class=#{error.class.name} error_message=#{error.message.inspect}" + ) + end + def email_valid_and_preferred?(email_address) org_domains = Shipit.preferred_org_emails return true if org_domains.blank? diff --git a/test/models/users_test.rb b/test/models/users_test.rb index 4d2b81756..2732c0ca1 100644 --- a/test/models/users_test.rb +++ b/test/models/users_test.rb @@ -212,6 +212,31 @@ class UsersTest < ActiveSupport::TestCase @user.refresh_from_github! end + test "#refresh_from_github! instruments transient GitHub failures and reraises" do + error = Net::OpenTimeout.new("execution expired") + events = [] + subscriber = lambda do |*args| + events << ActiveSupport::Notifications::Event.new(*args) + end + + Shipit.github.api.expects(:user).with(@user.github_id).raises(error) + Rails.logger.expects(:warn).with(regexp_matches(/Transient GitHub user refresh error/)) + + ActiveSupport::Notifications.subscribed(subscriber, "transient_github_refresh_error.shipit") do + assert_raises Net::OpenTimeout do + @user.refresh_from_github! + end + end + + assert_equal 1, events.size + assert_equal @user.id, events.first.payload[:user_id] + assert_equal @user.github_id, events.first.payload[:github_id] + assert_equal @user.login, events.first.payload[:github_login] + assert_equal "Net::OpenTimeout", events.first.payload[:error_class] + assert_equal "execution expired", events.first.payload[:error_message] + assert_equal "refresh_github_user", events.first.payload[:operation] + end + test "#github_api uses the user's access token" do assert_equal @user.github_access_token, @user.github_api.access_token end