fix: add back reason to test connection controller, updated test_connection/1 callback

lib/logflare/backends.ex

@@ -531,15 +531,10 @@ defmodule Logflare.Backends do
   @doc """
   Tests the connection for a given backend.
   """
-  @spec test_connection(Backend.t()) :: :ok | {:error, term()}
+  @spec test_connection(Backend.t()) :: :ok | {:error, :not_implemented} | {:error, atom()}
   def test_connection(%Backend{} = backend) do
     adaptor = Adaptor.get_adaptor(backend)
-
-    if function_exported?(adaptor, :test_connection, 1) do
-      adaptor.test_connection(backend)
-    else
-      {:error, :not_implemented}
-    end
+    adaptor.test_connection(backend)
   end
 
   @doc """

lib/logflare/backends/adaptor.ex

@@ -222,7 +222,7 @@ defmodule Logflare.Backends.Adaptor do
   @doc """
   Optional callback to test the underlying connection for an adaptor. May not be applicable for some adaptors.
   """
-  @callback test_connection(Backend.t()) :: :ok | {:error, term()}
+  @callback test_connection(Backend.t()) :: :ok | {:error, :not_implemented} | {:error, atom()}
 
   @doc """
   Optional callback to transform a stored backend config before usage.
@@ -295,7 +295,6 @@ defmodule Logflare.Backends.Adaptor do
                       execute_query: 3,
                       map_query_parameters: 4,
                       pre_ingest: 3,
-                      test_connection: 1,
                       transform_config: 1,
                       transform_query: 3,
                       send_alert: 3,

lib/logflare/backends/adaptor/bigquery_adaptor.ex

@@ -160,6 +160,9 @@ defmodule Logflare.Backends.Adaptor.BigQueryAdaptor do
     |> String.replace("-", "_")
   end
 
+  @impl Logflare.Backends.Adaptor
+  def test_connection(_), do: {:error, :not_implemented}
+
   @impl Logflare.Backends.Adaptor
   def ecto_to_sql(%Ecto.Query{} = query, _opts) do
     with {:ok, {pg_sql, pg_params}} <- SqlUtils.ecto_to_pg_sql(query) do

lib/logflare/backends/adaptor/clickhouse_adaptor.ex

@@ -191,7 +191,7 @@ defmodule Logflare.Backends.Adaptor.ClickHouseAdaptor do
           :ok
           | {:error, :permissions_missing}
           | {:error, :read_permissions_missing}
-          | {:error, term()}
+          | {:error, :grant_check_unknown_failure}
   def test_connection(%Backend{config: config} = backend) do
     with :ok <- check_ingest_grants(backend, config),
          :ok <- maybe_check_read_grants(backend, config) do
@@ -200,7 +200,7 @@ defmodule Logflare.Backends.Adaptor.ClickHouseAdaptor do
   end
 
   @spec check_ingest_grants(Backend.t(), map()) ::
-          :ok | {:error, :permissions_missing} | {:error, term()}
+          :ok | {:error, :permissions_missing} | {:error, :grant_check_unknown_failure}
   defp check_ingest_grants(%Backend{} = backend, config) do
     sql_statement = QueryTemplates.grant_check_statement()
 
@@ -222,7 +222,7 @@ defmodule Logflare.Backends.Adaptor.ClickHouseAdaptor do
           backend_id: backend.id
         )
 
-        error_result
+        {:error, :grant_check_unknown_failure}
     end
   end
 
@@ -255,7 +255,7 @@ defmodule Logflare.Backends.Adaptor.ClickHouseAdaptor do
           backend_id: backend.id
         )
 
-        error_result
+        {:error, :grant_check_unknown_failure}
     end
   end
 

lib/logflare/backends/adaptor/elastic_adaptor.ex

@@ -79,4 +79,7 @@ defmodule Logflare.Backends.Adaptor.ElasticAdaptor do
     changeset
     |> validate_required([:url])
   end
+
+  @impl Logflare.Backends.Adaptor
+  def test_connection(_), do: {:error, :not_implemented}
 end

lib/logflare/backends/adaptor/otlp_adaptor/common.ex

