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

ZogStriP · web-flow · commit d2b1ca9f16cb · 2025-11-25T11:20:32.000+01:00
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
diff --git a/app/services/discourse_id/register.rb b/app/services/discourse_id/register.rb
@@ -33,25 +33,37 @@ def request_challenge
     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 @@ def register_with_challenge(token:)
     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 @@ def store_credentials(data:)
   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
diff --git a/config/locales/server.en.yml b/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."
diff --git a/lib/validators/enable_discourse_id_validator.rb b/lib/validators/enable_discourse_id_validator.rb
@@ -20,7 +20,7 @@ def error_message
     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
 
diff --git a/spec/lib/validators/enable_discourse_id_validator_spec.rb b/spec/lib/validators/enable_discourse_id_validator_spec.rb
@@ -49,14 +49,14 @@
           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 @@
           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
diff --git a/spec/services/discourse_id/register_spec.rb b/spec/services/discourse_id/register_spec.rb
@@ -46,6 +46,14 @@
         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 @@
         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 @@
         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 @@
           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
