From 1ad6ec98c033a7146283260174e590b32faa130b Mon Sep 17 00:00:00 2001 From: Matt Stubbs Date: Mon, 1 Jun 2026 14:06:10 +1000 Subject: [PATCH 1/4] Add backend query error normalization --- lib/logflare/alerting.ex | 19 +-- .../backends/adaptor/bigquery_adaptor.ex | 47 +++-- .../backends/adaptor/clickhouse_adaptor.ex | 51 ++++-- .../backends/adaptor/postgres_adaptor.ex | 50 +++++- lib/logflare/backends/query_error.ex | 20 +++ lib/logflare/ecto/bigquery/bq_repo.ex | 2 +- lib/logflare/google/bigquery/gen_utils.ex | 8 +- lib/logflare_web/live/alerts/alerts_live.ex | 10 +- .../live/search_live/logs_search_lv.ex | 51 ++---- lib/logflare_web/query_error_helpers.ex | 126 ++++++++++++++ test/e2e/features/logs_search_test.exs | 10 ++ test/logflare/alerting_test.exs | 27 +++ .../adaptor/bigquery_adaptor_test.exs | 76 +++++++++ .../clickhouse_adaptor/provisioner_test.exs | 8 +- .../adaptor/clickhouse_adaptor_test.exs | 33 +++- .../adaptor/postgres_adaptor_test.exs | 68 ++++++++ test/logflare/backends/query_error_test.exs | 19 +++ test/logflare/endpoints/cache_test.exs | 17 +- .../controllers/endpoints_controller_test.exs | 2 +- .../live/alerts/alerts_live_test.exs | 17 ++ .../logflare_web/live/log_event_live_test.exs | 4 +- .../live/search_live/logs_search_lv_test.exs | 160 +++++++++++++++++- .../logflare_web/query_error_helpers_test.exs | 5 + test/support/test_utils.ex | 9 +- 24 files changed, 737 insertions(+), 102 deletions(-) create mode 100644 lib/logflare/backends/query_error.ex create mode 100644 lib/logflare_web/query_error_helpers.ex create mode 100644 test/logflare/backends/query_error_test.exs create mode 100644 test/logflare_web/query_error_helpers_test.exs diff --git a/lib/logflare/alerting.ex b/lib/logflare/alerting.ex index ceea7b013d..6a5de5f0ca 100644 --- a/lib/logflare/alerting.ex +++ b/lib/logflare/alerting.ex @@ -15,10 +15,10 @@ defmodule Logflare.Alerting do alias Logflare.Backends.Adaptor.SlackAdaptor alias Logflare.Backends.Adaptor.WebhookAdaptor alias Logflare.Backends.Adaptor.QueryResult + alias Logflare.Backends.QueryError alias Logflare.Cluster alias Logflare.Endpoints alias Logflare.Google.BigQuery.GCPConfig - alias Logflare.Google.BigQuery.GenUtils alias Logflare.Repo alias Logflare.Teams alias Logflare.TeamUsers.TeamUser @@ -467,23 +467,14 @@ defmodule Logflare.Alerting do ) do {:ok, result} else - {:error, %Tesla.Env{body: body}} -> - decoded = Jason.decode!(body)["error"] - - error = - decoded - |> GenUtils.process_bq_errors(alert_query.user_id) - |> case do - %{"message" => msg} -> msg - other -> other - end - + {:error, %QueryError{} = error} -> Logger.error("Alert query execution failed with bad response", user_id: alert_query.user_id, alert_query_id: alert_query.id, alert_name: alert_query.name, - possible_reservation_error: BigQueryAdaptor.reservation_error?(decoded), - error_string: inspect(error) + possible_reservation_error: BigQueryAdaptor.reservation_error?(error.raw_error), + error_code: error.code, + error_string: inspect(error.message) ) {:error, error} diff --git a/lib/logflare/backends/adaptor/bigquery_adaptor.ex b/lib/logflare/backends/adaptor/bigquery_adaptor.ex index cdbd63abf9..200c2e238b 100644 --- a/lib/logflare/backends/adaptor/bigquery_adaptor.ex +++ b/lib/logflare/backends/adaptor/bigquery_adaptor.ex @@ -21,6 +21,7 @@ defmodule Logflare.Backends.Adaptor.BigQueryAdaptor do alias Logflare.Backends.Ecto.SqlUtils alias Logflare.Backends.IngestEventQueue alias Logflare.Backends.Adaptor.QueryResult + alias Logflare.Backends.QueryError alias Logflare.BigQuery.SchemaTypes alias Logflare.Billing alias Logflare.BqRepo @@ -685,7 +686,7 @@ defmodule Logflare.Backends.Adaptor.BigQueryAdaptor do query_opts :: Keyword.t() ) :: {:ok, QueryResult.t()} - | {:error, any()} + | {:error, QueryError.t()} defp execute_user_query(%User{} = user, project_id, query_string, bq_params, query_opts) when is_non_empty_binary(query_string) and is_list(bq_params) and is_list(query_opts) do case BqRepo.query_with_sql_and_params( @@ -704,17 +705,33 @@ defmodule Logflare.Backends.Adaptor.BigQueryAdaptor do bq_params: bq_params })} - {:error, %{body: body}} -> - decoded = Jason.decode!(body)["error"] - error = GenUtils.process_bq_errors(decoded, user.id) - maybe_warn_reservation_error(decoded, user, project_id, query_opts) - {:error, error} + {:error, error} -> + maybe_warn_reservation_error(error, user, project_id, query_opts) + + {:error, to_query_error(error, user.id)} + end + end - {:error, err} when is_atom(err) -> - {:error, GenUtils.process_bq_errors(err, user.id)} + @spec to_query_error(term(), pos_integer()) :: QueryError.t() + defp to_query_error(error, _user_id) when error in [:timeout, :closed, :emfile] do + %QueryError{ + message: GenUtils.get_tesla_error_message(error), + code: :connection_error, + raw_error: error, + backend: Logflare.Backends.Adaptor.BigQueryAdaptor + } + end - {:error, err} -> - {:error, err} + defp to_query_error(%{body: body}, user_id) do + with %{"error" => raw_error} <- Jason.decode!(body), + %{"message" => message} = processed_error <- + GenUtils.process_bq_errors(raw_error, user_id) do + %QueryError{ + message: message, + code: :invalid_query, + raw_error: processed_error, + backend: Logflare.Backends.Adaptor.BigQueryAdaptor + } end end @@ -741,13 +758,15 @@ defmodule Logflare.Backends.Adaptor.BigQueryAdaptor do end @spec maybe_warn_reservation_error( - decoded :: any(), + error :: any(), user :: User.t(), project_id :: String.t(), query_opts :: Keyword.t() ) :: :ok - defp maybe_warn_reservation_error(decoded, %User{} = user, project_id, query_opts) do - if reservation_error?(decoded) and not caller_logs_own_errors?(query_opts) do + defp maybe_warn_reservation_error(%{body: body}, %User{} = user, project_id, query_opts) do + with %{"error" => decoded} <- Jason.decode!(body), + true <- reservation_error?(decoded), + false <- caller_logs_own_errors?(query_opts) do Logger.warning("Possible BigQuery reservation error", user_id: user.id, project_id: project_id, @@ -760,6 +779,8 @@ defmodule Logflare.Backends.Adaptor.BigQueryAdaptor do :ok end + defp maybe_warn_reservation_error(_error, %User{}, _project_id, _query_opts), do: :ok + @spec caller_logs_own_errors?(query_opts :: Keyword.t()) :: boolean() defp caller_logs_own_errors?(query_opts) do Keyword.get(query_opts, :query_type) == :alerts diff --git a/lib/logflare/backends/adaptor/clickhouse_adaptor.ex b/lib/logflare/backends/adaptor/clickhouse_adaptor.ex index 62ef4a830d..7f1d388a0f 100644 --- a/lib/logflare/backends/adaptor/clickhouse_adaptor.ex +++ b/lib/logflare/backends/adaptor/clickhouse_adaptor.ex @@ -30,6 +30,7 @@ defmodule Logflare.Backends.Adaptor.ClickHouseAdaptor do alias Logflare.Backends.Ecto.SqlUtils alias Logflare.Backends.IngestEventQueue alias Logflare.Backends.Adaptor.QueryResult + alias Logflare.Backends.QueryError alias Logflare.LogEvent alias Logflare.LogEvent.TypeDetection @@ -344,7 +345,7 @@ defmodule Logflare.Backends.Adaptor.ClickHouseAdaptor do statement :: iodata(), params :: map | [term] | [row :: [term]] | iodata | Enumerable.t(), [Ch.query_option()] - ) :: {:ok, Ch.Result.t()} | {:error, Exception.t()} + ) :: {:ok, Ch.Result.t()} | {:error, QueryError.t()} def execute_ch_query(backend, statement, params \\ [], opts \\ []) def execute_ch_query(%Backend{} = backend, statement, params, opts) @@ -367,26 +368,26 @@ defmodule Logflare.Backends.Adaptor.ClickHouseAdaptor do {:ok, %Ch.Result{} = result} -> {:ok, decode_ch_result(result)} - {:error, %Ch.Error{message: error_msg}} when is_non_empty_binary(error_msg) -> + {:error, %Ch.Error{message: error_msg} = error} when is_non_empty_binary(error_msg) -> Logger.warning( "ClickHouse query failed: #{inspect(error_msg)}", backend_id: backend.id, host: read_host(backend) ) - {:error, "Error executing ClickHouse query"} + {:error, to_query_error(error)} - {:error, %{message: message}} when is_non_empty_binary(message) -> + {:error, %{message: message} = error} when is_non_empty_binary(message) -> Logger.warning( "ClickHouse query failed: #{inspect(message)}", backend_id: backend.id, host: read_host(backend) ) - {:error, "Error executing ClickHouse query"} + {:error, to_query_error(error)} - {:error, _} -> - {:error, "Error executing ClickHouse query"} + {:error, error} -> + {:error, to_query_error(error)} end end end @@ -414,6 +415,34 @@ defmodule Logflare.Backends.Adaptor.ClickHouseAdaptor do @ch_slow_pool_checkout_ms end + @spec to_query_error(term()) :: QueryError.t() + defp to_query_error(%Ch.Error{message: message} = error) when is_non_empty_binary(message) do + %QueryError{ + message: message, + code: :invalid_query, + raw_error: error, + backend: Logflare.Backends.Adaptor.ClickHouseAdaptor + } + end + + defp to_query_error(%DBConnection.ConnectionError{message: message} = error) do + %QueryError{ + message: message, + code: :connection_error, + raw_error: error, + backend: Logflare.Backends.Adaptor.ClickHouseAdaptor + } + end + + defp to_query_error(error) do + %QueryError{ + message: inspect(error), + code: :backend_error, + raw_error: error, + backend: Logflare.Backends.Adaptor.ClickHouseAdaptor + } + end + @spec execute_direct_query(url :: String.t(), config :: map(), statement :: String.t()) :: {:ok, list()} | {:error, term()} defp execute_direct_query(url, config, statement) do @@ -439,15 +468,15 @@ defmodule Logflare.Backends.Adaptor.ClickHouseAdaptor do {:ok, %Ch.Result{} = result} -> {:ok, decode_ch_result(result)} - {:error, _} -> - {:error, "Error executing ClickHouse query"} + {:error, error} -> + {:error, to_query_error(error)} end after GenServer.stop(pid) end - {:error, _} -> - {:error, "Error executing ClickHouse query"} + {:error, error} -> + {:error, to_query_error(error)} end end diff --git a/lib/logflare/backends/adaptor/postgres_adaptor.ex b/lib/logflare/backends/adaptor/postgres_adaptor.ex index 2dcbadb5fb..61a581c17b 100644 --- a/lib/logflare/backends/adaptor/postgres_adaptor.ex +++ b/lib/logflare/backends/adaptor/postgres_adaptor.ex @@ -23,6 +23,7 @@ defmodule Logflare.Backends.Adaptor.PostgresAdaptor do alias Logflare.Backends.Backend alias Logflare.Backends.Ecto.SqlUtils alias Logflare.Backends.Adaptor.QueryResult + alias Logflare.Backends.QueryError alias Logflare.SingleTenant alias Logflare.Sources.Source alias Logflare.Sql @@ -91,12 +92,17 @@ defmodule Logflare.Backends.Adaptor.PostgresAdaptor do def execute_query(%Backend{} = backend, %Ecto.Query{} = query, _opts) do mod = PgRepo.create_repo(backend) - result = - query - |> mod.all() - |> Enum.map(&nested_map_update/1) + try do + result = + query + |> mod.all() + |> Enum.map(&nested_map_update/1) - {:ok, QueryResult.new(result, pg_meta(result))} + {:ok, QueryResult.new(result, pg_meta(result))} + rescue + error in [Postgrex.Error, DBConnection.ConnectionError, Ecto.QueryError] -> + {:error, to_query_error(error)} + end end def execute_query(%Backend{} = backend, query_string, opts) @@ -119,6 +125,12 @@ defmodule Logflare.Backends.Adaptor.PostgresAdaptor do end {:ok, QueryResult.new(rows, pg_meta(rows))} + else + {:error, :cannot_connect} = error -> + error + + {:error, error} -> + {:error, to_query_error(error)} end end @@ -212,6 +224,34 @@ defmodule Logflare.Backends.Adaptor.PostgresAdaptor do Map.put(config, :url, updated) end + @spec to_query_error(term()) :: QueryError.t() + defp to_query_error(%Postgrex.Error{} = error) do + %QueryError{ + message: Exception.message(error), + code: :invalid_query, + raw_error: error, + backend: Logflare.Backends.Adaptor.PostgresAdaptor + } + end + + defp to_query_error(%DBConnection.ConnectionError{message: message} = error) do + %QueryError{ + message: message, + code: :connection_error, + raw_error: error, + backend: Logflare.Backends.Adaptor.PostgresAdaptor + } + end + + defp to_query_error(%Ecto.QueryError{message: message} = error) do + %QueryError{ + message: message, + code: :invalid_query, + raw_error: error, + backend: Logflare.Backends.Adaptor.PostgresAdaptor + } + end + # expose PgRepo functions defdelegate create_repo(backend), to: PgRepo defdelegate table_name(source), to: PgRepo diff --git a/lib/logflare/backends/query_error.ex b/lib/logflare/backends/query_error.ex new file mode 100644 index 0000000000..b67aba2369 --- /dev/null +++ b/lib/logflare/backends/query_error.ex @@ -0,0 +1,20 @@ +defmodule Logflare.Backends.QueryError do + @moduledoc false + + @derive {Jason.Encoder, only: [:message]} + @enforce_keys [:message, :code, :raw_error, :backend] + defstruct [:message, :code, :raw_error, :backend, :description] + + @type backend :: + Logflare.Backends.Adaptor.BigQueryAdaptor + | Logflare.Backends.Adaptor.ClickHouseAdaptor + | Logflare.Backends.Adaptor.PostgresAdaptor + @type code :: :invalid_query | :connection_error | :backend_error + @type t :: %__MODULE__{ + message: String.t(), + code: code(), + raw_error: term(), + backend: backend(), + description: String.t() | nil + } +end diff --git a/lib/logflare/ecto/bigquery/bq_repo.ex b/lib/logflare/ecto/bigquery/bq_repo.ex index a3e39db363..3499ded7f0 100644 --- a/lib/logflare/ecto/bigquery/bq_repo.ex +++ b/lib/logflare/ecto/bigquery/bq_repo.ex @@ -18,7 +18,7 @@ defmodule Logflare.BqRepo do optional(atom()) => any() } @type query_result :: - {:ok, results()} | {:error, Tesla.Env.t()} + {:ok, results()} | {:error, Tesla.Env.t() | GenUtils.transport_error()} @spec query_with_sql_and_params( Logflare.User.t(), diff --git a/lib/logflare/google/bigquery/gen_utils.ex b/lib/logflare/google/bigquery/gen_utils.ex index 2194e64c28..90b93271c2 100644 --- a/lib/logflare/google/bigquery/gen_utils.ex +++ b/lib/logflare/google/bigquery/gen_utils.ex @@ -21,6 +21,9 @@ defmodule Logflare.Google.BigQuery.GenUtils do @default_dataset_location "US" @table_ttl 604_800_000 + @type transport_error :: :emfile | :timeout | :closed + @type google_api_result :: {:ok, any()} | {:error, Tesla.Env.t() | transport_error()} + @doc """ Returns the default TTL used (in days) for initializing the table. """ @@ -152,15 +155,14 @@ defmodule Logflare.Google.BigQuery.GenUtils do "#{account_id}" end - @spec maybe_parse_google_api_result({:ok, any()} | {:error, Tesla.Env.t()}) :: - {:ok, any()} | {:error, Tesla.Env.t()} + @spec maybe_parse_google_api_result(google_api_result()) :: google_api_result() def maybe_parse_google_api_result({:error, %Tesla.Env{} = teslaenv}) do {:error, teslaenv} end def maybe_parse_google_api_result(x), do: x - @spec get_tesla_error_message(:emfile | :timeout | :closed | Tesla.Env.t()) :: String.t() + @spec get_tesla_error_message(transport_error() | Tesla.Env.t()) :: String.t() def get_tesla_error_message(%Tesla.Env{} = message) do case JSON.decode(message.body) do {:ok, body} -> diff --git a/lib/logflare_web/live/alerts/alerts_live.ex b/lib/logflare_web/live/alerts/alerts_live.ex index 9aa3e9d64e..7dec5ada87 100644 --- a/lib/logflare_web/live/alerts/alerts_live.ex +++ b/lib/logflare_web/live/alerts/alerts_live.ex @@ -11,9 +11,11 @@ defmodule LogflareWeb.AlertsLive do alias Logflare.Alerting.AlertQuery alias Logflare.Backends alias Logflare.Backends.Adaptor.QueryResult + alias Logflare.Backends.QueryError alias Logflare.Endpoints alias Logflare.Repo alias LogflareWeb.QueryComponents + alias LogflareWeb.QueryErrorHelpers alias LogflareWeb.Utils require Logger @@ -235,7 +237,7 @@ defmodule LogflareWeb.AlertsLive do socket |> put_flash( :error, - "Error when running query: #{inspect(err)}" + "Error when running query: #{format_query_error(err)}" )} end end @@ -266,7 +268,7 @@ defmodule LogflareWeb.AlertsLive do socket |> put_flash( :error, - "Error when running query: #{inspect(err)}" + "Error when running query: #{format_query_error(err)}" )} end end @@ -405,6 +407,10 @@ defmodule LogflareWeb.AlertsLive do |> assign(:modal_node, nil)} end + defp format_query_error(%QueryError{} = error) do + QueryErrorHelpers.query_error_message(error) || error.message + end + def handle_info({:query_string_updated, query_string}, socket) do {:noreply, assign(socket, :query_string, query_string)} end diff --git a/lib/logflare_web/live/search_live/logs_search_lv.ex b/lib/logflare_web/live/search_live/logs_search_lv.ex index c51c0c99fd..0c7754aae6 100644 --- a/lib/logflare_web/live/search_live/logs_search_lv.ex +++ b/lib/logflare_web/live/search_live/logs_search_lv.ex @@ -9,6 +9,7 @@ defmodule LogflareWeb.Source.SearchLV do import LogflareWeb.ModalLiveHelpers import LogflareWeb.SearchLV.Utils + alias Logflare.Backends.QueryError alias Logflare.Billing alias Logflare.Logs.SearchQueryExecutor alias Logflare.Logs.SearchOperations @@ -24,6 +25,7 @@ defmodule LogflareWeb.Source.SearchLV do alias Logflare.User alias Logflare.Users alias LogflareWeb.Helpers.BqSchema, as: BqSchemaHelpers + alias LogflareWeb.QueryErrorHelpers alias LogflareWeb.Router.Helpers, as: Routes alias LogflareWeb.SearchLive.FormComponents alias LogflareWeb.SearchLive.SubheadComponents @@ -686,22 +688,9 @@ defmodule LogflareWeb.Source.SearchLV do |> assign(chart_loading: false) |> put_halt_flash_message(search_op) - %Tesla.Env{status: 400} = err -> - Logger.error("Backend search error for source: #{source.token}", - error_string: inspect(err), - source_id: source.token - ) - - send(self(), :soft_pause) - - socket - |> assign(loading: false) - |> assign(chart_loading: false) - |> put_flash_query_error(err) - err -> Logger.error("Backend search error for source: #{source.token}", - error_string: inspect(err), + error_string: backend_search_error_string(err), source_id: source.token ) @@ -740,6 +729,9 @@ defmodule LogflareWeb.Source.SearchLV do {:noreply, put_flash(socket, type, message)} end + defp backend_search_error_string(%QueryError{raw_error: raw_error}), do: inspect(raw_error) + defp backend_search_error_string(error), do: inspect(error) + defp assign_new_search_with_qs(socket, params, schema_flatmap) do %{querystring: qs, tailing?: tailing?} = params @@ -1110,32 +1102,21 @@ defmodule LogflareWeb.Source.SearchLV do end end - defp put_flash_query_error(socket, %Tesla.Env{status: 400} = response) do - case Jason.decode(response.body) do - {:ok, %{"error" => %{"message" => "Query exceeded limit for bytes billed:" <> rest}}} -> - [limit | _] = String.split(rest, ".") - - {size, units} = limit |> String.trim() |> String.to_integer() |> Utils.humanize_bytes() - - socket - |> put_flash( + defp put_flash_query_error(socket, response) do + with %QueryError{} = error <- response, + message when is_binary(message) and message != "" <- + QueryErrorHelpers.query_error_message(error) do + put_flash(socket, :error, "Query halted: " <> message) + else + _ -> + put_flash( + socket, :error, - "Query halted: total bytes processed for this query is expected to be greater than #{round(size)} #{units}" + "Backend error! Retry your query. Please contact support if this continues." ) - - _ -> - put_flash_query_error(socket, nil) end end - defp put_flash_query_error(socket, _response) do - socket - |> put_flash( - :error, - "Backend error! Retry your query. Please contact support if this continues." - ) - end - defp put_halt_flash_message(socket, search_op) do {:halted, message} = search_op.status diff --git a/lib/logflare_web/query_error_helpers.ex b/lib/logflare_web/query_error_helpers.ex new file mode 100644 index 0000000000..80cc034d2b --- /dev/null +++ b/lib/logflare_web/query_error_helpers.ex @@ -0,0 +1,126 @@ +defmodule LogflareWeb.QueryErrorHelpers do + @moduledoc false + + alias Logflare.Backends.QueryError + alias LogflareWeb.Utils + + @doc """ + Returns a user-facing query error message from a backend %QueryError{}. + + iex> error = %Logflare.Backends.QueryError{ + ...> message: "Unrecognized name: notthere at [1:8]", + ...> code: :invalid_query, + ...> raw_error: %{"message" => "Unrecognized name: notthere at [1:8]"}, + ...> backend: Logflare.Backends.Adaptor.BigQueryAdaptor + ...> } + iex> LogflareWeb.QueryErrorHelpers.query_error_message(error) + ~s(Field "notthere" does not exist.) + + iex> error = %Logflare.Backends.QueryError{ + ...> message: "Code: 47. DB::Exception: Unknown expression identifier `notthere` in scope SELECT notthere. (UNKNOWN_IDENTIFIER)", + ...> code: :invalid_query, + ...> raw_error: %Ch.Error{message: "Code: 47. DB::Exception: Unknown expression identifier `notthere` in scope SELECT notthere. (UNKNOWN_IDENTIFIER)"}, + ...> backend: Logflare.Backends.Adaptor.ClickHouseAdaptor + ...> } + iex> LogflareWeb.QueryErrorHelpers.query_error_message(error) + ~s(Field "notthere" does not exist.) + + iex> error = %Logflare.Backends.QueryError{ + ...> message: ~s|ERROR 42703 (undefined_column) column "notthere" does not exist\\n\\n query: select notthere|, + ...> code: :invalid_query, + ...> raw_error: %Postgrex.Error{message: ~s|column "notthere" does not exist|}, + ...> backend: Logflare.Backends.Adaptor.PostgresAdaptor + ...> } + iex> LogflareWeb.QueryErrorHelpers.query_error_message(error) + ~s(Field "notthere" does not exist.) + """ + @spec query_error_message(QueryError.t()) :: String.t() | nil + def query_error_message(%QueryError{ + backend: Logflare.Backends.Adaptor.BigQueryAdaptor, + raw_error: %{"reason" => "billingTierLimitExceeded"}, + message: message + }) do + with [_match, limit] <- + Regex.run(~r/Query exceeded limit for bytes billed:\s*(\d+)\./, message) do + {size, units} = limit |> String.to_integer() |> Utils.humanize_bytes() + + "total bytes processed for this query is expected to be greater than #{round(size)} #{units}" + end + end + + def query_error_message(%QueryError{backend: backend, message: message}), + do: missing_field_message(backend, message) + + defp missing_field_message(backend, message) do + case extract_missing_field(backend, message) do + nil -> + nil + + field -> + ~s(Field "#{field}" does not exist.) + end + end + + defp extract_missing_field( + Logflare.Backends.Adaptor.BigQueryAdaptor, + "Unrecognized name: " <> rest + ) do + rest + |> first_field_token() + |> normalize_field() + end + + defp extract_missing_field( + Logflare.Backends.Adaptor.BigQueryAdaptor, + "Field name " <> rest + ) do + case String.split(rest, " does not exist", parts: 2) do + [field, _] -> normalize_field(field) + _ -> nil + end + end + + defp extract_missing_field(Logflare.Backends.Adaptor.BigQueryAdaptor, _message) do + nil + end + + defp extract_missing_field(Logflare.Backends.Adaptor.ClickHouseAdaptor, message) do + message + |> extract_field(~r/Unknown (?:expression )?identifier:? [`"']?([^`"'\s,;]+)/) + |> normalize_field() + end + + defp extract_missing_field(Logflare.Backends.Adaptor.PostgresAdaptor, message) do + message + |> extract_field(~r/column\s+["'`]?([^"'`\s]+)["'`]?\s+does not exist/) + |> normalize_path_field() + end + + defp extract_field(message, pattern) do + case Regex.run(pattern, message) do + [_match, field] -> field + nil -> nil + end + end + + defp normalize_field(nil), do: nil + + defp normalize_field(field) do + Regex.replace(~r/^[`"'.]+|[`"'.,]+$/, field, "") + end + + defp first_field_token(field) do + field + |> String.split([" ", ",", ";"], parts: 2) + |> List.first() + end + + defp normalize_path_field(nil), do: nil + + defp normalize_path_field(field) do + field + |> normalize_field() + |> String.split(".") + |> List.last() + end +end diff --git a/test/e2e/features/logs_search_test.exs b/test/e2e/features/logs_search_test.exs index 11282773ed..1794cea225 100644 --- a/test/e2e/features/logs_search_test.exs +++ b/test/e2e/features/logs_search_test.exs @@ -56,6 +56,16 @@ defmodule E2e.Features.LogsSearchTest do |> refute_has("#logs-list-container", text: non_matching_message) end + test "shows a missing field error from the search page", %{conn: conn, source: source} do + conn + |> visit(~p"/auth/login/single_tenant") + |> assert_path(~p"/dashboard") + |> visit(~p"/sources/#{source.id}/search?#{%{querystring: "s:nonexistent"}}") + |> wait_for_selector(".message .alert", state: "attached") + |> assert_has(".message .alert p", text: "nonexistent") + |> assert_has(".message .alert p", text: "does not exist") + end + test "cancelling the datepicker resumes tailing", %{ conn: conn, source: source diff --git a/test/logflare/alerting_test.exs b/test/logflare/alerting_test.exs index 87531590f0..d9f91282cb 100644 --- a/test/logflare/alerting_test.exs +++ b/test/logflare/alerting_test.exs @@ -6,6 +6,7 @@ defmodule Logflare.AlertingTest do alias Logflare.Alerting.AlertQuery alias Logflare.Alerting.AlertWorker alias Logflare.Backends.Adaptor.QueryResult + alias Logflare.Backends.QueryError alias Logflare.Utils.Tasks doctest Logflare.SynEventHandler @@ -270,6 +271,32 @@ defmodule Logflare.AlertingTest do assert reservation == user.bigquery_reservation_alerts end + test "execute_alert_query returns normalized query error message", %{user: user} do + alert_query = insert(:alert, user: user) |> Logflare.Repo.preload([:user]) + + expect(GoogleApi.BigQuery.V2.Api.Jobs, :bigquery_jobs_query, 1, fn _conn, _proj_id, _opts -> + body = + %{ + error: %{ + message: "Unrecognized name: notthere at [1:8]", + reason: "invalidQuery" + } + } + |> Jason.encode!() + + {:error, %Tesla.Env{status: 400, body: body}} + end) + + assert {:error, + %QueryError{ + code: :invalid_query, + backend: Logflare.Backends.Adaptor.BigQueryAdaptor, + message: "Unrecognized name: notthere at [1:8]", + description: nil + }} = + Alerting.execute_alert_query(alert_query) + end + test "execute_alert_query with query composition" do expect(GoogleApi.BigQuery.V2.Api.Jobs, :bigquery_jobs_query, 1, fn _conn, _proj_id, opts -> assert opts[:body].query =~ "current_datetime" diff --git a/test/logflare/backends/adaptor/bigquery_adaptor_test.exs b/test/logflare/backends/adaptor/bigquery_adaptor_test.exs index 3c89166200..1363a6b34d 100644 --- a/test/logflare/backends/adaptor/bigquery_adaptor_test.exs +++ b/test/logflare/backends/adaptor/bigquery_adaptor_test.exs @@ -9,6 +9,7 @@ defmodule Logflare.Backends.Adaptor.BigQueryAdaptorTest do alias Logflare.Backends.Backend alias Logflare.Backends.Adaptor.BigQueryAdaptor alias Logflare.Backends.Adaptor.QueryResult + alias Logflare.Backends.QueryError # Characters illegal in a BigQuery dataset identifier: SQL delimiters, # identifier-quoting characters, whitespace, and shell metacharacters. @@ -218,6 +219,81 @@ defmodule Logflare.Backends.Adaptor.BigQueryAdaptorTest do }} = result end + + test "execute_query translates errors to QueryError", %{user: user} do + stub(GoogleApi.BigQuery.V2.Api.Jobs, :bigquery_jobs_query, fn _conn, _proj_id, _opts -> + {:error, + TestUtils.gen_bq_error("Unrecognized name: notthere at [1:8]", + reason: "invalidQuery" + )} + end) + + assert {:error, + %QueryError{ + code: :invalid_query, + backend: Logflare.Backends.Adaptor.BigQueryAdaptor, + message: "Unrecognized name: notthere at [1:8]", + description: nil, + raw_error: %{ + "message" => "Unrecognized name: notthere at [1:8]", + "reason" => "invalidQuery" + } + }} = + BigQueryAdaptor.execute_query( + {user.bigquery_project_id || "test-project", user.bigquery_dataset_id, user.id}, + {"select notthere", []}, + [] + ) + end + + test "execute_query normalizes bytes billed limit errors", %{user: user} do + stub(GoogleApi.BigQuery.V2.Api.Jobs, :bigquery_jobs_query, fn _conn, _proj_id, _opts -> + {:error, + TestUtils.gen_bq_error( + "Query exceeded limit for bytes billed: 2000000000. 20004857600 or higher required.", + reason: "billingTierLimitExceeded" + )} + end) + + assert {:error, + %QueryError{ + code: :invalid_query, + backend: Logflare.Backends.Adaptor.BigQueryAdaptor, + message: + "Query exceeded limit for bytes billed: 2000000000. 20004857600 or higher required.", + description: nil, + raw_error: %{ + "message" => + "Query exceeded limit for bytes billed: 2000000000. 20004857600 or higher required.", + "reason" => "billingTierLimitExceeded" + } + }} = + BigQueryAdaptor.execute_query( + {user.bigquery_project_id || "test-project", user.bigquery_dataset_id, user.id}, + {"select count(*) from logs", []}, + [] + ) + end + + test "execute_query normalizes transport timeout errors", %{user: user} do + stub(GoogleApi.BigQuery.V2.Api.Jobs, :bigquery_jobs_query, fn _conn, _proj_id, _opts -> + {:error, :timeout} + end) + + assert {:error, + %QueryError{ + code: :connection_error, + backend: Logflare.Backends.Adaptor.BigQueryAdaptor, + message: "timeout", + description: nil, + raw_error: :timeout + }} = + BigQueryAdaptor.execute_query( + {user.bigquery_project_id || "test-project", user.bigquery_dataset_id, user.id}, + {"select count(*) from logs", []}, + [] + ) + end end describe "build_base_query_opts reservation" do diff --git a/test/logflare/backends/adaptor/clickhouse_adaptor/provisioner_test.exs b/test/logflare/backends/adaptor/clickhouse_adaptor/provisioner_test.exs index 180eb2dee0..730096bf6c 100644 --- a/test/logflare/backends/adaptor/clickhouse_adaptor/provisioner_test.exs +++ b/test/logflare/backends/adaptor/clickhouse_adaptor/provisioner_test.exs @@ -3,6 +3,7 @@ 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 @@ -87,7 +88,12 @@ defmodule Logflare.Backends.Adaptor.ClickHouseAdaptor.ProvisionerTest do TestUtils.retry_assert(fn -> assert_receive {:DOWN, ^ref, :process, ^pid, - {:shutdown, {:error, "Error executing ClickHouse query"}}}, + {:shutdown, + {:error, + %QueryError{ + code: :connection_error, + backend: Logflare.Backends.Adaptor.ClickHouseAdaptor + }}}}, 5_000 end) end diff --git a/test/logflare/backends/adaptor/clickhouse_adaptor_test.exs b/test/logflare/backends/adaptor/clickhouse_adaptor_test.exs index 248d72d477..ef9f3b2120 100644 --- a/test/logflare/backends/adaptor/clickhouse_adaptor_test.exs +++ b/test/logflare/backends/adaptor/clickhouse_adaptor_test.exs @@ -16,6 +16,7 @@ defmodule Logflare.Backends.Adaptor.ClickHouseAdaptorTest do alias Logflare.Backends.Backend alias Logflare.Backends.Ecto.SqlUtils alias Logflare.Backends.Adaptor.QueryResult + alias Logflare.Backends.QueryError alias Logflare.Lql.BackendTransformer.ClickHouse, as: ClickHouseLQLTransformer alias Logflare.Lql.Rules.FilterRule @@ -87,7 +88,37 @@ defmodule Logflare.Backends.Adaptor.ClickHouseAdaptorTest do result = ClickHouseAdaptor.execute_ch_query(backend, "INVALID SQL QUERY") - assert {:error, _} = result + assert {:error, %QueryError{} = error} = result + assert error.backend == ClickHouseAdaptor + assert error.code in [:invalid_query, :connection_error] + + if error.code == :invalid_query do + assert %Ch.Error{} = error.raw_error + assert error.description == nil + end + end + + test "normalizes missing field query errors", %{backend: backend} do + expect(Ch, :query, fn _pool, _statement, _params, _opts -> + {:error, + %Ch.Error{ + message: + "Code: 47. DB::Exception: Unknown expression identifier `notthere` in scope SELECT notthere. (UNKNOWN_IDENTIFIER)" + }} + end) + + result = + ClickHouseAdaptor.execute_ch_query(backend, "SELECT notthere") + + assert {:error, + %QueryError{ + code: :invalid_query, + backend: Logflare.Backends.Adaptor.ClickHouseAdaptor, + message: + "Code: 47. DB::Exception: Unknown expression identifier `notthere` in scope SELECT notthere. (UNKNOWN_IDENTIFIER)", + raw_error: %Ch.Error{}, + description: nil + }} = result end test "logs a warning when connection checkout is slow", %{backend: backend} do diff --git a/test/logflare/backends/adaptor/postgres_adaptor_test.exs b/test/logflare/backends/adaptor/postgres_adaptor_test.exs index cd1ca3e510..6b6af941d8 100644 --- a/test/logflare/backends/adaptor/postgres_adaptor_test.exs +++ b/test/logflare/backends/adaptor/postgres_adaptor_test.exs @@ -9,6 +9,7 @@ defmodule Logflare.Backends.Adaptor.PostgresAdaptorTest do alias Logflare.Backends.Adaptor.PostgresAdaptor.SharedRepo alias Logflare.Backends.AdaptorSupervisor alias Logflare.Backends.Adaptor.QueryResult + alias Logflare.Backends.QueryError alias Logflare.Endpoints alias Logflare.SystemMetrics.AllLogsLogged @@ -95,6 +96,73 @@ defmodule Logflare.Backends.Adaptor.PostgresAdaptorTest do ) end + test "execute_query/3 normalizes Ecto query undefined column errors", %{ + backend: backend, + source: source + } do + log_event = build(:log_event, source: source, test: "data") + + assert {:ok, _} = Backends.ingest_logs([log_event], source) + + query = + from(l in PostgresAdaptor.table_name(source), + select: field(l, :notthere) + ) + + TestUtils.retry_assert(fn -> + assert {:error, + %QueryError{ + code: :invalid_query, + backend: Logflare.Backends.Adaptor.PostgresAdaptor, + message: message, + raw_error: %Postgrex.Error{}, + description: nil + }} = PostgresAdaptor.execute_query(backend, query, []) + + assert message =~ "notthere" + end) + end + + test "execute_query/3 normalizes raw SQL undefined column errors", %{ + backend: backend, + source: source + } do + log_event = build(:log_event, source: source, test: "data") + + assert {:ok, _} = Backends.ingest_logs([log_event], source) + + query = "select notthere from #{PostgresAdaptor.table_name(source)}" + + TestUtils.retry_assert(fn -> + assert {:error, + %QueryError{ + code: :invalid_query, + backend: Logflare.Backends.Adaptor.PostgresAdaptor, + message: message, + raw_error: %Postgrex.Error{}, + description: nil + }} = PostgresAdaptor.execute_query(backend, query, []) + + assert message =~ "notthere" + end) + end + + test "execute_query/3 normalizes raw SQL syntax errors", %{backend: backend} do + TestUtils.retry_assert(fn -> + assert {:error, + %QueryError{ + code: :invalid_query, + backend: Logflare.Backends.Adaptor.PostgresAdaptor, + message: message, + raw_error: %Postgrex.Error{}, + description: nil + }} = PostgresAdaptor.execute_query(backend, "select from", []) + + assert message =~ "syntax_error" + assert message =~ "syntax error" + end) + end + test "ingest/2 and execute_query/2 dispatched message with metadata transformation into list", %{ backend: backend, diff --git a/test/logflare/backends/query_error_test.exs b/test/logflare/backends/query_error_test.exs new file mode 100644 index 0000000000..6d1e904c27 --- /dev/null +++ b/test/logflare/backends/query_error_test.exs @@ -0,0 +1,19 @@ +defmodule Logflare.Backends.QueryErrorTest do + use ExUnit.Case, async: true + + alias Logflare.Backends.QueryError + + describe "JSON encoding" do + test "encodes only the public message" do + error = %QueryError{ + message: "raw backend message", + code: :invalid_query, + raw_error: %{"message" => "raw backend message"}, + backend: Logflare.Backends.Adaptor.BigQueryAdaptor, + description: "user-facing description" + } + + assert Jason.encode!(error) == ~s({"message":"raw backend message"}) + end + end +end diff --git a/test/logflare/endpoints/cache_test.exs b/test/logflare/endpoints/cache_test.exs index b83d96921a..677a9c96ea 100644 --- a/test/logflare/endpoints/cache_test.exs +++ b/test/logflare/endpoints/cache_test.exs @@ -1,6 +1,7 @@ defmodule Logflare.Endpoints.CacheTest do use Logflare.DataCase + alias Logflare.Backends.QueryError alias Logflare.Endpoints describe "cache behavior" do @@ -57,7 +58,13 @@ defmodule Logflare.Endpoints.CacheTest do {:ok, cache_pid} = start_supervised({Logflare.Endpoints.ResultsCache, {endpoint, %{}, []}}) assert Process.alive?(cache_pid) - assert {:error, %{"message" => :timeout}} = Endpoints.run_cached_query(endpoint) + assert {:error, + %QueryError{ + code: :connection_error, + backend: Logflare.Backends.Adaptor.BigQueryAdaptor, + message: "timeout", + raw_error: :timeout + }} = Endpoints.run_cached_query(endpoint) refute Process.alive?(cache_pid) end @@ -97,7 +104,13 @@ defmodule Logflare.Endpoints.CacheTest do {:ok, cache_pid} = start_supervised({Logflare.Endpoints.ResultsCache, {endpoint, %{}, []}}) assert Process.alive?(cache_pid) - assert {:error, %{"message" => "BQ Error"}} = Endpoints.run_cached_query(endpoint) + assert {:error, + %QueryError{ + code: :invalid_query, + backend: Logflare.Backends.Adaptor.BigQueryAdaptor, + message: "BQ Error", + raw_error: %{"message" => "BQ Error"} + }} = Endpoints.run_cached_query(endpoint) refute Process.alive?(cache_pid) end diff --git a/test/logflare_web/controllers/endpoints_controller_test.exs b/test/logflare_web/controllers/endpoints_controller_test.exs index efe7d60c20..239fe0e711 100644 --- a/test/logflare_web/controllers/endpoints_controller_test.exs +++ b/test/logflare_web/controllers/endpoints_controller_test.exs @@ -30,7 +30,7 @@ defmodule LogflareWeb.EndpointsControllerTest do pid = self() expect(BigQueryJobs, :bigquery_jobs_query, fn _conn, _proj_id, _opts -> - {:error, :failed_request} + {:error, TestUtils.gen_bq_error("failed_request")} end) conn = diff --git a/test/logflare_web/live/alerts/alerts_live_test.exs b/test/logflare_web/live/alerts/alerts_live_test.exs index 8923dcf0ec..1af1877a3e 100644 --- a/test/logflare_web/live/alerts/alerts_live_test.exs +++ b/test/logflare_web/live/alerts/alerts_live_test.exs @@ -413,6 +413,23 @@ defmodule LogflareWeb.AlertsLiveTest do |> render_click() =~ "some error" end + test "missing field errors are displayed with user-facing message", %{ + conn: conn, + alert_query: alert_query + } do + GoogleApi.BigQuery.V2.Api.Jobs + |> expect(:bigquery_jobs_query, 1, fn _conn, _proj_id, _opts -> + {:error, TestUtils.gen_bq_error("Unrecognized name: notthere at [1:8]")} + end) + + {:ok, view, _html} = live_with_redirect(conn, ~p"/alerts/#{alert_query.id}") + + assert view + |> element("button", "Run query") + |> render_click() =~ + "Field "notthere" does not exist." + end + test "test query from edit page uses the submitted query", %{ conn: conn, alert_query: alert_query diff --git a/test/logflare_web/live/log_event_live_test.exs b/test/logflare_web/live/log_event_live_test.exs index ac8d57a8bc..67426d6a4e 100644 --- a/test/logflare_web/live/log_event_live_test.exs +++ b/test/logflare_web/live/log_event_live_test.exs @@ -96,7 +96,7 @@ defmodule LogflareWeb.LogEventLiveTest do le = build(:log_event, message: "some err message") expect(GoogleApi.BigQuery.V2.Api.Jobs, :bigquery_jobs_query, 1, fn _conn, _proj_id, _opts -> - {:error, "some error"} + {:error, TestUtils.gen_bq_error("Unrecognized name: notthere at [1:8]")} end) logs = @@ -115,6 +115,6 @@ defmodule LogflareWeb.LogEventLiveTest do end) assert logs =~ "Error loading log event" - assert logs =~ "some error" + assert logs =~ "Unrecognized name: notthere" end end diff --git a/test/logflare_web/live/search_live/logs_search_lv_test.exs b/test/logflare_web/live/search_live/logs_search_lv_test.exs index 0d8f2d772e..3cc2a83e39 100644 --- a/test/logflare_web/live/search_live/logs_search_lv_test.exs +++ b/test/logflare_web/live/search_live/logs_search_lv_test.exs @@ -8,6 +8,10 @@ defmodule LogflareWeb.Source.SearchLVTest do alias GoogleApi.BigQuery.V2.Model.TableSchema, as: TS alias GoogleApi.BigQuery.V2.Model.TableFieldSchema, as: TFS alias Logflare.Backends + alias Logflare.Backends.Adaptor.BigQueryAdaptor + alias Logflare.Backends.Adaptor.ClickHouseAdaptor + alias Logflare.Backends.Adaptor.PostgresAdaptor + alias Logflare.Backends.QueryError alias Logflare.Google.BigQuery.SchemaUtils alias Logflare.SingleTenant alias Logflare.Sources.Source.BigQuery.Schema @@ -1368,21 +1372,152 @@ defmodule LogflareWeb.Source.SearchLVTest do %{executor_pid: search_executor_pid} = get_view_assigns(view) allow_sandbox(search_executor_pid) - error_response = - %{ - error: %{ - message: - "Query exceeded limit for bytes billed: 2000000000. 20004857600 or higher required." - } - } - |> Jason.encode!() + message = + "Query exceeded limit for bytes billed: 2000000000. 20004857600 or higher required." - send(view.pid, {:search_error, %{error: %Tesla.Env{status: 400, body: error_response}}}) + send_query_error( + view, + message: message, + backend: BigQueryAdaptor, + raw_error: %{ + "message" => message, + "reason" => "billingTierLimitExceeded" + } + ) assert render(view) =~ "Query halted: total bytes processed for this query is expected to be greater than 2 GB" end + test "shows user-facing error for backend missing field errors", %{ + conn: conn, + source: source + } do + assert {:ok, view, _html} = + live_with_redirect( + conn, + Routes.live_path(conn, SearchLV, source, querystring: "t:20022") + ) + + %{executor_pid: search_executor_pid} = get_view_assigns(view) + allow_sandbox(search_executor_pid) + + message = "Unrecognized name: notthere at [1:8]" + + send_query_error( + view, + message: message, + backend: BigQueryAdaptor, + raw_error: %{"message" => message} + ) + + assert render(view) =~ + "Query halted: Field "notthere" does not exist." + end + + test "shows user-facing error for BigQuery nested missing field errors", %{ + conn: conn, + source: source + } do + assert {:ok, view, _html} = + live_with_redirect( + conn, + Routes.live_path(conn, SearchLV, source, querystring: "t:20022") + ) + + %{executor_pid: search_executor_pid} = get_view_assigns(view) + allow_sandbox(search_executor_pid) + + message = "Field name nonexistent does not exist in STRUCT at [1:42]" + + send_query_error( + view, + message: message, + backend: BigQueryAdaptor, + raw_error: %{"message" => message} + ) + + assert render(view) =~ + "Query halted: Field "nonexistent" does not exist." + end + + test "shows user-facing error for ClickHouse missing field errors", %{ + conn: conn, + source: source + } do + assert {:ok, view, _html} = + live_with_redirect( + conn, + Routes.live_path(conn, SearchLV, source, querystring: "t:20022") + ) + + %{executor_pid: search_executor_pid} = get_view_assigns(view) + allow_sandbox(search_executor_pid) + + message = + "Code: 47. DB::Exception: Unknown expression identifier `notthere` in scope SELECT notthere. (UNKNOWN_IDENTIFIER) (version 26.2.19.43 (official build))\n" + + send_query_error( + view, + message: message, + backend: ClickHouseAdaptor, + raw_error: %Ch.Error{message: message} + ) + + assert render(view) =~ + "Query halted: Field "notthere" does not exist." + end + + test "shows user-facing error for Postgres missing field errors", %{ + conn: conn, + source: source + } do + assert {:ok, view, _html} = + live_with_redirect( + conn, + Routes.live_path(conn, SearchLV, source, querystring: "t:20022") + ) + + %{executor_pid: search_executor_pid} = get_view_assigns(view) + allow_sandbox(search_executor_pid) + + send_query_error( + view, + message: + ~s|ERROR 42703 (undefined_column) column "notthere" does not exist\n\n query: select notthere|, + backend: PostgresAdaptor, + raw_error: %Postgrex.Error{message: ~s|column "notthere" does not exist|} + ) + + assert render(view) =~ + "Query halted: Field "notthere" does not exist." + end + + test "shows generic backend error for unclassified query errors", %{ + conn: conn, + source: source + } do + assert {:ok, view, _html} = + live_with_redirect( + conn, + Routes.live_path(conn, SearchLV, source, querystring: "t:20022") + ) + + %{executor_pid: search_executor_pid} = get_view_assigns(view) + allow_sandbox(search_executor_pid) + + send_query_error( + view, + message: "raw backend syntax error", + backend: BigQueryAdaptor, + raw_error: %RuntimeError{message: "raw backend syntax error"}, + description: nil + ) + + assert render(view) =~ + "Backend error! Retry your query. Please contact support if this continues." + end + test "redirected for non-owner user", %{conn: conn, source: source} do non_owner_user = insert(:user) non_owner_team = insert(:team, user: non_owner_user) @@ -2281,6 +2416,13 @@ defmodule LogflareWeb.Source.SearchLVTest do end end + defp send_query_error(view, attrs) do + attrs = Keyword.put_new(attrs, :code, :invalid_query) + error = struct!(QueryError, attrs) + + send(view.pid, {:search_error, %{error: error}}) + end + defp wait_for_search_completed(prev_completed_at, timeout \\ 2_000) do receive do {:wait_for_render, %{last_query_completed_at: completed_at}} diff --git a/test/logflare_web/query_error_helpers_test.exs b/test/logflare_web/query_error_helpers_test.exs new file mode 100644 index 0000000000..f2586169c1 --- /dev/null +++ b/test/logflare_web/query_error_helpers_test.exs @@ -0,0 +1,5 @@ +defmodule LogflareWeb.QueryErrorHelpersTest do + use ExUnit.Case, async: true + + doctest LogflareWeb.QueryErrorHelpers +end diff --git a/test/support/test_utils.ex b/test/support/test_utils.ex index 96cad047a2..56b9ab84f6 100644 --- a/test/support/test_utils.ex +++ b/test/support/test_utils.ex @@ -148,10 +148,15 @@ defmodule Logflare.TestUtils do end) ``` """ - def gen_bq_error(err) do + def gen_bq_error(err, attrs \\ []) do + error = + attrs + |> Enum.into(%{}, fn {key, value} -> {to_string(key), value} end) + |> Map.put("message", err) + %Tesla.Env{ status: 400, - body: Jason.encode!(%{error: %{message: err}}) + body: Jason.encode!(%{error: error}) } end From 4bad81e262896a2d442c6cfd1436c4281d54bece Mon Sep 17 00:00:00 2001 From: Matt Stubbs <22266+msmithstubbs@users.noreply.github.com> Date: Thu, 4 Jun 2026 16:57:53 +1000 Subject: [PATCH 2/4] Use fallback error message in AlertsLive Co-authored-by: depthfirst-app[bot] <184448029+depthfirst-app[bot]@users.noreply.github.com> --- lib/logflare_web/live/alerts/alerts_live.ex | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/logflare_web/live/alerts/alerts_live.ex b/lib/logflare_web/live/alerts/alerts_live.ex index 7dec5ada87..0e076489a0 100644 --- a/lib/logflare_web/live/alerts/alerts_live.ex +++ b/lib/logflare_web/live/alerts/alerts_live.ex @@ -408,7 +408,9 @@ defmodule LogflareWeb.AlertsLive do end defp format_query_error(%QueryError{} = error) do - QueryErrorHelpers.query_error_message(error) || error.message + QueryErrorHelpers.query_error_message(error) || + "Backend error! Retry your query. Please contact support if this continues." + end end def handle_info({:query_string_updated, query_string}, socket) do From d847e2af1da0d370262988a5598251cbc98b0748 Mon Sep 17 00:00:00 2001 From: Matt Stubbs Date: Fri, 5 Jun 2026 10:36:02 +1000 Subject: [PATCH 3/4] move query error logging into backend adaptor --- lib/logflare/alerting.ex | 106 +++++++-------- .../backends/adaptor/bigquery_adaptor.ex | 87 +++++++++---- .../backends/adaptor/clickhouse_adaptor.ex | 75 ++++++----- .../backends/adaptor/postgres_adaptor.ex | 79 ++++++++---- lib/logflare/backends/query_error.ex | 39 ++++-- lib/logflare/endpoints.ex | 5 +- lib/logflare/logs/search_operations.ex | 26 +++- lib/logflare/sql.ex | 4 + lib/logflare/utils/logger_metadata.ex | 16 +++ .../controllers/api/backend_controller.ex | 4 +- .../controllers/api/fallback_controller.ex | 9 ++ .../controllers/endpoints_controller.ex | 11 +- lib/logflare_web/live/alerts/alerts_live.ex | 8 +- lib/logflare_web/live/backends/components.ex | 16 ++- .../live/endpoints/endpoints_live.ex | 5 +- lib/logflare_web/live/query_live.ex | 5 +- .../search_live/event_context_component.ex | 9 +- .../live/search_live/logs_search_lv.ex | 29 ++--- lib/logflare_web/query_error_helpers.ex | 66 ++++++++-- test/logflare/alerting_test.exs | 57 +++++--- .../adaptor/bigquery_adaptor_test.exs | 36 ++++-- .../clickhouse_adaptor/provisioner_test.exs | 2 +- .../adaptor/clickhouse_adaptor_test.exs | 32 +++-- .../adaptor/postgres_adaptor_test.exs | 28 ++-- test/logflare/backends/query_error_test.exs | 88 ++++++++++++- .../backends/user_monitoring_test.exs | 66 ++++++++++ test/logflare/endpoints/cache_test.exs | 6 +- test/logflare/sql_test.exs | 7 + test/logflare/utils/logger_metadata_test.exs | 36 ++++++ .../api/backend_controller_test.exs | 2 +- .../controllers/api/query_controller_test.exs | 9 +- .../controllers/endpoints_controller_test.exs | 7 +- .../live/alerts/alerts_live_test.exs | 42 +++--- .../live/backends/components_test.exs | 8 +- .../live/search_live/logs_search_lv_test.exs | 9 +- .../live_views/endpoints_live_test.exs | 26 ++++ .../live_views/query_live_test.exs | 19 +++ .../logflare_web/query_error_helpers_test.exs | 122 ++++++++++++++++++ 38 files changed, 889 insertions(+), 312 deletions(-) create mode 100644 lib/logflare/utils/logger_metadata.ex create mode 100644 test/logflare/utils/logger_metadata_test.exs diff --git a/lib/logflare/alerting.ex b/lib/logflare/alerting.ex index 6a5de5f0ca..0368b1a6bf 100644 --- a/lib/logflare/alerting.ex +++ b/lib/logflare/alerting.ex @@ -15,7 +15,6 @@ defmodule Logflare.Alerting do alias Logflare.Backends.Adaptor.SlackAdaptor alias Logflare.Backends.Adaptor.WebhookAdaptor alias Logflare.Backends.Adaptor.QueryResult - alias Logflare.Backends.QueryError alias Logflare.Cluster alias Logflare.Endpoints alias Logflare.Google.BigQuery.GCPConfig @@ -23,6 +22,7 @@ defmodule Logflare.Alerting do alias Logflare.Teams alias Logflare.TeamUsers.TeamUser alias Logflare.User + alias Logflare.Utils.LoggerMetadata require Logger require OpenTelemetry.Tracer @@ -431,63 +431,51 @@ defmodule Logflare.Alerting do @spec execute_alert_query(AlertQuery.t(), use_query_cache: boolean) :: {:ok, QueryResult.t()} | {:error, any()} def execute_alert_query(%AlertQuery{user: %User{}} = alert_query, opts \\ []) do - Logger.debug("Executing AlertQuery | #{alert_query.name} | #{alert_query.id}") - - endpoints = Endpoints.list_endpoints_by(user_id: alert_query.user_id) - use_query_cache = Keyword.get(opts, :use_query_cache, true) - - alerts = - list_alert_queries_by_user_id(alert_query.user_id) - |> Enum.filter(&(&1.id != alert_query.id)) - - with {:ok, expanded_query} <- - Logflare.Sql.expand_subqueries( - alert_query.language, - alert_query.query, - endpoints ++ alerts - ), - {:ok, transformed_query} <- - Logflare.Sql.transform(alert_query.language, expanded_query, alert_query.user_id), - {:ok, result} <- - BigQueryAdaptor.execute_query( - { - alert_query.user.bigquery_project_id || GCPConfig.default_project_id(), - alert_query.user.bigquery_dataset_id, - alert_query.user.id - }, - {transformed_query, []}, - parameterMode: "NAMED", - maxResults: 1000, - location: alert_query.user.bigquery_dataset_location, - use_query_cache: use_query_cache, - labels: %{ - "alert_id" => alert_query.id - }, - query_type: :alerts - ) do - {:ok, result} - else - {:error, %QueryError{} = error} -> - Logger.error("Alert query execution failed with bad response", - user_id: alert_query.user_id, - alert_query_id: alert_query.id, - alert_name: alert_query.name, - possible_reservation_error: BigQueryAdaptor.reservation_error?(error.raw_error), - error_code: error.code, - error_string: inspect(error.message) - ) - - {:error, error} - - {:error, error} -> - Logger.error("Alert query execution failed with an unknown error", - user_id: alert_query.user_id, - alert_query_id: alert_query.id, - alert_name: alert_query.name, - error_string: inspect(error) - ) - - {:error, error} - end + LoggerMetadata.with_metadata(alert_query_logger_metadata(alert_query), fn -> + Logger.debug("Executing AlertQuery | #{alert_query.name} | #{alert_query.id}") + + endpoints = Endpoints.list_endpoints_by(user_id: alert_query.user_id) + use_query_cache = Keyword.get(opts, :use_query_cache, true) + + alerts = + list_alert_queries_by_user_id(alert_query.user_id) + |> Enum.filter(&(&1.id != alert_query.id)) + + with {:ok, expanded_query} <- + Logflare.Sql.expand_subqueries( + alert_query.language, + alert_query.query, + endpoints ++ alerts + ), + {:ok, transformed_query} <- + Logflare.Sql.transform(alert_query.language, expanded_query, alert_query.user_id), + {:ok, result} <- + BigQueryAdaptor.execute_query( + { + alert_query.user.bigquery_project_id || GCPConfig.default_project_id(), + alert_query.user.bigquery_dataset_id, + alert_query.user.id + }, + {transformed_query, []}, + parameterMode: "NAMED", + maxResults: 1000, + location: alert_query.user.bigquery_dataset_location, + use_query_cache: use_query_cache, + labels: %{ + "alert_id" => alert_query.id + }, + query_type: :alerts + ) do + {:ok, result} + end + end) + end + + defp alert_query_logger_metadata(%AlertQuery{} = alert_query) do + [ + user_id: alert_query.user_id, + alert_query_id: alert_query.id, + alert_name: alert_query.name + ] end end diff --git a/lib/logflare/backends/adaptor/bigquery_adaptor.ex b/lib/logflare/backends/adaptor/bigquery_adaptor.ex index 200c2e238b..7999d496cd 100644 --- a/lib/logflare/backends/adaptor/bigquery_adaptor.ex +++ b/lib/logflare/backends/adaptor/bigquery_adaptor.ex @@ -706,35 +706,76 @@ defmodule Logflare.Backends.Adaptor.BigQueryAdaptor do })} {:error, error} -> - maybe_warn_reservation_error(error, user, project_id, query_opts) + query_error = + error + |> to_query_error(user.id) + |> QueryError.log( + user_id: user.id, + bigquery_project_id: project_id + ) - {:error, to_query_error(error, user.id)} + maybe_warn_reservation_error(query_error, user, project_id, query_opts) + + {:error, query_error} end end - @spec to_query_error(term(), pos_integer()) :: QueryError.t() + @spec to_query_error(Tesla.Env.t() | GenUtils.transport_error(), pos_integer()) :: + QueryError.t() defp to_query_error(error, _user_id) when error in [:timeout, :closed, :emfile] do %QueryError{ - message: GenUtils.get_tesla_error_message(error), - code: :connection_error, + kind: :connection_error, raw_error: error, - backend: Logflare.Backends.Adaptor.BigQueryAdaptor + backend: __MODULE__ } end - defp to_query_error(%{body: body}, user_id) do - with %{"error" => raw_error} <- Jason.decode!(body), - %{"message" => message} = processed_error <- + defp to_query_error(%{body: body}, user_id) when is_binary(body) do + with {:ok, %{"error" => raw_error}} <- Jason.decode(body), + %{"message" => _message} = processed_error <- GenUtils.process_bq_errors(raw_error, user_id) do - %QueryError{ - message: message, - code: :invalid_query, - raw_error: processed_error, - backend: Logflare.Backends.Adaptor.BigQueryAdaptor - } + processed_error + |> query_error_kind() + |> query_error(processed_error) + else + _error -> query_error(:backend_error, body) + end + end + + defp to_query_error(%{body: body}, _user_id) do + query_error(:backend_error, body) + end + + @spec query_error_kind(map()) :: QueryError.kind() + defp query_error_kind(%{"reason" => "billingTierLimitExceeded"}), do: :backend_error + defp query_error_kind(%{"reason" => "invalidQuery"}), do: :invalid_query + + defp query_error_kind(%{"errors" => errors}) when is_list(errors) do + if Enum.any?(errors, &match?(%{"reason" => "invalidQuery"}, &1)) do + :invalid_query + else + :backend_error end end + defp query_error_kind(processed_error) do + with %{"message" => message} when is_binary(message) <- processed_error, + true <- String.starts_with?(message, ["Unrecognized name:", "Field name"]) do + :invalid_query + else + _ -> :backend_error + end + end + + @spec query_error(QueryError.kind(), term()) :: QueryError.t() + defp query_error(kind, raw_error) do + %QueryError{ + kind: kind, + raw_error: raw_error, + backend: __MODULE__ + } + end + @spec pg_sql_to_bq_sql(sql :: String.t()) :: String.t() defp pg_sql_to_bq_sql(sql) when is_non_empty_binary(sql) do sql @@ -758,29 +799,31 @@ defmodule Logflare.Backends.Adaptor.BigQueryAdaptor do end @spec maybe_warn_reservation_error( - error :: any(), + error :: QueryError.t(), user :: User.t(), project_id :: String.t(), query_opts :: Keyword.t() ) :: :ok - defp maybe_warn_reservation_error(%{body: body}, %User{} = user, project_id, query_opts) do - with %{"error" => decoded} <- Jason.decode!(body), - true <- reservation_error?(decoded), + defp maybe_warn_reservation_error( + %QueryError{raw_error: error}, + %User{} = user, + project_id, + query_opts + ) do + with true <- reservation_error?(error), false <- caller_logs_own_errors?(query_opts) do Logger.warning("Possible BigQuery reservation error", user_id: user.id, project_id: project_id, reservation: Keyword.get(query_opts, :reservation), query_type: Keyword.get(query_opts, :query_type), - bq_error_message: decoded["message"] + bq_error_message: error["message"] ) end :ok end - defp maybe_warn_reservation_error(_error, %User{}, _project_id, _query_opts), do: :ok - @spec caller_logs_own_errors?(query_opts :: Keyword.t()) :: boolean() defp caller_logs_own_errors?(query_opts) do Keyword.get(query_opts, :query_type) == :alerts diff --git a/lib/logflare/backends/adaptor/clickhouse_adaptor.ex b/lib/logflare/backends/adaptor/clickhouse_adaptor.ex index 7f1d388a0f..bd856f6a85 100644 --- a/lib/logflare/backends/adaptor/clickhouse_adaptor.ex +++ b/lib/logflare/backends/adaptor/clickhouse_adaptor.ex @@ -368,26 +368,16 @@ defmodule Logflare.Backends.Adaptor.ClickHouseAdaptor do {:ok, %Ch.Result{} = result} -> {:ok, decode_ch_result(result)} - {:error, %Ch.Error{message: error_msg} = error} when is_non_empty_binary(error_msg) -> - Logger.warning( - "ClickHouse query failed: #{inspect(error_msg)}", - backend_id: backend.id, - host: read_host(backend) - ) - - {:error, to_query_error(error)} - - {:error, %{message: message} = error} when is_non_empty_binary(message) -> - Logger.warning( - "ClickHouse query failed: #{inspect(message)}", - backend_id: backend.id, - host: read_host(backend) - ) - - {:error, to_query_error(error)} - {:error, error} -> - {:error, to_query_error(error)} + {:error, + error + |> to_query_error() + |> QueryError.log( + user_id: backend.user_id, + backend_id: backend.id, + backend_token: backend.token, + host: read_host(backend) + )} end end end @@ -416,30 +406,39 @@ defmodule Logflare.Backends.Adaptor.ClickHouseAdaptor do end @spec to_query_error(term()) :: QueryError.t() - defp to_query_error(%Ch.Error{message: message} = error) when is_non_empty_binary(message) do - %QueryError{ - message: message, - code: :invalid_query, - raw_error: error, - backend: Logflare.Backends.Adaptor.ClickHouseAdaptor - } + defp to_query_error(%Ch.Error{} = error) do + error + |> ch_query_error_kind() + |> query_error(error) end - defp to_query_error(%DBConnection.ConnectionError{message: message} = error) do - %QueryError{ - message: message, - code: :connection_error, - raw_error: error, - backend: Logflare.Backends.Adaptor.ClickHouseAdaptor - } + defp to_query_error(%DBConnection.ConnectionError{} = error) do + query_error(:connection_error, error) end defp to_query_error(error) do + query_error(:backend_error, error) + end + + @spec ch_query_error_kind(term()) :: QueryError.kind() + defp ch_query_error_kind(%Ch.Error{code: code}) when code in [47, 62], do: :invalid_query + + defp ch_query_error_kind(%Ch.Error{message: message}) when is_binary(message) do + if message =~ "UNKNOWN_IDENTIFIER" or message =~ "SYNTAX_ERROR" do + :invalid_query + else + :backend_error + end + end + + defp ch_query_error_kind(%Ch.Error{}), do: :backend_error + + @spec query_error(QueryError.kind(), term()) :: QueryError.t() + defp query_error(kind, raw_error) do %QueryError{ - message: inspect(error), - code: :backend_error, - raw_error: error, - backend: Logflare.Backends.Adaptor.ClickHouseAdaptor + kind: kind, + raw_error: raw_error, + backend: __MODULE__ } end @@ -557,7 +556,7 @@ defmodule Logflare.Backends.Adaptor.ClickHouseAdaptor do Creates one table per log type: `_logs`, `_metrics`, and `_traces`. """ - @spec provision_ingest_tables(Backend.t()) :: :ok | {:error, Exception.t()} + @spec provision_ingest_tables(Backend.t()) :: :ok | {:error, QueryError.t()} def provision_ingest_tables(%Backend{config: config} = backend) do cloud? = clickhouse_cloud?(backend) diff --git a/lib/logflare/backends/adaptor/postgres_adaptor.ex b/lib/logflare/backends/adaptor/postgres_adaptor.ex index 61a581c17b..3f8212cdf3 100644 --- a/lib/logflare/backends/adaptor/postgres_adaptor.ex +++ b/lib/logflare/backends/adaptor/postgres_adaptor.ex @@ -101,7 +101,7 @@ defmodule Logflare.Backends.Adaptor.PostgresAdaptor do {:ok, QueryResult.new(result, pg_meta(result))} rescue error in [Postgrex.Error, DBConnection.ConnectionError, Ecto.QueryError] -> - {:error, to_query_error(error)} + {:error, error |> to_query_error() |> log_query_error(backend)} end end @@ -126,11 +126,8 @@ defmodule Logflare.Backends.Adaptor.PostgresAdaptor do {:ok, QueryResult.new(rows, pg_meta(rows))} else - {:error, :cannot_connect} = error -> - error - {:error, error} -> - {:error, to_query_error(error)} + {:error, error |> to_query_error() |> log_query_error(backend)} end end @@ -224,34 +221,68 @@ defmodule Logflare.Backends.Adaptor.PostgresAdaptor do Map.put(config, :url, updated) end - @spec to_query_error(term()) :: QueryError.t() + @spec to_query_error( + :cannot_connect + | Postgrex.Error.t() + | DBConnection.ConnectionError.t() + | %Ecto.QueryError{} + ) :: QueryError.t() + defp to_query_error(:cannot_connect) do + query_error(:connection_error, :cannot_connect) + end + defp to_query_error(%Postgrex.Error{} = error) do - %QueryError{ - message: Exception.message(error), - code: :invalid_query, - raw_error: error, - backend: Logflare.Backends.Adaptor.PostgresAdaptor - } + error + |> postgres_query_error_kind() + |> query_error(error) end - defp to_query_error(%DBConnection.ConnectionError{message: message} = error) do - %QueryError{ - message: message, - code: :connection_error, - raw_error: error, - backend: Logflare.Backends.Adaptor.PostgresAdaptor - } + defp to_query_error(%DBConnection.ConnectionError{} = error) do + query_error(:connection_error, error) end - defp to_query_error(%Ecto.QueryError{message: message} = error) do + defp to_query_error(%Ecto.QueryError{} = error) do + query_error(:backend_error, error) + end + + @spec postgres_query_error_kind(Postgrex.Error.t()) :: QueryError.kind() + defp postgres_query_error_kind(%Postgrex.Error{postgres: %{code: code}}) + when code in [:undefined_column, :syntax_error], + do: :invalid_query + + defp postgres_query_error_kind(%Postgrex.Error{postgres: %{pg_code: pg_code}}) + when pg_code in ["42703", "42601"], + do: :invalid_query + + defp postgres_query_error_kind(%Postgrex.Error{} = error) do + message = Exception.message(error) + + if message =~ "undefined_column" or message =~ "syntax_error" or + message =~ ~r/column\s+["'`]?([^"'`\s]+)["'`]?\s+does not exist/ do + :invalid_query + else + :backend_error + end + end + + @spec query_error(QueryError.kind(), term()) :: QueryError.t() + defp query_error(kind, raw_error) do %QueryError{ - message: message, - code: :invalid_query, - raw_error: error, - backend: Logflare.Backends.Adaptor.PostgresAdaptor + kind: kind, + raw_error: raw_error, + backend: __MODULE__ } end + @spec log_query_error(QueryError.t(), Backend.t()) :: QueryError.t() + defp log_query_error(%QueryError{} = error, %Backend{} = backend) do + QueryError.log(error, + user_id: backend.user_id, + backend_id: backend.id, + backend_token: backend.token + ) + end + # expose PgRepo functions defdelegate create_repo(backend), to: PgRepo defdelegate table_name(source), to: PgRepo diff --git a/lib/logflare/backends/query_error.ex b/lib/logflare/backends/query_error.ex index b67aba2369..a1ae942789 100644 --- a/lib/logflare/backends/query_error.ex +++ b/lib/logflare/backends/query_error.ex @@ -1,20 +1,37 @@ defmodule Logflare.Backends.QueryError do @moduledoc false - @derive {Jason.Encoder, only: [:message]} - @enforce_keys [:message, :code, :raw_error, :backend] - defstruct [:message, :code, :raw_error, :backend, :description] + require Logger - @type backend :: - Logflare.Backends.Adaptor.BigQueryAdaptor - | Logflare.Backends.Adaptor.ClickHouseAdaptor - | Logflare.Backends.Adaptor.PostgresAdaptor - @type code :: :invalid_query | :connection_error | :backend_error + @enforce_keys [:kind, :raw_error, :backend] + defstruct [:kind, :raw_error, :backend, :description] + + @type kind :: :invalid_query | :connection_error | :backend_error @type t :: %__MODULE__{ - message: String.t(), - code: code(), + kind: kind(), raw_error: term(), - backend: backend(), + backend: module(), description: String.t() | nil } + + @spec log(t(), Keyword.t()) :: t() + def log(error, metadata \\ []) + + def log(%__MODULE__{kind: :invalid_query} = error, metadata) when is_list(metadata) do + error + end + + def log(%__MODULE__{} = error, metadata) when is_list(metadata) do + Logger.error( + "Backend query error", + metadata + |> Keyword.merge( + backend: inspect(error.backend), + error_kind: error.kind, + error_string: inspect(error.raw_error) + ) + ) + + error + end end diff --git a/lib/logflare/endpoints.ex b/lib/logflare/endpoints.ex index 78795fc846..83a6194fbe 100644 --- a/lib/logflare/endpoints.ex +++ b/lib/logflare/endpoints.ex @@ -9,8 +9,9 @@ defmodule Logflare.Endpoints do alias Logflare.Alerting alias Logflare.Alerting.AlertQuery alias Logflare.Backends - alias Logflare.Backends.Backend alias Logflare.Backends.Adaptor.QueryResult + alias Logflare.Backends.Backend + alias Logflare.Backends.QueryError alias Logflare.Endpoints.PiiRedactor alias Logflare.Endpoints.EndpointQuery alias Logflare.Endpoints.Resolver @@ -35,7 +36,7 @@ defmodule Logflare.Endpoints do @typep origin :: User.t() | TeamUser.t() | OauthAccessToken.t() @typep run_query_return :: {:ok, %{required(:rows) => [term()], optional(atom()) => any()}} - | {:error, String.t()} + | {:error, String.t() | QueryError.t()} defguardp is_integer_or_string(value) when is_integer(value) or is_non_empty_binary(value) diff --git a/lib/logflare/logs/search_operations.ex b/lib/logflare/logs/search_operations.ex index df93c5418e..6f4c54387d 100644 --- a/lib/logflare/logs/search_operations.ex +++ b/lib/logflare/logs/search_operations.ex @@ -24,6 +24,7 @@ defmodule Logflare.Logs.SearchOperations do alias Logflare.SourceSchemas alias Logflare.Sources alias Logflare.Utils.Chart, as: ChartUtils + alias Logflare.Utils.LoggerMetadata alias Logflare.Utils.List, as: ListUtils @type chart_period :: :day | :hour | :minute | :second @@ -70,20 +71,31 @@ defmodule Logflare.Logs.SearchOperations do @spec execute_backend_query(SO.t()) :: {:ok, map()} | {:error, term()} defp execute_backend_query(%SO{backend_type: :postgres} = so) do - backend = postgres_backend(so) + source_logger_metadata(so) + |> LoggerMetadata.with_metadata(fn -> + backend = postgres_backend(so) - PostgresAdaptor.execute_query(backend, so.query, query_type: :search) + PostgresAdaptor.execute_query(backend, so.query, query_type: :search) + end) end defp execute_backend_query(%SO{} = so) do bq_project_id = so.source.user.bigquery_project_id || GCPConfig.default_project_id() %{bigquery_dataset_id: dataset_id} = GenUtils.get_bq_user_info(so.source.token) - BigQueryAdaptor.execute_query( - {bq_project_id, dataset_id, so.source.user.id}, - so.query, - query_type: :search - ) + source_logger_metadata(so) + |> LoggerMetadata.with_metadata(fn -> + BigQueryAdaptor.execute_query( + {bq_project_id, dataset_id, so.source.user.id}, + so.query, + query_type: :search + ) + end) + end + + @spec source_logger_metadata(SO.t()) :: Keyword.t() + defp source_logger_metadata(%SO{} = so) do + [source_id: so.source.token, source_token: so.source.token] end @spec put_sql_string(SO.t(), QueryResult.t()) :: SO.t() diff --git a/lib/logflare/sql.ex b/lib/logflare/sql.ex index 5a312b85d7..cd75044f78 100644 --- a/lib/logflare/sql.ex +++ b/lib/logflare/sql.ex @@ -158,6 +158,10 @@ defmodule Logflare.Sql do queries :: [AlertQuery.t() | Endpoints.EndpointQuery.t()] ) :: {:ok, String.t()} | {:error, String.t()} + def expand_subqueries(language, "", queries) + when language in @valid_query_languages and is_list(queries), + do: {:error, "Query cannot be empty"} + def expand_subqueries(_language, input, []), do: {:ok, input} def expand_subqueries(language, input, queries) diff --git a/lib/logflare/utils/logger_metadata.ex b/lib/logflare/utils/logger_metadata.ex new file mode 100644 index 0000000000..6e2572ffce --- /dev/null +++ b/lib/logflare/utils/logger_metadata.ex @@ -0,0 +1,16 @@ +defmodule Logflare.Utils.LoggerMetadata do + @moduledoc false + + @spec with_metadata(Keyword.t(), (-> term())) :: term() + def with_metadata(metadata, fun) when is_list(metadata) and is_function(fun, 0) do + previous_metadata = Logger.metadata() + + Logger.metadata(metadata) + + try do + fun.() + after + Logger.reset_metadata(previous_metadata) + end + end +end diff --git a/lib/logflare_web/controllers/api/backend_controller.ex b/lib/logflare_web/controllers/api/backend_controller.ex index 01395bbab6..31cddb6cd7 100644 --- a/lib/logflare_web/controllers/api/backend_controller.ex +++ b/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} -> conn - |> json(%{connected?: false, reason: reason}) + |> json(%{connected?: false}) end end end diff --git a/lib/logflare_web/controllers/api/fallback_controller.ex b/lib/logflare_web/controllers/api/fallback_controller.ex index 6b3164f1f5..ad648a2c78 100644 --- a/lib/logflare_web/controllers/api/fallback_controller.ex +++ b/lib/logflare_web/controllers/api/fallback_controller.ex @@ -1,6 +1,9 @@ defmodule LogflareWeb.Api.FallbackController do use Phoenix.Controller + alias Ecto.Changeset + alias Logflare.Backends.QueryError + alias LogflareWeb.QueryErrorHelpers def call(conn, {:error, %Changeset{} = changeset}) do errors = Changeset.traverse_errors(changeset, fn _, _, {message, _} -> message end) @@ -45,6 +48,12 @@ defmodule LogflareWeb.Api.FallbackController do |> json(%{error: "Not Found"}) end + def call(conn, {:error, %QueryError{}}) do + conn + |> put_status(400) + |> json(%{error: QueryErrorHelpers.generic_query_error_message()}) + end + def call(conn, {:error, %{} = err_map}) do conn |> put_status(400) diff --git a/lib/logflare_web/controllers/endpoints_controller.ex b/lib/logflare_web/controllers/endpoints_controller.ex index 61a3da7bfe..3a9598d50d 100644 --- a/lib/logflare_web/controllers/endpoints_controller.ex +++ b/lib/logflare_web/controllers/endpoints_controller.ex @@ -3,12 +3,14 @@ defmodule LogflareWeb.EndpointsController do use OpenApiSpex.ControllerSpecs require Logger - alias Logflare.Endpoints + alias Logflare.Backends.QueryError + alias Logflare.Endpoints alias LogflareWeb.JsonParser alias LogflareWeb.OpenApi.Unauthorized alias LogflareWeb.OpenApi.ServerError alias LogflareWeb.OpenApiSchemas.EndpointQuery + alias LogflareWeb.QueryErrorHelpers @plug_parsers_init Plug.Parsers.init( parsers: [JsonParser], @@ -88,8 +90,11 @@ defmodule LogflareWeb.EndpointsController do Logger.debug("Endpoint cache result, #{inspect(result, pretty: true)}") render(conn, "query.json", result: result.rows) - {:error, errors} -> - render(conn, "query.json", error: errors) + {:error, error = %QueryError{}} -> + render(conn, "query.json", error: QueryErrorHelpers.query_error_message(error)) + + {:error, _errors} -> + render(conn, "query.json", error: QueryErrorHelpers.generic_query_error_message()) end end diff --git a/lib/logflare_web/live/alerts/alerts_live.ex b/lib/logflare_web/live/alerts/alerts_live.ex index 0e076489a0..03b407f195 100644 --- a/lib/logflare_web/live/alerts/alerts_live.ex +++ b/lib/logflare_web/live/alerts/alerts_live.ex @@ -408,11 +408,13 @@ defmodule LogflareWeb.AlertsLive do end defp format_query_error(%QueryError{} = error) do - QueryErrorHelpers.query_error_message(error) || - "Backend error! Retry your query. Please contact support if this continues." - end + QueryErrorHelpers.query_error_message(error) end + defp format_query_error(error) when is_binary(error), do: error + + defp format_query_error(_error), do: QueryErrorHelpers.generic_query_error_message() + def handle_info({:query_string_updated, query_string}, socket) do {:noreply, assign(socket, :query_string, query_string)} end diff --git a/lib/logflare_web/live/backends/components.ex b/lib/logflare_web/live/backends/components.ex index 8684fc6bb9..2b6cc507f8 100644 --- a/lib/logflare_web/live/backends/components.ex +++ b/lib/logflare_web/live/backends/components.ex @@ -9,9 +9,11 @@ defmodule LogflareWeb.Backends.Components do ~H""" <.async_result :let={_ok} assign={@status}> <:loading><.indicator icon="spinner" color="tw-text-white" animation="tw-animate-spin" )} /> - <:failed :let={{atom, reason}}> - <% reason = if atom == :error, do: reason, else: "Internal error" %> - <.indicator icon="times" color="tw-text-red-500" )} /> {reason} + <:failed :let={reason}> + <.indicator icon="times" color="tw-text-red-500" )} /> + + {status_error_message(reason)} + <.indicator icon="check" color="tw-text-green-500" )} /> @@ -26,4 +28,12 @@ defmodule LogflareWeb.Backends.Components do """ end + + defp status_error_message({:error, %Logflare.Backends.QueryError{} = query_error}) do + LogflareWeb.QueryErrorHelpers.query_error_message(query_error) + end + + defp status_error_message(_reason) do + LogflareWeb.QueryErrorHelpers.generic_query_error_message() + end end diff --git a/lib/logflare_web/live/endpoints/endpoints_live.ex b/lib/logflare_web/live/endpoints/endpoints_live.ex index feac16d6f3..68596163ef 100644 --- a/lib/logflare_web/live/endpoints/endpoints_live.ex +++ b/lib/logflare_web/live/endpoints/endpoints_live.ex @@ -14,6 +14,7 @@ defmodule LogflareWeb.EndpointsLive do alias Logflare.Endpoints alias Logflare.Endpoints.PiiRedactor alias LogflareWeb.QueryComponents + alias LogflareWeb.QueryErrorHelpers alias Logflare.Utils embed_templates("actions/*", suffix: "_action") @@ -276,9 +277,11 @@ defmodule LogflareWeb.EndpointsLive do |> assign(:total_bytes_processed, nil)} {:error, err} -> + message = if is_binary(err), do: err, else: QueryErrorHelpers.query_error_message(err) + {:noreply, socket - |> put_flash(:error, "Error occured when running query: #{inspect(err)}")} + |> put_flash(:error, "Error occured running query: #{message}")} end end diff --git a/lib/logflare_web/live/query_live.ex b/lib/logflare_web/live/query_live.ex index 091cf62725..7d01a90185 100644 --- a/lib/logflare_web/live/query_live.ex +++ b/lib/logflare_web/live/query_live.ex @@ -11,6 +11,7 @@ defmodule LogflareWeb.QueryLive do alias Logflare.Teams.TeamContext alias LogflareWeb.AuthLive alias LogflareWeb.QueryComponents + alias LogflareWeb.QueryErrorHelpers alias LogflareWeb.Utils def render(assigns) do @@ -308,8 +309,10 @@ defmodule LogflareWeb.QueryLive do |> assign(:total_bytes_processed, total_bytes_processed) {:error, err} -> + message = if is_binary(err), do: err, else: QueryErrorHelpers.query_error_message(err) + socket - |> put_flash(:error, "Error occurred when running query: #{inspect(err)}") + |> put_flash(:error, "Error occurred running query: #{message}") end end end diff --git a/lib/logflare_web/live/search_live/event_context_component.ex b/lib/logflare_web/live/search_live/event_context_component.ex index 108099015c..f678c0e327 100644 --- a/lib/logflare_web/live/search_live/event_context_component.ex +++ b/lib/logflare_web/live/search_live/event_context_component.ex @@ -9,8 +9,6 @@ defmodule LogflareWeb.SearchLive.EventContextComponent do alias Logflare.Sources.Source.BigQuery.SchemaBuilder import LogflareWeb.SearchLive.LogEventComponents, only: [log_event: 1] - require Logger - @impl true def update(assigns, socket) do %{ @@ -94,12 +92,7 @@ defmodule LogflareWeb.SearchLive.EventContextComponent do |> stream(:log_events, events, reset: true)} end - def handle_async(:logs, {:ok, %{error: error, source: source}}, socket) do - Logger.error("Backend context search error for source: #{source.token}", - error_string: inspect(error), - source_id: source.token - ) - + def handle_async(:logs, {:ok, %{error: _error, source: _source}}, socket) do {:noreply, socket |> assign(:logs, AsyncResult.failed(socket.assigns.logs, "An error occurred.")) diff --git a/lib/logflare_web/live/search_live/logs_search_lv.ex b/lib/logflare_web/live/search_live/logs_search_lv.ex index 0c7754aae6..611222faaa 100644 --- a/lib/logflare_web/live/search_live/logs_search_lv.ex +++ b/lib/logflare_web/live/search_live/logs_search_lv.ex @@ -677,7 +677,7 @@ defmodule LogflareWeb.Source.SearchLV do {:noreply, socket} end - def handle_info({:search_error, search_op}, %{assigns: %{source: source}} = socket) do + def handle_info({:search_error, search_op}, socket) do socket = case search_op.error do :halted -> @@ -689,11 +689,6 @@ defmodule LogflareWeb.Source.SearchLV do |> put_halt_flash_message(search_op) err -> - Logger.error("Backend search error for source: #{source.token}", - error_string: backend_search_error_string(err), - source_id: source.token - ) - send(self(), :soft_pause) socket @@ -729,9 +724,6 @@ defmodule LogflareWeb.Source.SearchLV do {:noreply, put_flash(socket, type, message)} end - defp backend_search_error_string(%QueryError{raw_error: raw_error}), do: inspect(raw_error) - defp backend_search_error_string(error), do: inspect(error) - defp assign_new_search_with_qs(socket, params, schema_flatmap) do %{querystring: qs, tailing?: tailing?} = params @@ -1103,18 +1095,13 @@ defmodule LogflareWeb.Source.SearchLV do end defp put_flash_query_error(socket, response) do - with %QueryError{} = error <- response, - message when is_binary(message) and message != "" <- - QueryErrorHelpers.query_error_message(error) do - put_flash(socket, :error, "Query halted: " <> message) - else - _ -> - put_flash( - socket, - :error, - "Backend error! Retry your query. Please contact support if this continues." - ) - end + message = + case response do + %QueryError{} = error -> QueryErrorHelpers.query_error_message(error) + _ -> QueryErrorHelpers.generic_query_error_message() + end + + put_flash(socket, :error, "Query halted: " <> message) end defp put_halt_flash_message(socket, search_op) do diff --git a/lib/logflare_web/query_error_helpers.ex b/lib/logflare_web/query_error_helpers.ex index 80cc034d2b..cff2274d0b 100644 --- a/lib/logflare_web/query_error_helpers.ex +++ b/lib/logflare_web/query_error_helpers.ex @@ -4,12 +4,13 @@ defmodule LogflareWeb.QueryErrorHelpers do alias Logflare.Backends.QueryError alias LogflareWeb.Utils + @generic_query_error_message "Backend error! Retry your query. Please contact support if this continues." + @doc """ Returns a user-facing query error message from a backend %QueryError{}. iex> error = %Logflare.Backends.QueryError{ - ...> message: "Unrecognized name: notthere at [1:8]", - ...> code: :invalid_query, + ...> kind: :invalid_query, ...> raw_error: %{"message" => "Unrecognized name: notthere at [1:8]"}, ...> backend: Logflare.Backends.Adaptor.BigQueryAdaptor ...> } @@ -17,8 +18,7 @@ defmodule LogflareWeb.QueryErrorHelpers do ~s(Field "notthere" does not exist.) iex> error = %Logflare.Backends.QueryError{ - ...> message: "Code: 47. DB::Exception: Unknown expression identifier `notthere` in scope SELECT notthere. (UNKNOWN_IDENTIFIER)", - ...> code: :invalid_query, + ...> kind: :invalid_query, ...> raw_error: %Ch.Error{message: "Code: 47. DB::Exception: Unknown expression identifier `notthere` in scope SELECT notthere. (UNKNOWN_IDENTIFIER)"}, ...> backend: Logflare.Backends.Adaptor.ClickHouseAdaptor ...> } @@ -26,20 +26,25 @@ defmodule LogflareWeb.QueryErrorHelpers do ~s(Field "notthere" does not exist.) iex> error = %Logflare.Backends.QueryError{ - ...> message: ~s|ERROR 42703 (undefined_column) column "notthere" does not exist\\n\\n query: select notthere|, - ...> code: :invalid_query, + ...> kind: :invalid_query, ...> raw_error: %Postgrex.Error{message: ~s|column "notthere" does not exist|}, ...> backend: Logflare.Backends.Adaptor.PostgresAdaptor ...> } iex> LogflareWeb.QueryErrorHelpers.query_error_message(error) ~s(Field "notthere" does not exist.) """ - @spec query_error_message(QueryError.t()) :: String.t() | nil - def query_error_message(%QueryError{ - backend: Logflare.Backends.Adaptor.BigQueryAdaptor, - raw_error: %{"reason" => "billingTierLimitExceeded"}, - message: message - }) do + @spec query_error_message(QueryError.t()) :: String.t() + def query_error_message(%QueryError{} = error) do + classified_query_error_message(error) || generic_query_error_message() + end + + @spec generic_query_error_message() :: String.t() + def generic_query_error_message, do: @generic_query_error_message + + defp classified_query_error_message(%QueryError{ + backend: Logflare.Backends.Adaptor.BigQueryAdaptor, + raw_error: %{"reason" => "billingTierLimitExceeded", "message" => message} + }) do with [_match, limit] <- Regex.run(~r/Query exceeded limit for bytes billed:\s*(\d+)\./, message) do {size, units} = limit |> String.to_integer() |> Utils.humanize_bytes() @@ -48,8 +53,33 @@ defmodule LogflareWeb.QueryErrorHelpers do end end - def query_error_message(%QueryError{backend: backend, message: message}), - do: missing_field_message(backend, message) + defp classified_query_error_message(%QueryError{ + kind: :invalid_query, + backend: backend, + raw_error: raw_error + }) do + case raw_error_message(raw_error) do + message when is_binary(message) -> invalid_query_message(backend, message) + nil -> nil + end + end + + defp classified_query_error_message(%QueryError{}), do: nil + + defp invalid_query_message(Logflare.Backends.Adaptor.BigQueryAdaptor, message) do + case message do + "Query without FROM clause cannot have a WHERE clause" <> _rest -> + message + + _ -> + missing_field_message(Logflare.Backends.Adaptor.BigQueryAdaptor, message) || + generic_query_error_message() + end + end + + defp invalid_query_message(backend, message) do + missing_field_message(backend, message) + end defp missing_field_message(backend, message) do case extract_missing_field(backend, message) do @@ -103,6 +133,14 @@ defmodule LogflareWeb.QueryErrorHelpers do end end + defp raw_error_message(%{"message" => message}) when is_binary(message), do: message + + defp raw_error_message(%Postgrex.Error{postgres: %{message: message}}) when is_binary(message), + do: message + + defp raw_error_message(%{message: message}) when is_binary(message), do: message + defp raw_error_message(_raw_error), do: nil + defp normalize_field(nil), do: nil defp normalize_field(field) do diff --git a/test/logflare/alerting_test.exs b/test/logflare/alerting_test.exs index d9f91282cb..cc8389a5eb 100644 --- a/test/logflare/alerting_test.exs +++ b/test/logflare/alerting_test.exs @@ -2,6 +2,8 @@ defmodule Logflare.AlertingTest do use Logflare.DataCase, async: false use Oban.Testing, repo: Logflare.Repo + import ExUnit.CaptureLog + alias Logflare.Alerting alias Logflare.Alerting.AlertQuery alias Logflare.Alerting.AlertWorker @@ -271,30 +273,47 @@ defmodule Logflare.AlertingTest do assert reservation == user.bigquery_reservation_alerts end - test "execute_alert_query returns normalized query error message", %{user: user} do + test "execute_alert_query logs backend errors with alert metadata", %{ + user: user + } do alert_query = insert(:alert, user: user) |> Logflare.Repo.preload([:user]) expect(GoogleApi.BigQuery.V2.Api.Jobs, :bigquery_jobs_query, 1, fn _conn, _proj_id, _opts -> - body = - %{ - error: %{ - message: "Unrecognized name: notthere at [1:8]", - reason: "invalidQuery" - } - } - |> Jason.encode!() - - {:error, %Tesla.Env{status: 400, body: body}} + {:error, :timeout} end) - assert {:error, - %QueryError{ - code: :invalid_query, - backend: Logflare.Backends.Adaptor.BigQueryAdaptor, - message: "Unrecognized name: notthere at [1:8]", - description: nil - }} = - Alerting.execute_alert_query(alert_query) + previous_metadata = Logger.metadata() + + log = + capture_log( + [level: :error, metadata: [:user_id, :alert_query_id, :alert_name, :error_kind]], + fn -> + assert {:error, + %QueryError{ + kind: :connection_error, + backend: Logflare.Backends.Adaptor.BigQueryAdaptor, + description: nil + }} = + Alerting.execute_alert_query(alert_query) + end + ) + + assert Logger.metadata() == previous_metadata + assert log =~ "Backend query error" + assert log =~ "user_id=#{alert_query.user_id}" + assert log =~ "alert_query_id=#{alert_query.id}" + assert log =~ "alert_name=" + assert log =~ "error_kind=connection_error" + end + + test "execute_alert_query returns an error for empty queries", %{user: user} do + insert(:endpoint, user: user) + + alert_query = + insert(:alert, user: user, query: "") + |> Logflare.Repo.preload([:user]) + + assert {:error, "Query cannot be empty"} = Alerting.execute_alert_query(alert_query) end test "execute_alert_query with query composition" do diff --git a/test/logflare/backends/adaptor/bigquery_adaptor_test.exs b/test/logflare/backends/adaptor/bigquery_adaptor_test.exs index 1363a6b34d..fadd93d76b 100644 --- a/test/logflare/backends/adaptor/bigquery_adaptor_test.exs +++ b/test/logflare/backends/adaptor/bigquery_adaptor_test.exs @@ -228,11 +228,12 @@ defmodule Logflare.Backends.Adaptor.BigQueryAdaptorTest do )} end) + project_id = user.bigquery_project_id || "test-project" + assert {:error, %QueryError{ - code: :invalid_query, + kind: :invalid_query, backend: Logflare.Backends.Adaptor.BigQueryAdaptor, - message: "Unrecognized name: notthere at [1:8]", description: nil, raw_error: %{ "message" => "Unrecognized name: notthere at [1:8]", @@ -240,7 +241,7 @@ defmodule Logflare.Backends.Adaptor.BigQueryAdaptorTest do } }} = BigQueryAdaptor.execute_query( - {user.bigquery_project_id || "test-project", user.bigquery_dataset_id, user.id}, + {project_id, user.bigquery_dataset_id, user.id}, {"select notthere", []}, [] ) @@ -257,10 +258,8 @@ defmodule Logflare.Backends.Adaptor.BigQueryAdaptorTest do assert {:error, %QueryError{ - code: :invalid_query, + kind: :backend_error, backend: Logflare.Backends.Adaptor.BigQueryAdaptor, - message: - "Query exceeded limit for bytes billed: 2000000000. 20004857600 or higher required.", description: nil, raw_error: %{ "message" => @@ -282,9 +281,8 @@ defmodule Logflare.Backends.Adaptor.BigQueryAdaptorTest do assert {:error, %QueryError{ - code: :connection_error, + kind: :connection_error, backend: Logflare.Backends.Adaptor.BigQueryAdaptor, - message: "timeout", description: nil, raw_error: :timeout }} = @@ -294,6 +292,28 @@ defmodule Logflare.Backends.Adaptor.BigQueryAdaptorTest do [] ) end + + test "execute_query maps non-invalid BigQuery reasons as backend errors", %{user: user} do + stub(GoogleApi.BigQuery.V2.Api.Jobs, :bigquery_jobs_query, fn _conn, _proj_id, _opts -> + {:error, TestUtils.gen_bq_error("backend unavailable", reason: "backendError")} + end) + + assert {:error, + %QueryError{ + kind: :backend_error, + backend: Logflare.Backends.Adaptor.BigQueryAdaptor, + description: nil, + raw_error: %{ + "message" => "backend unavailable", + "reason" => "backendError" + } + }} = + BigQueryAdaptor.execute_query( + {user.bigquery_project_id || "test-project", user.bigquery_dataset_id, user.id}, + {"select count(*) from logs", []}, + [] + ) + end end describe "build_base_query_opts reservation" do diff --git a/test/logflare/backends/adaptor/clickhouse_adaptor/provisioner_test.exs b/test/logflare/backends/adaptor/clickhouse_adaptor/provisioner_test.exs index 730096bf6c..a28f253914 100644 --- a/test/logflare/backends/adaptor/clickhouse_adaptor/provisioner_test.exs +++ b/test/logflare/backends/adaptor/clickhouse_adaptor/provisioner_test.exs @@ -91,7 +91,7 @@ defmodule Logflare.Backends.Adaptor.ClickHouseAdaptor.ProvisionerTest do {:shutdown, {:error, %QueryError{ - code: :connection_error, + kind: :connection_error, backend: Logflare.Backends.Adaptor.ClickHouseAdaptor }}}}, 5_000 diff --git a/test/logflare/backends/adaptor/clickhouse_adaptor_test.exs b/test/logflare/backends/adaptor/clickhouse_adaptor_test.exs index ef9f3b2120..96516eb5da 100644 --- a/test/logflare/backends/adaptor/clickhouse_adaptor_test.exs +++ b/test/logflare/backends/adaptor/clickhouse_adaptor_test.exs @@ -90,9 +90,9 @@ defmodule Logflare.Backends.Adaptor.ClickHouseAdaptorTest do assert {:error, %QueryError{} = error} = result assert error.backend == ClickHouseAdaptor - assert error.code in [:invalid_query, :connection_error] + assert error.kind in [:invalid_query, :connection_error] - if error.code == :invalid_query do + if error.kind == :invalid_query do assert %Ch.Error{} = error.raw_error assert error.description == nil end @@ -102,23 +102,37 @@ defmodule Logflare.Backends.Adaptor.ClickHouseAdaptorTest do expect(Ch, :query, fn _pool, _statement, _params, _opts -> {:error, %Ch.Error{ + code: 47, message: "Code: 47. DB::Exception: Unknown expression identifier `notthere` in scope SELECT notthere. (UNKNOWN_IDENTIFIER)" }} end) - result = - ClickHouseAdaptor.execute_ch_query(backend, "SELECT notthere") + assert {:error, + %QueryError{ + kind: :invalid_query, + backend: Logflare.Backends.Adaptor.ClickHouseAdaptor, + raw_error: %Ch.Error{ + code: 47, + message: + "Code: 47. DB::Exception: Unknown expression identifier `notthere` in scope SELECT notthere. (UNKNOWN_IDENTIFIER)" + }, + description: nil + }} = ClickHouseAdaptor.execute_ch_query(backend, "SELECT notthere") + end + + test "normalizes ClickHouse server errors as backend errors", %{backend: backend} do + expect(Ch, :query, fn _pool, _statement, _params, _opts -> + {:error, %Ch.Error{code: 999, message: "Backend server error"}} + end) assert {:error, %QueryError{ - code: :invalid_query, + kind: :backend_error, backend: Logflare.Backends.Adaptor.ClickHouseAdaptor, - message: - "Code: 47. DB::Exception: Unknown expression identifier `notthere` in scope SELECT notthere. (UNKNOWN_IDENTIFIER)", - raw_error: %Ch.Error{}, + raw_error: %Ch.Error{code: 999, message: "Backend server error"}, description: nil - }} = result + }} = ClickHouseAdaptor.execute_ch_query(backend, "SELECT 1") end test "logs a warning when connection checkout is slow", %{backend: backend} do diff --git a/test/logflare/backends/adaptor/postgres_adaptor_test.exs b/test/logflare/backends/adaptor/postgres_adaptor_test.exs index 6b6af941d8..024a11aaae 100644 --- a/test/logflare/backends/adaptor/postgres_adaptor_test.exs +++ b/test/logflare/backends/adaptor/postgres_adaptor_test.exs @@ -112,14 +112,13 @@ defmodule Logflare.Backends.Adaptor.PostgresAdaptorTest do TestUtils.retry_assert(fn -> assert {:error, %QueryError{ - code: :invalid_query, + kind: :invalid_query, backend: Logflare.Backends.Adaptor.PostgresAdaptor, - message: message, raw_error: %Postgrex.Error{}, description: nil - }} = PostgresAdaptor.execute_query(backend, query, []) + } = error} = PostgresAdaptor.execute_query(backend, query, []) - assert message =~ "notthere" + assert Exception.message(error.raw_error) =~ "notthere" end) end @@ -136,14 +135,13 @@ defmodule Logflare.Backends.Adaptor.PostgresAdaptorTest do TestUtils.retry_assert(fn -> assert {:error, %QueryError{ - code: :invalid_query, + kind: :invalid_query, backend: Logflare.Backends.Adaptor.PostgresAdaptor, - message: message, raw_error: %Postgrex.Error{}, description: nil - }} = PostgresAdaptor.execute_query(backend, query, []) + } = error} = PostgresAdaptor.execute_query(backend, query, []) - assert message =~ "notthere" + assert Exception.message(error.raw_error) =~ "notthere" end) end @@ -151,13 +149,13 @@ defmodule Logflare.Backends.Adaptor.PostgresAdaptorTest do TestUtils.retry_assert(fn -> assert {:error, %QueryError{ - code: :invalid_query, + kind: :invalid_query, backend: Logflare.Backends.Adaptor.PostgresAdaptor, - message: message, raw_error: %Postgrex.Error{}, description: nil - }} = PostgresAdaptor.execute_query(backend, "select from", []) + } = error} = PostgresAdaptor.execute_query(backend, "select from", []) + message = Exception.message(error.raw_error) assert message =~ "syntax_error" assert message =~ "syntax error" end) @@ -233,7 +231,13 @@ defmodule Logflare.Backends.Adaptor.PostgresAdaptorTest do {:error, :cannot_connect} end) - assert {:error, :cannot_connect} = PostgresAdaptor.test_connection(backend) + assert {:error, + %QueryError{ + kind: :connection_error, + backend: Logflare.Backends.Adaptor.PostgresAdaptor, + raw_error: :cannot_connect, + description: nil + }} = PostgresAdaptor.test_connection(backend) end end diff --git a/test/logflare/backends/query_error_test.exs b/test/logflare/backends/query_error_test.exs index 6d1e904c27..71adb1800c 100644 --- a/test/logflare/backends/query_error_test.exs +++ b/test/logflare/backends/query_error_test.exs @@ -1,19 +1,97 @@ defmodule Logflare.Backends.QueryErrorTest do use ExUnit.Case, async: true + import ExUnit.CaptureLog + + alias Logflare.Backends.Adaptor.BigQueryAdaptor alias Logflare.Backends.QueryError - describe "JSON encoding" do - test "encodes only the public message" do + describe "struct" do + test "stores backend, kind, and raw error detail" do error = %QueryError{ - message: "raw backend message", - code: :invalid_query, + kind: :invalid_query, raw_error: %{"message" => "raw backend message"}, backend: Logflare.Backends.Adaptor.BigQueryAdaptor, description: "user-facing description" } - assert Jason.encode!(error) == ~s({"message":"raw backend message"}) + assert error.kind == :invalid_query + assert error.raw_error == %{"message" => "raw backend message"} + assert error.backend == BigQueryAdaptor end end + + describe "log/2" do + test "does not log invalid query errors" do + error = query_error(raw_error: %{"message" => "raw user query detail"}) + + log = + capture_log( + [level: :error, metadata: [:user_id, :backend_id, :error_kind, :error_string]], + fn -> + assert ^error = + QueryError.log(error, + user_id: 123, + backend_id: 456, + source_token: nil + ) + end + ) + + assert log == "" + end + + test "logs backend errors with user metadata and raw backend detail" do + error = query_error(kind: :backend_error, raw_error: %{"message" => "raw backend detail"}) + + log = + capture_log( + [level: :error, metadata: [:user_id, :backend_id, :error_kind, :error_string]], + fn -> + assert ^error = + QueryError.log(error, + user_id: 123, + backend_id: 456, + source_token: nil + ) + end + ) + + assert log =~ "Backend query error" + assert log =~ "user_id=123" + assert log =~ "backend_id=456" + assert log =~ "error_kind=backend_error" + assert log =~ "raw backend detail" + refute log =~ "source_token=" + end + + test "logs query errors without requiring user metadata" do + error = query_error(raw_error: :timeout, kind: :connection_error) + + log = + capture_log([level: :error, metadata: [:user_id, :error_kind, :error_string]], fn -> + assert ^error = QueryError.log(error) + end) + + assert log =~ "Backend query error" + assert log =~ "error_kind=connection_error" + assert log =~ "timeout" + refute log =~ "user_id=" + end + end + + defp query_error(attrs) do + attrs = + Keyword.merge( + [ + kind: :invalid_query, + raw_error: %{"message" => "backend failed"}, + backend: BigQueryAdaptor, + description: nil + ], + attrs + ) + + struct!(QueryError, attrs) + end end diff --git a/test/logflare/backends/user_monitoring_test.exs b/test/logflare/backends/user_monitoring_test.exs index 8848c9cbd2..5a3a05c44f 100644 --- a/test/logflare/backends/user_monitoring_test.exs +++ b/test/logflare/backends/user_monitoring_test.exs @@ -8,6 +8,7 @@ defmodule Logflare.Backends.UserMonitoringTest do alias Logflare.Users alias Logflare.Sources alias Logflare.Backends + alias Logflare.Backends.QueryError alias Logflare.Backends.SourceSup alias Logflare.Backends.UserMonitoring alias Logflare.SystemMetrics.AllLogsLogged @@ -67,6 +68,60 @@ defmodule Logflare.Backends.UserMonitoringTest do end) end + test "query error logs are routed to user's system source when monitoring is on", %{ + user: user, + source: source + } do + {:ok, user} = Users.update_user_allowed(user, %{system_monitoring: true}) + system_source = Sources.get_by(user_id: user.id, system_source_type: :logs) + + error = %QueryError{ + kind: :backend_error, + raw_error: %{"message" => "raw query detail"}, + backend: Logflare.Backends.Adaptor.BigQueryAdaptor + } + + TestUtils.retry_assert(fn -> + assert capture_log(fn -> + QueryError.log(error, + user_id: user.id, + source_token: source.token + ) + end) =~ "Backend query error" + + assert Enum.any?( + Backends.list_recent_logs(system_source), + &query_error_log_event?/1 + ) + end) + end + + test "invalid query errors are not routed to user's system source when monitoring is on", %{ + user: user, + source: source + } do + {:ok, user} = Users.update_user_allowed(user, %{system_monitoring: true}) + system_source = Sources.get_by(user_id: user.id, system_source_type: :logs) + + error = %QueryError{ + kind: :invalid_query, + raw_error: %{"message" => "raw user query detail"}, + backend: Logflare.Backends.Adaptor.BigQueryAdaptor + } + + assert capture_log(fn -> + QueryError.log(error, + user_id: user.id, + source_token: source.token + ) + end) == "" + + refute Enum.any?( + Backends.list_recent_logs(system_source), + &query_error_log_event?/1 + ) + end + test "are not routed to user's system source when not monitoring", %{ user: user, source: source @@ -86,6 +141,17 @@ defmodule Logflare.Backends.UserMonitoringTest do end end + defp query_error_log_event?(%{ + body: %{ + "event_message" => "Backend query error", + "metadata" => %{"error_kind" => "backend_error", "error_string" => error_string} + } + }) do + error_string =~ "raw query detail" + end + + defp query_error_log_event?(_event), do: false + describe "system monitoring labels" do setup :start_otel_exporter diff --git a/test/logflare/endpoints/cache_test.exs b/test/logflare/endpoints/cache_test.exs index 677a9c96ea..7c0a3e860d 100644 --- a/test/logflare/endpoints/cache_test.exs +++ b/test/logflare/endpoints/cache_test.exs @@ -60,9 +60,8 @@ defmodule Logflare.Endpoints.CacheTest do assert {:error, %QueryError{ - code: :connection_error, + kind: :connection_error, backend: Logflare.Backends.Adaptor.BigQueryAdaptor, - message: "timeout", raw_error: :timeout }} = Endpoints.run_cached_query(endpoint) @@ -106,9 +105,8 @@ defmodule Logflare.Endpoints.CacheTest do assert {:error, %QueryError{ - code: :invalid_query, + kind: :backend_error, backend: Logflare.Backends.Adaptor.BigQueryAdaptor, - message: "BQ Error", raw_error: %{"message" => "BQ Error"} }} = Endpoints.run_cached_query(endpoint) diff --git a/test/logflare/sql_test.exs b/test/logflare/sql_test.exs index 594a3042d0..64b264ca98 100644 --- a/test/logflare/sql_test.exs +++ b/test/logflare/sql_test.exs @@ -904,6 +904,13 @@ defmodule Logflare.SqlTest do assert {:error, _} = Sql.expand_subqueries(:bq_sql, "select from", [alert]) end + test "expand_subqueries/3 returns an error for empty input" do + alert = build(:alert, name: "my.alert", query: "select 'id' as id", language: :bq_sql) + + assert {:error, "Query cannot be empty"} = Sql.expand_subqueries(:bq_sql, "", [alert]) + assert {:error, "Query cannot be empty"} = Sql.expand_subqueries(:bq_sql, "", []) + end + describe "transform/3 for :postgres backends" do setup do user = insert(:user) diff --git a/test/logflare/utils/logger_metadata_test.exs b/test/logflare/utils/logger_metadata_test.exs new file mode 100644 index 0000000000..d99d09b9b2 --- /dev/null +++ b/test/logflare/utils/logger_metadata_test.exs @@ -0,0 +1,36 @@ +defmodule Logflare.Utils.LoggerMetadataTest do + use ExUnit.Case, async: true + import ExUnit.CaptureLog + + require Logger + alias Logflare.Utils.LoggerMetadata + + describe "with_metadata/2" do + setup do + Logger.metadata(request_id: "before") + end + + test "sets metadata while the function runs and restores previous metadata" do + log = + capture_log([level: :error, metadata: [:request_id, :user_id]], fn -> + LoggerMetadata.with_metadata([user_id: 123], fn -> + Logger.error("test") + end) + end) + + assert log =~ "request_id=before" + assert log =~ "user_id=123" + assert Logger.metadata() == [request_id: "before"] + end + + test "restores previous metadata when the function raises" do + assert_raise RuntimeError, "boom", fn -> + LoggerMetadata.with_metadata([user_id: 123], fn -> + raise "boom" + end) + end + + assert Logger.metadata() == [request_id: "before"] + end + end +end diff --git a/test/logflare_web/controllers/api/backend_controller_test.exs b/test/logflare_web/controllers/api/backend_controller_test.exs index 7f9fe4ea13..8ab66201c1 100644 --- a/test/logflare_web/controllers/api/backend_controller_test.exs +++ b/test/logflare_web/controllers/api/backend_controller_test.exs @@ -482,7 +482,7 @@ defmodule LogflareWeb.Api.BackendControllerTest do |> post("/api/backends/#{backend.token}/test") |> json_response(200) - assert response == %{"connected?" => false, "reason" => "some_reason"} + assert response == %{"connected?" => false} end test "returns 404 if backend doesn't exist or doesn't belong to user", %{ diff --git a/test/logflare_web/controllers/api/query_controller_test.exs b/test/logflare_web/controllers/api/query_controller_test.exs index b5a2a44a1f..096cb50bc0 100644 --- a/test/logflare_web/controllers/api/query_controller_test.exs +++ b/test/logflare_web/controllers/api/query_controller_test.exs @@ -74,7 +74,7 @@ defmodule LogflareWeb.Api.QueryControllerTest do assert %{"result" => [%{"my_time" => "123"}]} = response end - test "BQ errors are propagated", %{ + test "BQ errors return a generic response", %{ conn: conn, user: user } do @@ -89,7 +89,12 @@ defmodule LogflareWeb.Api.QueryControllerTest do |> get(~p"/api/query?#{[bq_sql: ~s|select current_datetime() as 'my_time'|]}") |> json_response(400) - assert %{"error" => %{"message" => "some error"}} = response + assert %{ + "error" => + "Backend error! Retry your query. Please contact support if this continues." + } = response + + refute inspect(response) =~ "some error" end end diff --git a/test/logflare_web/controllers/endpoints_controller_test.exs b/test/logflare_web/controllers/endpoints_controller_test.exs index 239fe0e711..c84cb097d3 100644 --- a/test/logflare_web/controllers/endpoints_controller_test.exs +++ b/test/logflare_web/controllers/endpoints_controller_test.exs @@ -10,6 +10,7 @@ defmodule LogflareWeb.EndpointsControllerTest do alias Logflare.Sources alias Logflare.Sources.Source alias Logflare.SystemMetrics.AllLogsLogged + alias LogflareWeb.QueryErrorHelpers setup do start_supervised!(AllLogsLogged) @@ -42,7 +43,7 @@ defmodule LogflareWeb.EndpointsControllerTest do |> json_response(200) |> assert_schema("EndpointQuery") - assert response.error == %{"message" => "failed_request"} + assert response.error == QueryErrorHelpers.generic_query_error_message() refute response.result refute conn.halted @@ -264,7 +265,7 @@ defmodule LogflareWeb.EndpointsControllerTest do |> assert_schema("EndpointQuery") assert response.error =~ - "Multiple CTEs available (first_cte, second_cte, final_data). You must specify which one to query using `f:name`" + LogflareWeb.QueryErrorHelpers.generic_query_error_message() refute response.result end @@ -387,7 +388,7 @@ defmodule LogflareWeb.EndpointsControllerTest do |> assert_schema("EndpointQuery") assert response.error =~ - "Table 'nonexistent' not found in available CTEs: first_cte, second_cte" + LogflareWeb.QueryErrorHelpers.generic_query_error_message() refute response.result end diff --git a/test/logflare_web/live/alerts/alerts_live_test.exs b/test/logflare_web/live/alerts/alerts_live_test.exs index 1af1877a3e..2975f3d0e5 100644 --- a/test/logflare_web/live/alerts/alerts_live_test.exs +++ b/test/logflare_web/live/alerts/alerts_live_test.exs @@ -367,7 +367,10 @@ defmodule LogflareWeb.AlertsLiveTest do assert html =~ "No results from query. Alert will not fire." end - test "errors from BQ are displayed", %{conn: conn, alert_query: alert_query} do + test "unclassified BQ errors display a generic message", %{ + conn: conn, + alert_query: alert_query + } do GoogleApi.BigQuery.V2.Api.Jobs |> expect(:bigquery_jobs_query, 1, fn _conn, _proj_id, _opts -> {:error, TestUtils.gen_bq_error("some error")} @@ -375,9 +378,13 @@ defmodule LogflareWeb.AlertsLiveTest do {:ok, view, _html} = live_with_redirect(conn, ~p"/alerts/#{alert_query.id}") - assert view - |> element("button", "Run query") - |> render_click() =~ "some error" + html = + view + |> element("button", "Run query") + |> render_click() + + assert html =~ "Backend error! Retry your query. Please contact support if this continues." + refute html =~ "some error" end end @@ -400,19 +407,6 @@ defmodule LogflareWeb.AlertsLiveTest do assert view |> render() =~ ~r/1 .+ processed/ end - test "errors from BQ are dispalyed", %{conn: conn, alert_query: alert_query} do - GoogleApi.BigQuery.V2.Api.Jobs - |> expect(:bigquery_jobs_query, 1, fn _conn, _proj_id, _opts -> - {:error, TestUtils.gen_bq_error("some error")} - end) - - {:ok, view, _html} = live_with_redirect(conn, ~p"/alerts/#{alert_query.id}") - - assert view - |> element("button", "Run query") - |> render_click() =~ "some error" - end - test "missing field errors are displayed with user-facing message", %{ conn: conn, alert_query: alert_query @@ -454,6 +448,20 @@ defmodule LogflareWeb.AlertsLiveTest do assert html =~ "edit-results" end + test "test query from edit page handles SQL validation errors", %{ + conn: conn, + alert_query: alert_query + } do + {:ok, view, _html} = live_with_redirect(conn, ~p"/alerts/#{alert_query.id}/edit") + + html = + view + |> element("form[phx-submit='run-query']") + |> render_submit(%{query: "select * from `my-source`"}) + + assert html =~ "Error when running query: restricted wildcard (*) in a result column" + end + test "test query from new page uses the submitted query", %{conn: conn} do test_query = "select current_timestamp() as test_col" diff --git a/test/logflare_web/live/backends/components_test.exs b/test/logflare_web/live/backends/components_test.exs index e4133a0ce7..878f23257a 100644 --- a/test/logflare_web/live/backends/components_test.exs +++ b/test/logflare_web/live/backends/components_test.exs @@ -16,23 +16,23 @@ defmodule LogflareWeb.Backends.ComponentsTest do refute html =~ "fa-check" end - test "renders an error icon and reason when the async result failed" do + test "renders a generic query error message when the async result failed" do result = AsyncResult.failed(AsyncResult.loading(), {:error, "boom"}) html = render_component(&Components.status_indicator/1, %{status: result}) assert html =~ "fa-times" assert html =~ "tw-text-red-500" - assert html =~ "boom" + assert html =~ "Backend error! Retry your query." refute html =~ "fa-spinner" refute html =~ "fa-check" end - test "renders a generic 'Internal error' message on non-error failure" do + test "renders a generic query error message on non-error failure" do result = AsyncResult.failed(AsyncResult.loading(), {:exit, :boom}) html = render_component(&Components.status_indicator/1, %{status: result}) assert html =~ "fa-times" - assert html =~ "Internal error" + assert html =~ "Backend error! Retry your query." refute html =~ "fa-spinner" refute html =~ "fa-check" end diff --git a/test/logflare_web/live/search_live/logs_search_lv_test.exs b/test/logflare_web/live/search_live/logs_search_lv_test.exs index 3cc2a83e39..6efaeda061 100644 --- a/test/logflare_web/live/search_live/logs_search_lv_test.exs +++ b/test/logflare_web/live/search_live/logs_search_lv_test.exs @@ -1377,7 +1377,6 @@ defmodule LogflareWeb.Source.SearchLVTest do send_query_error( view, - message: message, backend: BigQueryAdaptor, raw_error: %{ "message" => message, @@ -1406,7 +1405,6 @@ defmodule LogflareWeb.Source.SearchLVTest do send_query_error( view, - message: message, backend: BigQueryAdaptor, raw_error: %{"message" => message} ) @@ -1432,7 +1430,6 @@ defmodule LogflareWeb.Source.SearchLVTest do send_query_error( view, - message: message, backend: BigQueryAdaptor, raw_error: %{"message" => message} ) @@ -1459,7 +1456,6 @@ defmodule LogflareWeb.Source.SearchLVTest do send_query_error( view, - message: message, backend: ClickHouseAdaptor, raw_error: %Ch.Error{message: message} ) @@ -1483,8 +1479,6 @@ defmodule LogflareWeb.Source.SearchLVTest do send_query_error( view, - message: - ~s|ERROR 42703 (undefined_column) column "notthere" does not exist\n\n query: select notthere|, backend: PostgresAdaptor, raw_error: %Postgrex.Error{message: ~s|column "notthere" does not exist|} ) @@ -1508,7 +1502,6 @@ defmodule LogflareWeb.Source.SearchLVTest do send_query_error( view, - message: "raw backend syntax error", backend: BigQueryAdaptor, raw_error: %RuntimeError{message: "raw backend syntax error"}, description: nil @@ -2417,7 +2410,7 @@ defmodule LogflareWeb.Source.SearchLVTest do end defp send_query_error(view, attrs) do - attrs = Keyword.put_new(attrs, :code, :invalid_query) + attrs = Keyword.put_new(attrs, :kind, :invalid_query) error = struct!(QueryError, attrs) send(view.pid, {:search_error, %{error: error}}) diff --git a/test/logflare_web/live_views/endpoints_live_test.exs b/test/logflare_web/live_views/endpoints_live_test.exs index 3da048b704..45dd96e12d 100644 --- a/test/logflare_web/live_views/endpoints_live_test.exs +++ b/test/logflare_web/live_views/endpoints_live_test.exs @@ -485,6 +485,32 @@ defmodule LogflareWeb.EndpointsLiveTest do end end + describe "run query errors" do + test "backend errors display a generic message", %{conn: conn, user: user} do + endpoint = insert(:endpoint, user: user, query: "select current_datetime() as ts") + + GoogleApi.BigQuery.V2.Api.Jobs + |> expect(:bigquery_jobs_query, 1, fn _conn, _proj_id, _opts -> + {:error, TestUtils.gen_bq_error("raw backend detail", reason: "backendError")} + end) + + {:ok, view, _html} = live_with_redirect(conn, "/endpoints/#{endpoint.id}") + + html = + view + |> element("form", "Test query") + |> render_submit(%{ + run: %{ + query: endpoint.query, + params: %{} + } + }) + + assert html =~ LogflareWeb.QueryErrorHelpers.generic_query_error_message() + refute html =~ "raw backend detail" + end + end + defp change_editor_query(view, query) do result = view diff --git a/test/logflare_web/live_views/query_live_test.exs b/test/logflare_web/live_views/query_live_test.exs index 6d0efa5b3e..0d5c81284a 100644 --- a/test/logflare_web/live_views/query_live_test.exs +++ b/test/logflare_web/live_views/query_live_test.exs @@ -89,6 +89,25 @@ defmodule LogflareWeb.QueryLiveTest do assert render(view) =~ "some-data" end + test "backend errors display a generic message", %{conn: conn} do + GoogleApi.BigQuery.V2.Api.Jobs + |> expect(:bigquery_jobs_query, 1, fn _conn, _proj_id, _opts -> + {:error, TestUtils.gen_bq_error("raw backend detail", reason: "backendError")} + end) + + {:ok, view, _html} = live_with_redirect(conn, ~p"/query") + + view + |> render_hook("parse-query", %{ + value: "select current_timestamp() as ts" + }) + + html = submit_query_form(view, conn) + + assert html =~ LogflareWeb.QueryErrorHelpers.generic_query_error_message() + refute html =~ "raw backend detail" + end + test "parser error", %{conn: conn} do {:ok, view, _html} = live_with_redirect(conn, ~p"/query") diff --git a/test/logflare_web/query_error_helpers_test.exs b/test/logflare_web/query_error_helpers_test.exs index f2586169c1..fa7a1aff4a 100644 --- a/test/logflare_web/query_error_helpers_test.exs +++ b/test/logflare_web/query_error_helpers_test.exs @@ -1,5 +1,127 @@ defmodule LogflareWeb.QueryErrorHelpersTest do use ExUnit.Case, async: true + alias Logflare.Backends.Adaptor.BigQueryAdaptor + alias Logflare.Backends.Adaptor.ClickHouseAdaptor + alias Logflare.Backends.Adaptor.PostgresAdaptor + alias Logflare.Backends.QueryError + alias LogflareWeb.QueryErrorHelpers + doctest LogflareWeb.QueryErrorHelpers + + describe "query_error_message/1" do + test "returns a generic message for unclassified BigQuery errors" do + error = + query_error( + backend: BigQueryAdaptor, + raw_error: %RuntimeError{message: "raw backend syntax error near SELECT secret_field"} + ) + + message = QueryErrorHelpers.query_error_message(error) + + assert message == QueryErrorHelpers.generic_query_error_message() + refute message =~ "secret_field" + refute message =~ "raw backend" + end + + test "returns a generic message for unclassified ClickHouse errors" do + error = + query_error( + backend: ClickHouseAdaptor, + raw_error: %{message: "backend internal detail"} + ) + + assert QueryErrorHelpers.query_error_message(error) == + QueryErrorHelpers.generic_query_error_message() + end + + test "returns a generic message for unclassified Postgres errors" do + error = + query_error( + backend: PostgresAdaptor, + raw_error: %{message: "backend internal detail"} + ) + + assert QueryErrorHelpers.query_error_message(error) == + QueryErrorHelpers.generic_query_error_message() + end + + test "returns missing field message for classified query errors" do + error = + query_error( + backend: BigQueryAdaptor, + raw_error: %{"message" => "Unrecognized name: notthere at [1:8]"} + ) + + assert QueryErrorHelpers.query_error_message(error) == + ~s(Field "notthere" does not exist.) + end + + test "returns BigQuery query without FROM clause invalid query message" do + error = + query_error( + backend: BigQueryAdaptor, + raw_error: %{ + "code" => 400, + "errors" => [ + %{ + "domain" => "global", + "location" => "q", + "locationType" => "parameter", + "message" => "Query without FROM clause cannot have a WHERE clause at [1:47]", + "reason" => "invalidQuery" + } + ], + "message" => "Query without FROM clause cannot have a WHERE clause at [1:47]", + "status" => "INVALID_ARGUMENT" + } + ) + + assert QueryErrorHelpers.query_error_message(error) == + "Query without FROM clause cannot have a WHERE clause at [1:47]" + end + + test "returns bytes billed limit message for classified query errors" do + error = + query_error( + backend: BigQueryAdaptor, + raw_error: %{ + "message" => + "Query exceeded limit for bytes billed: 2000000000. 20004857600 or higher required.", + "reason" => "billingTierLimitExceeded" + } + ) + + assert QueryErrorHelpers.query_error_message(error) == + "total bytes processed for this query is expected to be greater than 2 GB" + end + + test "falls back to generic message when bytes billed parsing fails" do + error = + query_error( + backend: BigQueryAdaptor, + raw_error: %{ + "message" => + "Query exceeded limit for bytes billed but no numeric limit was returned", + "reason" => "billingTierLimitExceeded" + } + ) + + assert QueryErrorHelpers.query_error_message(error) == + QueryErrorHelpers.generic_query_error_message() + end + end + + defp query_error(attrs) do + attrs = + Keyword.merge( + [ + kind: :invalid_query, + description: nil + ], + attrs + ) + + struct!(QueryError, attrs) + end end From ef00e2de0cca8b1acfd03235fa0537b7ac2065d3 Mon Sep 17 00:00:00 2001 From: Matt Stubbs Date: Wed, 17 Jun 2026 16:11:25 +1000 Subject: [PATCH 4/4] bump ci