@@ -2,19 +2,59 @@ defmodule Logflare.Backends.Adaptor.OtlpAdaptor.Common do
   alias Logflare.Backends.Backend
   alias Logflare.Backends.Adaptor.HttpBased
 
+  require Logger
+
   @doc """
   A code for testing connection meant to be shared by all OTLP based adaptors.
 
   Sends an empty list of events, expecting success response.
   """
   @spec test_connection(module(), Backend.t()) ::
-          :ok | {:error, term()}
+          :ok
+          | {:error,
+             :http_client_error | :http_server_error | :http_unknown_error | :unknown_error}
   def test_connection(client_module, %Backend{} = backend) do
     case HttpBased.Client.send_events(client_module, [], backend) do
-      {:ok, %Tesla.Env{status: 200, body: %{partial_success: nil}}} -> :ok
-      {:ok, %Tesla.Env{status: 200, body: %{partial_success: %{error_message: ""}}}} -> :ok
-      {:ok, env} -> {:error, env}
-      {:error, _reason} = err -> err
+      {:ok, %Tesla.Env{status: 200, body: %{partial_success: nil}}} ->
+        :ok
+
+      {:ok, %Tesla.Env{status: 200, body: %{partial_success: %{error_message: ""}}}} ->
+        :ok
+
+      {:ok, %Tesla.Env{status: status, body: resp_body}} when status in 400..499 ->
+        Logger.warning(
+          "Client error when testing OTLP backend connection: #{status} #{inspect(resp_body)}",
+          backend_id: backend.id,
+          user_id: backend.user_id
+        )
+
+        {:error, :http_client_error}
+
+      {:ok, %Tesla.Env{status: status, body: resp_body}} when status in 500..599 ->
+        Logger.warning(
+          "Server error when testing OTLP backend connection: #{status} #{inspect(resp_body)}",
+          backend_id: backend.id,
+          user_id: backend.user_id
+        )
+
+        {:error, :http_server_error}
+
+      {:ok, %Tesla.Env{status: status, body: resp_body}} ->
+        Logger.warning(
+          "Unknown http error #{status} when testing OTLP backend connection: #{inspect(resp_body)}",
+          backend_id: backend.id,
+          user_id: backend.user_id
+        )
+
+        {:error, :http_unknown_error}
+
+      {:error, reason} ->
+        Logger.warning("Request error when testing OTLP backend connection: #{inspect(reason)}",
+          backend_id: backend.id,
+          user_id: backend.user_id
+        )
+
+        {:error, :unknown_error}
     end
   end
 end

lib/logflare/backends/adaptor/postgres_adaptor.ex

@@ -152,8 +152,21 @@ defmodule Logflare.Backends.Adaptor.PostgresAdaptor do
   @spec test_connection(Backend.t()) :: :ok | {:error, term()}
   def test_connection(%Backend{} = backend) do
     case execute_query(backend, "SELECT 1 AS result", []) do
-      {:ok, %QueryResult{rows: [%{"result" => 1}]}} -> :ok
-      {:error, _} = error -> error
+      {:ok, %QueryResult{rows: [%{"result" => 1}]}} ->
+        :ok
+
+      {:error, %QueryError{kind: kind}}
+      when kind in [:connection_error, :backend_error] ->
+        {:error, kind}
+
+      {:error, reason} ->
+        Logger.warning(
+          "Unexpected error when testing Postgres backend connection: #{inspect(reason)}",
+          backend_id: backend.id,
+          user_id: backend.user_id
+        )
+
+        {:error, :unknown_error}
     end
   end
 

lib/logflare/backends/adaptor/sentry_adaptor.ex

@@ -23,6 +23,8 @@ defmodule Logflare.Backends.Adaptor.SentryAdaptor do
   alias Logflare.Backends.Adaptor.SentryAdaptor.DSN
   alias Logflare.Backends.Adaptor.SentryAdaptor.EnvelopeBuilder
 
+  require Logger
+
   @behaviour Adaptor
   @behaviour HttpBased.Client
 
@@ -64,13 +66,49 @@ defmodule Logflare.Backends.Adaptor.SentryAdaptor do
   end
 
   @impl Adaptor
