Be more defensive with stacktrace parsing - v1.X (#131)

Author: binarylogic

lib/timber/events/exception_event.ex

@@ -47,9 +47,9 @@ defmodule Timber.Events.ExceptionEvent do
       |> Enum.map(&({&1, String.trim(&1)}))
 
     case do_new({nil, "", []}, lines) do
-      {name, message, backtrace} when is_binary(name) and length(backtrace) > 0 ->
-        name = String.slice(name, 0..(@name_limit - 1))
-        message = String.slice(message, 0..(@message_limit - 1))
+      {:ok, {name, message, backtrace}} when is_binary(name) and length(backtrace) > 0 ->
+        name = Timber.Utils.Logger.truncate(name, @name_limit)
+        message = Timber.Utils.Logger.truncate(message, @message_limit)
 
         {:ok, %__MODULE__{name: name, message: message, backtrace: backtrace}}
 
@@ -66,8 +66,12 @@ defmodule Timber.Events.ExceptionEvent do
   #    ** (RuntimeError) my message
   defp do_new({nil, _message, [] = backtrace}, [{_raw_line, ("** (" <> line_suffix)} | lines]) do
     # Using split since it is more performance with binary scanning
-    [name, message] = String.split(line_suffix, ")", parts: 2)
-    do_new({name, message, backtrace}, lines)
+    case String.split(line_suffix, ")", parts: 2) do
+      [name, message] ->
+        do_new({name, message, backtrace}, lines)
+
+      _ -> {:error, :malformed_error_message}
+    end
   end
 
   # Ignore other leading messages
@@ -76,29 +80,46 @@ defmodule Timber.Events.ExceptionEvent do
   #      (odin_client_api) web/controllers/page_controller.ex:5: Odin.ClientAPI.PageController.index/2
   defp do_new({name, message, backtrace}, [{_raw_line, ("(" <> line_suffix)} | lines]) when not is_nil(name) and not is_nil(message) do
     # Using split since it is more performance with binary scanning
-    [app_name, line_suffix] = String.split(line_suffix, ")", parts: 2)
-    [file, line_suffix] = String.split(line_suffix, ":", parts: 2)
-    [line_number, function] = String.split(line_suffix, ":", parts: 2)
-
-    app_name = String.slice(app_name, 0..(@app_name_limit - 1))
-    function = String.slice(function, 0..(@function_limit - 1))
-    file = String.slice(file, 0..(@file_limit - 1))
-
-    line = %{
-      app_name: app_name,
-      function: String.trim(function),
-      file: String.trim(file),
-      line: parse_line_number(line_number)
-    }
-    do_new({name, message, [line | backtrace]}, lines)
+    with [app_name, line_suffix] <- String.split(line_suffix, ") ", parts: 2),
+         [file, line_suffix] <- String.split(line_suffix, ":", parts: 2),
+         [line_number, function] <- String.split(line_suffix, ":", parts: 2)
+    do
+      app_name = Timber.Utils.Logger.truncate(app_name, @app_name_limit)
+
+      function =
+        function
+        |> String.trim()
+        |> Timber.Utils.Logger.truncate(@function_limit)
+
+      file =
+        file
+        |> String.trim()
+        |> Timber.Utils.Logger.truncate(@file_limit)
+
+      if function != "" && file != "" do
+        line = %{
+          app_name: app_name,
+          function: String.trim(function),
+          file: String.trim(file),
+          line: parse_line_number(line_number)
+        }
+        do_new({name, message, [line | backtrace]}, lines)
+      else
+        {:error, :malformed_stacktrace_line}
+      end
+
+    else
+      _ ->
+        {:error, :malformed_stacktrace_line}
+    end
   end
 
   # Ignore lines we don't recognize.
   defp do_new(acc, [_line | lines]), do: do_new(acc, lines)
 
   # Finish the iteration, reversing the backtrace for performance reasons.
   defp do_new({name, message, backtrace}, []) do
-    {name, String.trim(message), Enum.reverse(backtrace)}
+    {:ok, {name, String.trim(message), Enum.reverse(backtrace)}}
   end
 
   defp parse_line_number(line_str) do
@@ -113,4 +134,4 @@ defmodule Timber.Events.ExceptionEvent do
   """
   @spec message(t) :: IO.chardata
   def message(%__MODULE__{name: name, message: message}), do: [?(, name, ?), ?\s, message]
-end
+end

test/lib/timber/events/exception_event_test.exs

@@ -41,6 +41,28 @@ defmodule Timber.Events.ExceptionEventTest do
       }
     end
 
+    test "native functions" do
+      log_message =
+        """
+        ** (exit) an exception was raised:
+            ** (ArgumentError) argument error
+                (stdlib) :ets.lookup(:noproc, 111)
+        """
+      result = ExceptionEvent.new(log_message)
+      assert result == {:error, :could_not_parse_message}
+    end
+
+    test "malformed stacktrace" do
+      log_message =
+        """
+        ** (exit) an exception was raised:
+            ** (RuntimeError) boom
+                (my_app) malformed
+        """
+      result = ExceptionEvent.new(log_message)
+      assert result == {:error, :could_not_parse_message}
+    end
+
     test "malformed message" do
       {:error, :could_not_parse_message} = ExceptionEvent.new("testing")
     end