Make sure GCP/Elastic formatters won't crash on unknown error terms

Nebo15 · Sep 12, 2024 · 17025e0 · 17025e0
1 parent f1b0cc9
commit 17025e0
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 11 deletions.
diff --git a/lib/logger_json/formatters/elastic.ex b/lib/logger_json/formatters/elastic.ex
@@ -191,7 +191,7 @@ defmodule LoggerJSON.Formatters.Elastic do
   def format_crash_reason(message, {{:EXIT, pid}, reason}, _meta) do
     stacktrace = Exception.format_banner({:EXIT, pid}, reason, [])
     error_message = "process #{inspect(pid)} exit: #{inspect(reason)}"
-    format_error_fields(message, error_message, stacktrace, "EXIT")
+    format_error_fields(message, error_message, stacktrace, "exit")
   end
 
   def format_crash_reason(message, {:exit, reason}, _meta) do
@@ -238,10 +238,8 @@ defmodule LoggerJSON.Formatters.Elastic do
     format_error_fields(message, error_message, stacktrace, error)
   end
 
-  def format_crash_reason(message, {error, reason}, _meta) do
-    stacktrace = "** (#{inspect(error)}) #{inspect(reason)}"
-    error_message = "#{inspect(error)}: #{inspect(reason)}"
-    format_error_fields(message, error_message, stacktrace, "error")
+  def format_crash_reason(message, other, _meta) do
+    format_error_fields(message, inspect(other), nil, nil)
   end
 
   defp get_exception_id(%{id: id}), do: id
@@ -254,10 +252,10 @@ defmodule LoggerJSON.Formatters.Elastic do
   defp format_error_fields(message, error_message, stacktrace, type) do
     %{
       message: safe_chardata_to_string(message),
-      "error.stack_trace": stacktrace,
-      "error.message": error_message,
-      "error.type": type
+      "error.message": error_message
     }
+    |> maybe_put(:"error.stack_trace", stacktrace)
+    |> maybe_put(:"error.type", type)
   end
 
   # Formats the log.logger and log.origin fields as specified in https://www.elastic.co/guide/en/ecs/8.11/ecs-log.html

diff --git a/lib/logger_json/formatters/google_cloud.ex b/lib/logger_json/formatters/google_cloud.ex
@@ -208,6 +208,10 @@ defmodule LoggerJSON.Formatters.GoogleCloud do
     format_crash_reason(binary, {inspect(error), reason}, service_context, meta)
   end
 
+  def format_crash_reason(binary, _other, service_context, meta) do
+    format_reported_error_event(binary, nil, service_context, meta)
+  end
+
   defp format_reported_error_event(message, stacktrace, service_context, meta) do
     %{
       "@type": "type.googleapis.com/google.devtools.clouderrorreporting.v1beta1.ReportedErrorEvent",

diff --git a/test/logger_json/formatters/elastic_test.exs b/test/logger_json/formatters/elastic_test.exs
@@ -324,7 +324,7 @@ defmodule LoggerJSON.Formatters.ElasticTest do
              "message" => "oops!",
              "error.message" => error_message,
              "error.stack_trace" => stacktrace,
-             "error.type" => "EXIT",
+             "error.type" => "exit",
              "log.logger" => "Elixir.LoggerJSON.Formatters.ElasticTest",
              "log.origin" => %{
                "file.line" => _,

diff --git a/test/logger_json/formatters/google_cloud_test.exs b/test/logger_json/formatters/google_cloud_test.exs
@@ -443,8 +443,6 @@ defmodule LoggerJSON.Formatters.GoogleCloudTest do
         Process.sleep(100)
       end)
 
-    refute logs =~ "FORMATTER CRASH"
-
     [_, log_entry] =
       logs
       |> String.trim()
@@ -461,6 +459,40 @@ defmodule LoggerJSON.Formatters.GoogleCloudTest do
     assert message =~ ~r/Task #PID<\d+.\d+.\d+> started from #{inspect(test_pid)} terminating/
   end
 
+  test "does not crash on unknown error tuples" do
+    Logger.metadata(crash_reason: {{:something, :else}, [:unknown]})
+
+    log_entry =
+      capture_log(fn ->
+        Logger.debug("oops!")
+      end)
+      |> decode_or_print_error()
+
+    assert %{
+             "@type" => "type.googleapis.com/google.devtools.clouderrorreporting.v1beta1.ReportedErrorEvent",
+             "message" => "oops!",
+             "stack_trace" => "** ({:something, :else}) [:unknown]",
+             "serviceContext" => %{"service" => "nonode@nohost"}
+           } = log_entry
+  end
+
+  test "does not crash on unknown errors" do
+    Logger.metadata(crash_reason: :what_is_this?)
+
+    log_entry =
+      capture_log(fn ->
+        Logger.debug("oops!")
+      end)
+      |> decode_or_print_error()
+
+    assert %{
+             "@type" => "type.googleapis.com/google.devtools.clouderrorreporting.v1beta1.ReportedErrorEvent",
+             "message" => "oops!",
+             "stack_trace" => nil,
+             "serviceContext" => %{"service" => "nonode@nohost"}
+           } = log_entry
+  end
+
   test "logs process exits" do
     Logger.metadata(crash_reason: {{:EXIT, self()}, :sad_failure})