-  @spec test_connection(Backend.t()) :: :ok | {:error, term()}
+  @spec test_connection(Backend.t()) ::
+          :ok
+          | {:error,
+             :http_client_error | :http_server_error | :http_unknown_error | :unknown_error}
   def test_connection(%Backend{} = backend) do
     case HttpBased.Client.send_events(__MODULE__, [], backend) do
-      {:ok, %Tesla.Env{status: 200}} -> :ok
-      {:ok, %Tesla.Env{body: %{"detail" => detail}}} -> {:error, detail}
-      {:ok, env} -> {:error, "Unexpected response: #{env.status} #{inspect(env.body)}"}
-      {:error, reason} -> {:error, "Request error: #{inspect(reason)}"}
+      {:ok, %Tesla.Env{status: 200}} ->
+        :ok
+
+      {:ok, %Tesla.Env{status: status, body: resp_body}} when status in 400..499 ->
+        Logger.warning(
+          "Unexpected response when testing Sentry backend connection: #{status} #{inspect(resp_body)}",
+          backend_id: backend.id,
+          user_id: backend.user_id
+        )
+
+        {:error, :http_client_error}
+
+      {:ok, %Tesla.Env{status: status, body: resp_body}} when status in 500..599 ->
+        Logger.warning(
+          "Server error when testing Sentry backend connection: #{status} #{inspect(resp_body)}",
+          backend_id: backend.id,
+          user_id: backend.user_id
+        )
+
+        {:error, :http_server_error}
+
+      {:ok, %Tesla.Env{status: status, body: resp_body}} ->
+        Logger.warning(
+          "Unknown http error #{status} when testing Sentry backend connection: #{inspect(resp_body)}",
+          backend_id: backend.id,
+          user_id: backend.user_id
+        )
+
+        {:error, :http_unknown_error}
+
+      {:error, reason} ->
+        Logger.warning("Request error when testing Sentry backend connection: #{inspect(reason)}",
+          backend_id: backend.id,
+          user_id: backend.user_id
+        )
+
+        {:error, :unknown_error}
     end
   end
 

lib/logflare/backends/adaptor/syslog_adaptor.ex

@@ -9,6 +9,9 @@ defmodule Logflare.Backends.Adaptor.SyslogAdaptor do
   import NimbleParsec
   import Logflare.Logs.SyslogParser.Helpers
   alias Logflare.Backends.Adaptor.SyslogAdaptor.{Pool, Socket, Pipeline}
+  alias Logflare.Backends.Backend
+  require Logger
+
   @behaviour Logflare.Backends.Adaptor
 
   typedstruct enforce: true do
@@ -174,6 +177,7 @@ defmodule Logflare.Backends.Adaptor.SyslogAdaptor do
   end
 
   @impl Logflare.Backends.Adaptor
+  @spec test_connection(Backend.t()) :: :ok | {:error, :socket_closed | :timeout | :unknown_error}
   def test_connection(backend) do
     result =
       with {:ok, socket} <- Socket.connect(backend.config, to_timeout(second: 3)) do
@@ -182,13 +186,24 @@ defmodule Logflare.Backends.Adaptor.SyslogAdaptor do
       end
 
     with {:error, reason} <- result do
-      {:error, format_connection_error(reason)}
+      formatted_error = format_connection_error(reason)
+
+      Logger.warning("Unexpected error when testing Syslog backend connection: #{reason}",
+        backend_id: backend.id,
+        user_id: backend.user_id
+      )
+
+      if is_binary(formatted_error) do
+        {:error, :unknown_error}
+      else
+        {:error, formatted_error}
+      end
     end
   end
 
   # copied from mint: https://github.com/elixir-mint/mint/blob/0bfcc869b53b83989c24ba681d66d0a447b5a1c3/lib/mint/transport_error.ex#L86-L101
-  defp format_connection_error(:closed), do: "socket closed"
-  defp format_connection_error(:timeout), do: "timeout"
+  defp format_connection_error(:closed), do: :socket_closed
+  defp format_connection_error(:timeout), do: :timeout
 
   defp format_connection_error(reason) do
     case :ssl.format_error(reason) do

lib/logflare/backends/adaptor/webhook_adaptor.ex

@@ -24,6 +24,8 @@
   alias Logflare.Utils
   alias Logflare.Utils.SSRF
 
