FEATURE: log errors when enabling discourse id (#36110)

When admins try to enable discourse id via the site setting, the
automated registration process happen in the background. In the case of
an error, nothing was bubbled up back to the admin other than an
unhelpful error message.

On top of that, no errors were logged to help tech-savvy admins to
figure out what might be the problem(s).

This commit ensures that any error that is happening during the
registration process will be logged via the standard Rails logs.

On top of that, the error message shown to the admin has been updated to
hopefully provide a more helpful description.

Ref - https://meta.discourse.org/t/388711

app/services/discourse_id/register.rb

@@ -33,25 +33,37 @@ class DiscourseId::Register
     begin
       response = Net::HTTP.start(uri.hostname, uri.port, use_ssl:) { |http| http.request(request) }
     rescue StandardError => e
-      return fail!(error: "Challenge request failed: #{e.message}")
+      error = "Challenge request to '#{uri}' failed: #{e.message}."
+      log_error("request_challenge", error)
+      return fail!(error:)
     end
 
     if response.code.to_i != 200
-      return fail!(error: "Failed to request challenge: #{response.code}\nError: #{response.body}")
+      error = "Failed to request challenge: #{response.code}\nError: #{response.body}"
+      log_error("request_challenge", error)
+      return fail!(error:)
     end
 
     begin
       json = JSON.parse(response.body)
     rescue JSON::ParserError => e
-      return fail!(error: "Challenge response invalid JSON: #{e.message}")
+      error = "Challenge response invalid JSON: #{e.message}"
+      log_error("request_challenge", error)
+      return fail!(error:)
     end
 
     if json["domain"] != Discourse.current_hostname
-      return fail!(error: "Domain mismatch in challenge response")
+      error =
+        "Domain mismatch in challenge response (expected: #{Discourse.current_hostname}, got: #{json["domain"]})"
+      log_error("request_challenge", error)
+      return fail!(error:)
     end
 
     if Discourse.base_path.present? && json["path"] != Discourse.base_path
-      return fail!(error: "Path mismatch in challenge response")
+      error =
+        "Path mismatch in challenge response (expected: #{Discourse.base_path}, got: #{json["path"]})"
+      log_error("request_challenge", error)
+      return fail!(error:)
     end
 
     context[:token] = json["token"]
@@ -79,17 +91,23 @@ class DiscourseId::Register
     begin
       response = Net::HTTP.start(uri.hostname, uri.port, use_ssl:) { |http| http.request(request) }
     rescue StandardError => e
-      return fail!(error: "Registration request failed: #{e.message}")
+      error = "Registration request to '#{uri}' failed: #{e.message}."
+      log_error("register_with_challenge", error)
+      return fail!(error:)
     end
 
     if response.code.to_i != 200
-      return fail!(error: "Registration failed: #{response.code}\nError: #{response.body}")
+      error = "Registration failed: #{response.code}\nError: #{response.body}"
+      log_error("register_with_challenge", error)
+      return fail!(error:)
     end
 
     begin
       context[:data] = JSON.parse(response.body)
     rescue JSON::ParserError => e
-      fail!(error: "Registration response invalid JSON: #{e.message}")
+      error = "Registration response invalid JSON: #{e.message}"
+      log_error("register_with_challenge", error)
+      fail!(error:)
     end
   end
 
@@ -101,4 +119,8 @@ class DiscourseId::Register
   def discourse_id_url
     @url ||= SiteSetting.discourse_id_provider_url.presence || "https://id.discourse.com"
   end
+
+  def log_error(step, message)
+    Rails.logger.error("Discourse ID registration failed at step '#{step}'. Error: #{message}")
+  end
 end

config/locales/server.en.yml

@@ -2840,7 +2840,7 @@ en:
         other: "The list must contain exactly %{count} values."
       markdown_linkify_tlds: "You cannot include a value of '*'."
       google_oauth2_hd_groups: "You must configure all 'google oauth2 hd' settings before enabling this setting."
-      discourse_id_credentials: "You must configure Discourse ID credentials ('discourse_id_client_id' and 'discourse_id_client_secret') before enabling this setting."
+      discourse_id_registration: "Failed to automatically register with Discourse ID. This could be due to network connectivity issues, firewall restrictions, or the Discourse ID service being unreachable. Please check server logs for more details or contact support."
       linkedin_oidc_credentials: "You must configure LinkedIn OIDC credentials ('linkedin_oidc_client_id' and 'linkedin_oidc_client_secret') before enabling this setting."
       search_tokenize_chinese_enabled: "You must disable 'search_tokenize_chinese' before enabling this setting."
       search_tokenize_japanese_enabled: "You must disable 'search_tokenize_japanese' before enabling this setting."

lib/validators/enable_discourse_id_validator.rb

@@ -20,7 +20,7 @@ class EnableDiscourseIdValidator
     if @result&.error.present?
       @result.error
     elsif credentials_missing?
-      I18n.t("site_settings.errors.discourse_id_credentials")
+      I18n.t("site_settings.errors.discourse_id_registration")
     end
   end
 

spec/lib/validators/enable_discourse_id_validator_spec.rb

@@ -49,14 +49,14 @@ RSpec.describe EnableDiscourseIdValidator do
           expect(validator.error_message).to eq("an error")
         end
 
-        it "shows a default error message when something went _very_ wrong" do
+        it "shows a helpful error message when something went _very_ wrong" do
           failed_context = Service::Base::Context.new
           failed_context.fail
           allow(DiscourseId::Register).to receive(:call).and_return(failed_context)
 
           expect(validator.valid_value?("t")).to eq(false)
           expect(validator.error_message).to eq(
-            I18n.t("site_settings.errors.discourse_id_credentials"),
+            I18n.t("site_settings.errors.discourse_id_registration"),
           )
         end
       end
@@ -89,14 +89,14 @@ RSpec.describe EnableDiscourseIdValidator do
           expect(validator.error_message).to eq("another error")
         end
 
-        it "shows a default error message when something went _very_ wrong" do
+        it "shows a helpful error message when something went _very_ wrong" do
           failed_context = Service::Base::Context.new
           failed_context.fail(error: nil)
           allow(DiscourseId::Register).to receive(:call).and_return(failed_context)
 
           expect(validator.valid_value?("t")).to eq(false)
           expect(validator.error_message).to eq(
-            I18n.t("site_settings.errors.discourse_id_credentials"),
+            I18n.t("site_settings.errors.discourse_id_registration"),
           )
         end
       end

spec/services/discourse_id/register_spec.rb

@@ -46,6 +46,14 @@ RSpec.describe DiscourseId::Register do
         end
 
         it { is_expected.to fail_a_step(:request_challenge) }
+
+        it "logs detailed error with context" do
+          allow(Rails.logger).to receive(:error)
+          result
+          expect(Rails.logger).to have_received(:error).with(
+            /Discourse ID registration failed.*request_challenge.*Network error/m,
+          )
+        end
       end
 
       context "when challenge request returns non-200 status" do
@@ -57,6 +65,14 @@ RSpec.describe DiscourseId::Register do
         end
 
         it { is_expected.to fail_a_step(:request_challenge) }
+
+        it "logs error with status code and response body" do
+          allow(Rails.logger).to receive(:error)
+          result
+          expect(Rails.logger).to have_received(:error).with(
+            /Discourse ID registration failed.*request_challenge.*400.*Bad Request/m,
+          )
+        end
       end
 
       context "when challenge response is invalid JSON" do
@@ -79,6 +95,14 @@ RSpec.describe DiscourseId::Register do
         end
 
         it { is_expected.to fail_a_step(:request_challenge) }
+
+        it "logs error with expected and actual domains" do
+          allow(Rails.logger).to receive(:error)
+          result
+          expect(Rails.logger).to have_received(:error).with(
+            /Discourse ID registration failed.*request_challenge.*Domain mismatch.*expected.*#{Discourse.current_hostname}.*got.*wrong-domain\.com/m,
+          )
+        end
       end
 
       context "when challenge response has path mismatch" do
@@ -118,6 +142,14 @@ RSpec.describe DiscourseId::Register do
           end
 
           it { is_expected.to fail_a_step(:register_with_challenge) }
+
+          it "logs detailed error with context" do
+            allow(Rails.logger).to receive(:error)
+            result
+            expect(Rails.logger).to have_received(:error).with(
+              /Discourse ID registration failed.*register_with_challenge.*Connection timeout/m,
+            )
+          end
         end
 
         context "when registration returns non-200 status" do