diff --git a/app/jobs/slack_profile_sync_job.rb b/app/jobs/slack_profile_sync_job.rb index ae1de0a7f..c94b135f1 100644 --- a/app/jobs/slack_profile_sync_job.rb +++ b/app/jobs/slack_profile_sync_job.rb @@ -22,5 +22,6 @@ def perform(user_id) retry_job(wait: [ e.retry_after, polynomial_delay ].max.seconds) rescue => e report_error(e, message: "Failed to update Slack username and avatar for user #{user_id}") + raise end end diff --git a/app/models/concerns/slack_integration.rb b/app/models/concerns/slack_integration.rb index f3f0f87ef..502c73b32 100644 --- a/app/models/concerns/slack_integration.rb +++ b/app/models/concerns/slack_integration.rb @@ -1,6 +1,13 @@ module SlackIntegration extend ActiveSupport::Concern + class ApiError < StandardError + def initialize(error, status: nil) + details = [ status && "HTTP #{status}", error ].compact.join(": ") + super("Slack profile API request failed: #{details}") + end + end + class RateLimitedError < StandardError attr_reader :retry_after @@ -30,10 +37,11 @@ def raw_slack_user_info response = HTTP.auth("Bearer #{access_token}") .get("https://slack.com/api/users.info?user=#{slack_uid}") raise RateLimitedError, response.headers["Retry-After"] if response.status.code == 429 - return nil unless response.status.success? data = JSON.parse(response.body.to_s) - data["user"] if data["ok"] + raise ApiError.new(data["error"] || "unexpected response", status: response.status.code) unless response.status.success? && data["ok"] + + data.fetch("user") end def update_from_slack diff --git a/test/jobs/slack_profile_sync_job_test.rb b/test/jobs/slack_profile_sync_job_test.rb index 13ae754de..3e7309503 100644 --- a/test/jobs/slack_profile_sync_job_test.rb +++ b/test/jobs/slack_profile_sync_job_test.rb @@ -65,7 +65,7 @@ class SlackProfileSyncJobTest < ActiveJob::TestCase assert_nil user.slack_synced_at end - test "preserves the existing profile when Slack returns an API error" do + test "fails visibly and preserves the existing profile when Slack returns an API error" do synced_at = 2.days.ago user = User.create!( timezone: "UTC", @@ -75,10 +75,13 @@ class SlackProfileSyncJobTest < ActiveJob::TestCase slack_synced_at: synced_at ) stub_request(:get, "https://slack.com/api/users.info?user=U_API_ERROR") - .to_return(status: 500, body: { ok: false, error: "internal_error" }.to_json) + .to_return(body: { ok: false, error: "missing_scope" }.to_json) - SlackProfileSyncJob.perform_now(user.id) + error = assert_raises(SlackIntegration::ApiError) do + SlackProfileSyncJob.perform_now(user.id) + end + assert_includes error.message, "missing_scope" user.reload assert_equal "existing-name", user.slack_username assert_equal "https://example.com/existing-avatar.png", user.slack_avatar_url