[Fix] Usage along Plug.ErrorHandler

README.md

@@ -93,29 +93,6 @@ defmodule YourApp.Router do
     ]
 ```
 
-## Setup when `handle_errors/2` is already being used
-
-If you are using or want to use your own implementation of `handle_errors/2` for the` Plug.ErrorHandler` module, be sure to include the usage of `BoomNotifier` after
-that.
-
-In addition, you will have to add the `notify_error/2` callback that `BoomNotifier` provides within your implementation of `handle_errors/2`.
-
-```elixir
-defmodule YourApp.Router do
-  use Phoenix.Router
-
-  use Plug.ErrorHandler
-
-  def handle_errors(conn, error) do
-    # ...
-    notify_error(conn, error)
-    # ...
-  end
-
-  use BoomNotifier,
-    ...
-```
-
 ## Notification Trigger
 
 By default, `BoomNotifier` will send a notification every time an exception is
@@ -295,6 +272,11 @@ defmodule YourApp.Router do
 end
 ```
 
+## Implementation details
+
+Boom uses `Plug.ErrorHandler` to trigger notifications.
+If you are already using that module you must use `BoomNotifier` after it.
+
 ## License
 
 BoomNotifier is released under the terms of the [MIT License](https://github.com/wyeworks/boom/blob/master/LICENSE).

lib/boom_notifier.ex

@@ -51,12 +51,14 @@ defmodule BoomNotifier do
   end
 
   defmacro __using__(config) do
-    quote location: :keep do
+    quote do
       import BoomNotifier
 
-      error_handler_in_use = {:handle_errors, 2} in Module.definitions_in(__MODULE__)
+      error_handler_in_use = Plug.ErrorHandler in @behaviour
 
-      unless error_handler_in_use do
+      if error_handler_in_use do
+        @before_compile BoomNotifier
+      else
         use Plug.ErrorHandler
 
         @impl Plug.ErrorHandler
@@ -107,4 +109,15 @@ defmodule BoomNotifier do
       end
     end
   end
+
+  defmacro __before_compile__(_env) do
+    quote do
+      defoverridable handle_errors: 2
+
+      def handle_errors(conn, error) do
+        super(conn, error)
+        notify_error(conn, error)
+      end
+    end
+  end
 end

test/unit/notifier_test.exs

@@ -295,7 +295,7 @@
     conn = conn(:get, "/")
 
     assert capture_log(fn ->
              defmodule PlugLogWithMissingParameterNotifier do
                use BoomNotifier,
                  notifier: MissingParameterNotifier,
                  options: [
@@ -318,7 +318,7 @@
     conn = conn(:get, "/")
 
     assert capture_log(fn ->
              defmodule PlugLogWithMissingParameterNotifier do
                use BoomNotifier, other: nil
 
                def call(_conn, _opts) do
@@ -395,10 +395,8 @@
     defmodule PlugErrorWithCallback do
       use Plug.ErrorHandler
 
-      def handle_errors(conn, error) do
-        send(self(), :before_callback)
-        notify_error(conn, error)
-        send(self(), :after_callback)
+      def handle_errors(_conn, _error) do
+        send(self(), :handle_errors_called)
       end
 
       use BoomNotifier,
@@ -417,9 +415,8 @@
 
       catch_error(PlugErrorWithCallback.call(conn, []))
 
-      assert_received :before_callback
       assert_receive(%{exception: _exception}, @receive_timeout)
-      assert_received :after_callback
+      assert_received :handle_errors_called
     end
   end
 end

test/unit/use_test.exs

@@ -0,0 +1,171 @@
+defmodule BoomNotifier.UseTest do
+  use ExUnit.Case
+
+  @already_sent {:plug_conn, :sent}
+
+  defmodule Notifier do
+    def notify(%{name: name}, _) do
+      pid = Process.whereis(BoomNotifier.UseTest)
+      send(pid, {:notification_sent, name})
+    end
+  end
+
+  defmodule TestException do
+    defexception [:message]
+  end
+
+  setup do
+    Process.register(self(), BoomNotifier.UseTest)
+
+    %{conn: Plug.Test.conn(:get, "/") |> Map.put(:owner, self())}
+  end
+
+  def assert_notification_sent(plug, conn) do
+    assert_raise(Plug.Conn.WrapperError, fn ->
+      plug.call(conn, [])
+    end)
+
+    assert_receive({:notification_sent, TestException})
+  end
+
+  describe "plug app" do
+    defmodule TestEndpointWithoutErrorHandler do
+      @moduledoc """
+      Plug app
+      """
+      use Plug.Router
+      use BoomNotifier, notifiers: [[notifier: Notifier, options: []]]
+
+      plug(:match)
+      plug(:dispatch)
+
+      get("/", do: raise(TestException, "Something went wrong at #{conn.request_path}"))
+    end
+
+    test "notifies on exception and does not send a response", %{
+      conn: conn
+    } do
+      assert_notification_sent(TestEndpointWithoutErrorHandler, conn)
+
+      refute_receive(@already_sent)
+    end
+  end
+
+  describe "plug app with Plug.ErrorHandler" do
+    defmodule TestEndpointWithErrorHandler do
+      @moduledoc """
+      Plug app with Plug.ErrorHandler
+      """
+      use Plug.Router
+      use Plug.ErrorHandler
+      use BoomNotifier, notifiers: [[notifier: Notifier, options: []]]
+
+      plug(:match)
+      plug(:dispatch)
+
+      get("/", do: raise(TestException, "Something went wrong at #{conn.request_path}"))
+    end
+
+    test "notifies on exception and sends a response", %{conn: conn} do
+      assert_notification_sent(TestEndpointWithErrorHandler, conn)
+
+      assert_receive(@already_sent)
+    end
+  end
+
+  describe "phoenix app" do
+    defmodule TestPhoenixRouter do
+      @moduledoc """
+      Phoenix app
+      """
+      use Phoenix.Router
+      import Phoenix.Controller
+
+      use BoomNotifier, notifiers: [[notifier: Notifier, options: []]]
+
+      get("/", TestPhoenixRouter.TestController, :index)
+    end
+
+    defmodule TestPhoenixRouter.TestController do
+      use Phoenix.Controller, namespace: TestPhoenixRouter
+
+      def index(_conn, _params) do
+        raise TestException, "Something went wrong"
+      end
+    end
+
+    test "notifies on exception", %{conn: conn} do
+      assert_notification_sent(TestPhoenixRouter, conn)
+
+      refute_receive(@already_sent)
+    end
+  end
+
+  describe "phoenix app with Plug.ErrorHandler" do
+    defmodule TestPhoenixRouterWithErrorHandler do
+      @moduledoc """
+      Phoenix app with Plug.ErrorHandler
+      """
+      use Phoenix.Router
+      import Phoenix.Controller
+      use Plug.ErrorHandler
+
+      use BoomNotifier, notifiers: [[notifier: Notifier, options: []]]
+
+      get("/", TestPhoenixRouterWithErrorHandler.TestController, :index)
+    end
+
+    defmodule TestPhoenixRouterWithErrorHandler.TestController do
+      use Phoenix.Controller, namespace: TestPhoenixRouterWithErrorHandler
+
+      def index(_conn, _params) do
+        raise TestException, "Something went wrong"
+      end
+    end
+
+    test "notifies on exception", %{conn: conn} do
+      assert_notification_sent(TestPhoenixRouterWithErrorHandler, conn)
+
+      assert_receive(@already_sent)
+    end
+  end
+
+  describe "phoenix app with Plug.ErrorHandler and handle_errors/2" do
+    defmodule TestPhoenixRouterWithErrorHandler2 do
+      @moduledoc """
+      Phoenix app with Plug.ErrorHandler and custom handle_errors/2
+      """
+      use Phoenix.Router
+      import Phoenix.Controller
+      use Plug.ErrorHandler
+
+      use BoomNotifier, notifiers: [[notifier: Notifier, options: []]]
+
+      def handle_errors(conn, _error) do
+        send_resp(conn, 504, custom_error_message())
+      end
+
+      get("/", TestPhoenixRouterWithErrorHandler2.TestController, :index)
+
+      def custom_error_message do
+        "custom_error_message"
+      end
+    end
+
+    defmodule TestPhoenixRouterWithErrorHandler2.TestController do
+      use Phoenix.Controller, namespace: TestPhoenixRouterWithErrorHandler2
+
+      def index(_conn, _params) do
+        raise TestException, "Something went wrong"
+      end
+    end
+
+    test "notifies on exception", %{conn: conn} do
+      assert_notification_sent(TestPhoenixRouterWithErrorHandler2, conn)
+
+      error_response = TestPhoenixRouterWithErrorHandler2.custom_error_message()
+      assert_receive(@already_sent)
+      assert_receive({_ref, {504, _, ^error_response}})
+    end
+  end
+end