Merge branch 'refactor-error-reporting' into 'master'

Improve error reporting and add test

See merge request framasoft/mobilizon!1010

lib/mobilizon.ex

@@ -68,14 +68,11 @@ defmodule Mobilizon do
 
     ErrorReporting.configure()
 
-    :ok = Oban.Telemetry.attach_default_logger()
-
-    :telemetry.attach_many(
-      "oban-errors",
-      [[:oban, :job, :exception], [:oban, :circuit, :trip]],
-      &ErrorReporting.handle_event/4,
-      %{}
-    )
+    # Only attach the telemetry logger when we aren't in an IEx shell
+    unless Code.ensure_loaded?(IEx) && IEx.started?() do
+      Oban.Telemetry.attach_default_logger(:info)
+      ErrorReporting.attach()
+    end
 
     Supervisor.start_link(children, strategy: :one_for_one, name: Mobilizon.Supervisor)
   end

lib/service/error_reporting/error_reporting.ex

@@ -7,6 +7,8 @@ defmodule Mobilizon.Service.ErrorReporting do
 
   @callback configure :: any()
 
+  @callback attach :: any()
+
   @callback handle_event(list(atom()), map(), map(), any()) :: any()
 
   @spec adapter :: module() | nil
@@ -15,6 +17,14 @@ defmodule Mobilizon.Service.ErrorReporting do
     if adapter && adapter.enabled?(), do: adapter, else: nil
   end
 
+  def attach do
+    adapter = adapter()
+
+    if adapter do
+      adapter.attach()
+    end
+  end
+
   @spec handle_event(list(atom()), map(), map(), any()) :: any()
   def handle_event(event_name, event_measurements, event_metadata, handler_config) do
     adapter = adapter()

lib/service/error_reporting/sentry.ex

@@ -28,6 +28,16 @@ defmodule Mobilizon.Service.ErrorReporting.Sentry do
     end
   end
 
+  @impl ErrorReporting
+  def attach do
+    :telemetry.attach(
+      "oban-errors",
+      [:oban, :job, :exception],
+      &handle_event/4,
+      []
+    )
+  end
+
   @impl ErrorReporting
   def handle_event([:oban, :job, :exception], measure, %{job: job} = meta, _) do
     extra =
@@ -42,4 +52,6 @@ defmodule Mobilizon.Service.ErrorReporting.Sentry do
   def handle_event([:oban, :circuit, :trip], _measure, meta, _) do
     Sentry.capture_exception(meta.error, stacktrace: meta.stacktrace, extra: meta)
   end
+
+  def handle_event(_, _, _, _), do: :ok
 end

test/service/error_reporting/error_reporting_test.exs

@@ -0,0 +1,80 @@
+defmodule Mobilizon.Service.ErrorReportingTest do
+  @moduledoc """
+  Mpdule to test loading and configuring error reporting adapters
+  """
+
+  use ExUnit.Case, async: true
+  import Mox
+  alias Mobilizon.Service.ErrorReporting
+
+  defmock(ErrorReportingMock, for: ErrorReporting)
+
+  describe "adapter/0 returns the enabled adapter" do
+    test "adapter/0 returns the configured adapter if enabled" do
+      expect(ErrorReportingMock, :enabled?, fn ->
+        true
+      end)
+
+      Mobilizon.Config.put([Mobilizon.Service.ErrorReporting, :adapter], ErrorReportingMock)
+
+      assert ErrorReportingMock == ErrorReporting.adapter()
+    end
+
+    test "adapter/0 returns nothing if configured adapter is not enabled" do
+      expect(ErrorReportingMock, :enabled?, fn ->
+        false
+      end)
+
+      Mobilizon.Config.put([Mobilizon.Service.ErrorReporting, :adapter], ErrorReportingMock)
+
+      assert nil == ErrorReporting.adapter()
+    end
+
+    test "adapter/0 returns nothing if not adapter is configured adapter is configured" do
+      expect(ErrorReportingMock, :enabled?, fn ->
+        true
+      end)
+
+      Mobilizon.Config.put([Mobilizon.Service.ErrorReporting, :adapter], nil)
+
+      assert nil == ErrorReporting.adapter()
+    end
+  end
+
+  describe "forwards to the configured adapter the method" do
+    setup do
+      expect(ErrorReportingMock, :enabled?, fn ->
+        true
+      end)
+
+      Mobilizon.Config.put([Mobilizon.Service.ErrorReporting, :adapter], ErrorReportingMock)
+
+      :ok
+    end
+
+    test "attach/0" do
+      expect(ErrorReportingMock, :attach, fn ->
+        :attached
+      end)
+
+      assert :attached == ErrorReporting.attach()
+    end
+
+    test "handle_event/4" do
+      expect(ErrorReportingMock, :handle_event, fn arg1, arg2, arg3, arg4 ->
+        {:handled_event, [arg1, arg2, arg3, arg4]}
+      end)
+
+      assert {:handled_event, [:one, :two, :three, :four]} ==
+               ErrorReporting.handle_event(:one, :two, :three, :four)
+    end
+
+    test "configure/0" do
+      expect(ErrorReportingMock, :configure, fn ->
+        :configured
+      end)
+
+      assert :configured == ErrorReporting.configure()
+    end
+  end
+end