fix(logger): retain auto-handler on invalid user config for Sentry.LoggerHandler

Co-authored-by: Copilot <copilot@github.com>

Author: solnic

lib/sentry/logger_handler.ex

@@ -263,18 +263,6 @@ defmodule Sentry.LoggerHandler do
   @doc false
   @spec adding_handler(:logger.handler_config()) :: {:ok, :logger.handler_config()}
   def adding_handler(config) do
-    # If a user explicitly registers their own Sentry.LoggerHandler (i.e., not the
-    # auto-registered :sentry_log_handler), remove the auto-registered one to prevent
-    # duplicate log capture. We spawn a separate process to do the removal because
-    # adding_handler/1 is called from within the logger gen_server process, and calling
-    # :logger.remove_handler/1 directly would deadlock.
-    if config.id != :sentry_log_handler do
-      _ = spawn(fn -> :logger.remove_handler(:sentry_log_handler) end)
-      :ok
-    else
-      :ok
-    end
-
     # The :config key may not be here.
     sentry_config = Map.get(config, :config, %{})
 
@@ -294,12 +282,26 @@ defmodule Sentry.LoggerHandler do
 
     config = Map.put(config, :config, handler_config)
 
-    if rate_limiting_config = config.config.rate_limiting do
-      _ = RateLimiter.start_under_sentry_supervisor(config.id, rate_limiting_config)
-      {:ok, config}
-    else
-      {:ok, config}
+    result =
+      if rate_limiting_config = config.config.rate_limiting do
+        _ = RateLimiter.start_under_sentry_supervisor(config.id, rate_limiting_config)
+        {:ok, config}
+      else
+        {:ok, config}
+      end
+
+    # If a user explicitly registers their own Sentry.LoggerHandler (i.e., not the
+    # auto-registered :sentry_log_handler), remove the auto-registered one to prevent
+    # duplicate log capture. We do this after all validation so that if config is
+    # invalid (and adding_handler raises), the auto-handler stays active. We spawn a
+    # separate process because adding_handler/1 is called from within the logger
+    # gen_server process, and calling :logger.remove_handler/1 directly would deadlock.
+    if config.id != :sentry_log_handler do
+      _ = spawn(fn -> :logger.remove_handler(:sentry_log_handler) end)
+      :ok
     end
+
+    result
   end
 
   # Callback for :logger handlers

test/sentry/application_test.exs

@@ -115,6 +115,21 @@ defmodule Sentry.ApplicationTest do
                :logger.get_handler_config(:sentry_log_handler)
     end
 
+    test "keeps auto-handler when a user adds a Sentry.LoggerHandler with invalid config" do
+      restart_sentry_with(dsn: "https://public@sentry.example.com/1", enable_logs: true)
+      assert {:ok, _} = :logger.get_handler_config(:sentry_log_handler)
+
+      user_handler = :"user_sentry_handler_#{System.unique_integer([:positive])}"
+
+      assert {:error, _reason} =
+               :logger.add_handler(user_handler, Sentry.LoggerHandler, %{
+                 config: %{sync_threshold: 10, discard_threshold: 20}
+               })
+
+      assert {:ok, _} = :logger.get_handler_config(:sentry_log_handler)
+      assert {:error, {:not_found, ^user_handler}} = :logger.get_handler_config(user_handler)
+    end
+
     test "auto-handler captures logs to the buffer" do
       restart_sentry_with(dsn: "https://public@sentry.example.com/1", enable_logs: true)