Allow saving occurrences with invalid context (#71)

This change updates how an occurrence is stored in order to validate if
the context can be serialized to be stored on the database.

If the context cannot be serialized, we emit a warning log message and
store an error as the context.

Extra tests have been added for the new functionality.

**Note:** This is a proposal. I think it may be a good idea but it adds
a bit more of complexity on occurrence building. On the other hand, we
avoid having issues which are invisible to our users by not storing
them.

Closes #65 (again)

Author: odarriba

lib/error_tracker.ex

@@ -70,6 +70,7 @@ defmodule ErrorTracker do
   import Ecto.Query
 
   alias ErrorTracker.Error
+  alias ErrorTracker.Occurrence
   alias ErrorTracker.Repo
   alias ErrorTracker.Telemetry
 
@@ -212,11 +213,12 @@ defmodule ErrorTracker do
 
         occurrence =
           error
-          |> Ecto.build_assoc(:occurrences,
+          |> Ecto.build_assoc(:occurrences)
+          |> Occurrence.changeset(%{
             stacktrace: stacktrace,
             context: context,
             reason: reason
-          )
+          })
           |> Repo.insert!()
 
         {error, occurrence}

lib/error_tracker/schemas/occurrence.ex

@@ -8,6 +8,9 @@ defmodule ErrorTracker.Occurrence do
 
   use Ecto.Schema
 
+  require Logger
+  import Ecto.Changeset
+
   schema "error_tracker_occurrences" do
     field :context, :map
     field :reason, :string
@@ -17,4 +20,54 @@ defmodule ErrorTracker.Occurrence do
 
     timestamps(type: :utc_datetime_usec, updated_at: false)
   end
+
+  @doc false
+  def changeset(occurrence, attrs) do
+    occurrence
+    |> cast(attrs, [:context, :reason])
+    |> maybe_put_stacktrace()
+    |> validate_required([:reason, :stacktrace])
+    |> validate_context()
+    |> foreign_key_constraint(:error)
+  end
+
+  # This function validates if the context can be serialized to JSON before
+  # storing it to the DB.
+  #
+  # If it cannot be serialized a warning log message is emitted and an error
+  # is stored in the context.
+  #
+  defp validate_context(changeset) do
+    if changeset.valid? do
+      context = get_field(changeset, :context, %{})
+
+      json_encoder =
+        ErrorTracker.Repo.with_adapter(fn
+          :postgres -> Application.get_env(:postgrex, :json_library, Jason)
+          :sqlite -> Application.get_env(:ecto_sqlite3, :json_library, Jason)
+        end)
+
+      case json_encoder.encode_to_iodata(context) do
+        {:ok, _} ->
+          put_change(changeset, :context, context)
+
+        {:error, _} ->
+          Logger.warning(
+            "[ErrorTracker] Context has been ignored: it is not serializable to JSON."
+          )
+
+          put_change(changeset, :context, %{
+            error: "Context not stored because it contains information not serializable to JSON."
+          })
+      end
+    else
+      changeset
+    end
+  end
+
+  defp maybe_put_stacktrace(changeset) do
+    if stacktrace = Map.get(changeset.params, "stacktrace"),
+      do: put_embed(changeset, :stacktrace, stacktrace),
+      else: changeset
+  end
 end

test/error_tracker/schemas/occurrence_test.exs

@@ -0,0 +1,39 @@
+defmodule ErrorTracker.OccurrenceTest do
+  use ErrorTracker.Test.Case
+
+  import Ecto.Changeset
+
+  alias ErrorTracker.Occurrence
+  alias ErrorTracker.Stacktrace
+
+  describe inspect(&Occurrence.changeset/2) do
+    test "works as expected with valid data" do
+      attrs = %{context: %{foo: :bar}, reason: "Test reason", stacktrace: %Stacktrace{}}
+      changeset = Occurrence.changeset(%Occurrence{}, attrs)
+
+      assert changeset.valid?
+    end
+
+    test "validates required fields" do
+      changeset = Occurrence.changeset(%Occurrence{}, %{})
+
+      refute changeset.valid?
+      assert {_, [validation: :required]} = changeset.errors[:reason]
+      assert {_, [validation: :required]} = changeset.errors[:stacktrace]
+    end
+
+    @tag capture_log: true
+    test "if context is not serializable, an error messgae is stored" do
+      attrs = %{
+        context: %{foo: %ErrorTracker.Error{}},
+        reason: "Test reason",
+        stacktrace: %Stacktrace{}
+      }
+
+      changeset = Occurrence.changeset(%Occurrence{}, attrs)
+
+      assert %{error: err} = get_field(changeset, :context)
+      assert err =~ "not serializable to JSON"
+    end
+  end
+end

test/error_tracker_test.exs

@@ -64,6 +64,14 @@ defmodule ErrorTrackerTest do
       assert error.reason == "This is a test"
       assert error.source_line =~ @relative_file_path
     end
+
+    @tag capture_log: true
+    test "reports errors with invalid context" do
+      # It's invalid because cannot be serialized to JSON
+      invalid_context = %{foo: %ErrorTracker.Error{}}
+
+      assert %Occurrence{} = report_error(fn -> raise "test" end, invalid_context)
+    end
   end
 
   describe inspect(&ErrorTracker.resolve/1) do

test/support/case.ex

@@ -16,16 +16,16 @@ defmodule ErrorTracker.Test.Case do
   @doc """
   Reports the error produced by the given function.
   """
-  def report_error(fun) do
+  def report_error(fun, context \\ %{}) do
     occurrence =
       try do
         fun.()
       rescue
         exception ->
-          ErrorTracker.report(exception, __STACKTRACE__)
+          ErrorTracker.report(exception, __STACKTRACE__, context)
       catch
         kind, reason ->
-          ErrorTracker.report({kind, reason}, __STACKTRACE__)
+          ErrorTracker.report({kind, reason}, __STACKTRACE__, context)
       end
 
     repo().preload(occurrence, :error)