+  require Logger
+
   @behaviour Logflare.Backends.Adaptor
 
   # Sentinel value substituted for secret header values by redact_config/1.
@@ -145,8 +147,11 @@
   `IncidentioAdaptor`) to share HTTP plumbing while choosing a payload shape
   the receiver will accept.
   """
-  @spec test_connection(Backend.t(), term()) :: :ok | {:error, term()}
-  def test_connection(%Backend{config: config}, body) do
+  @spec test_connection(Backend.t(), term()) ::
+          :ok
+          | {:error,
+             :http_client_error | :http_server_error | :http_unknown_error | :unknown_error}
+  def test_connection(%Backend{config: config, id: backend_id, user_id: user_id}, body) do
     response =
       __MODULE__.Client.send(
         url: config.url,
@@ -160,11 +165,41 @@
       {:ok, %Tesla.Env{status: status}} when status in 200..299 ->
         :ok
 
+      {:ok, %Tesla.Env{status: status, body: resp_body}} when status in 400..499 ->
+        Logger.warning(
+          "Client error when testing HTTP Webhook backend connection: #{status} #{inspect(resp_body)}",
+          backend_id: backend_id,
+          user_id: user_id
+        )
+
+        {:error, :http_client_error}
+
+      {:ok, %Tesla.Env{status: status, body: resp_body}} when status in 500..599 ->
+        Logger.warning(
+          "Server error when testing HTTP Webhook backend connection: #{status} #{inspect(resp_body)}",
+          backend_id: backend_id,
+          user_id: user_id
+        )
+
+        {:error, :http_server_error}
+
       {:ok, %Tesla.Env{status: status, body: resp_body}} ->
-        {:error, "Unexpected response: #{status} #{inspect(resp_body)}"}
+        Logger.warning(
+          "Unknown http error #{status} when testing HTTP Webhook backend connection: #{inspect(resp_body)}",
+          backend_id: backend_id,
+          user_id: user_id
+        )
+
+        {:error, :http_unknown_error}
 
       {:error, reason} ->
-        {:error, "Request error: #{inspect(reason)}"}
+        Logger.warning(
+          "Request error when testing HTTP Webhook backend connection: #{inspect(reason)}",
+          backend_id: backend_id,
+          user_id: user_id
+        )
+
+        {:error, :unknown_error}
     end
   end
 
@@ -373,7 +408,7 @@
     end
 
     def ack(_ack_ref, _successful, _failed) do
       # TODO: re-queue failed
     end
   end
 end

lib/logflare_web/controllers/api/backend_controller.ex

@@ -117,9 +117,9 @@ defmodule LogflareWeb.Api.BackendController do
           conn
           |> json(%{connected?: true})
 
-        {:error, _reason} ->
+        {:error, reason} when is_atom(reason) ->
           conn
-          |> json(%{connected?: false})
+          |> json(%{connected?: false, reason: reason})
       end
     end
   end

test/logflare/backends/adaptor/clickhouse_adaptor/provisioner_test.exs

@@ -3,7 +3,6 @@ defmodule Logflare.Backends.Adaptor.ClickHouseAdaptor.ProvisionerTest do
 
   alias Logflare.Backends.Adaptor.ClickHouseAdaptor
   alias Logflare.Backends.Adaptor.ClickHouseAdaptor.Provisioner
-  alias Logflare.Backends.QueryError
 
   import Logflare.ClickHouseMappedEvents
 
@@ -86,16 +85,9 @@ defmodule Logflare.Backends.Adaptor.ClickHouseAdaptor.ProvisionerTest do
       pid = start_supervised!({Provisioner, invalid_backend}, restart: :transient)
       ref = Process.monitor(pid)
 
-      TestUtils.retry_assert(fn ->
-        assert_receive {:DOWN, ^ref, :process, ^pid,
-                        {:shutdown,
-                         {:error,
-                          %QueryError{
-                            kind: :connection_error,
-                            backend: Logflare.Backends.Adaptor.ClickHouseAdaptor
-                          }}}},
-                       5_000
-      end)
+      assert_receive {:DOWN, ^ref, :process, ^pid,
+                      {:shutdown, {:error, :grant_check_unknown_failure}}},
+                     5_000
     end
   end