Merge pull request #40 from nirev/master

Only handle 5xx exceptions

Author: nirev

lib/plugsnag.ex

@@ -2,41 +2,37 @@ defmodule Plugsnag do
   defmacro __using__(options \\ []) do
     quote location: :keep do
       use Plug.ErrorHandler
-      import Plugsnag
 
-      if :code.is_loaded(Phoenix) do
-        defp handle_errors(_conn, %{reason: %Phoenix.Router.NoRouteError{}}) do
-          nil
+      defp handle_errors(conn, %{reason: exception, kind: :error} = assigns) do
+        # Only handle exceptions that get rendered as an HTTP 5xx status
+        if Plug.Exception.status(exception) >= 500 do
+          do_handle_errors(conn, assigns)
         end
       end
 
-      if :code.is_loaded(Ecto) do
-        defp handle_errors(conn, %{reason: %Ecto.NoResultsError{}}) do
-          nil
-        end
+      defp handle_errors(conn, %{reason: _exception} = assigns) do
+        do_handle_errors(conn, assigns)
       end
 
-      defp handle_errors(conn, %{reason: exception}) do
-        error_report_builder = unquote(
-          Keyword.get(
-            options,
-            :error_report_builder,
-            Plugsnag.BasicErrorReportBuilder
+      defp do_handle_errors(conn, %{reason: exception}) do
+        error_report_builder =
+          unquote(
+            Keyword.get(
+              options,
+              :error_report_builder,
+              Plugsnag.BasicErrorReportBuilder
+            )
           )
-        )
 
         options =
           %Plugsnag.ErrorReport{}
           |> error_report_builder.build_error_report(conn)
-          |> Map.from_struct
-          |> Keyword.new
+          |> Map.from_struct()
+          |> Keyword.new()
 
-        apply(reporter(), :report, [exception | [options]])
+        reporter = Application.get_env(:plugsnag, :reporter, Bugsnag)
+        apply(reporter, :report, [exception | [options]])
       end
     end
   end
-
-  def reporter do
-    Application.get_env(:plugsnag, :reporter, Bugsnag)
-  end
 end

test/plugsnag_test.exs

@@ -3,16 +3,36 @@ defmodule PlugsnagTest do
   use Plug.Test
 
   defmodule TestException do
-    defexception plug_status: 403, message: "oops"
+    defexception plug_status: 503, message: "oops"
+  end
+
+  defmodule NotFoundException do
+    defexception plug_status: 404, message: "not found"
   end
 
   defmodule ErrorRaisingPlug do
     defmacro __using__(_env) do
       quote do
         def call(conn, _opts) do
-          raise Plug.Conn.WrapperError, conn: conn,
-          kind: :error, stack: System.stacktrace,
-          reason: TestException.exception([])
+          raise Plug.Conn.WrapperError,
+            conn: conn,
+            kind: :error,
+            stack: System.stacktrace(),
+            reason: TestException.exception([])
+        end
+      end
+    end
+  end
+
+  defmodule NotFoundRaisingPlug do
+    defmacro __using__(_env) do
+      quote do
+        def call(conn, _opts) do
+          raise Plug.Conn.WrapperError,
+            conn: conn,
+            kind: :error,
+            stack: System.stacktrace(),
+            reason: NotFoundException.exception([])
         end
       end
     end
@@ -23,66 +43,84 @@ defmodule PlugsnagTest do
     use ErrorRaisingPlug
   end
 
+  defmodule NotFoundPlug do
+    use Plugsnag
+    use NotFoundRaisingPlug
+  end
+
   defmodule FakePlugsnag do
     def report(exception, options \\ []) do
-      send self(), {:report, {exception, options}}
+      send(self(), {:report, {exception, options}})
     end
   end
 
   setup do
     Application.put_env(:plugsnag, :reporter, FakePlugsnag)
   end
 
-  test "Raising an error on failure" do
-    conn = conn(:get, "/")
+  describe "5xx exception" do
+    test "Raising an error on failure" do
+      conn = conn(:get, "/")
 
-    assert_raise Plug.Conn.WrapperError, "** (PlugsnagTest.TestException) oops", fn ->
-      TestPlug.call(conn, [])
-    end
+      assert_raise Plug.Conn.WrapperError, "** (PlugsnagTest.TestException) oops", fn ->
+        TestPlug.call(conn, [])
+      end
 
-    assert_received {:report, {%TestException{}, _}}
-  end
+      assert_received {:report, {%TestException{}, _}}
+    end
 
-  test "includes connection metadata in the report" do
-    conn = conn(:get, "/?hello=computer")
+    test "includes connection metadata in the report" do
+      conn = conn(:get, "/?hello=computer")
 
-    catch_error TestPlug.call(conn, [])
-    assert_received {:report, {%TestException{}, options}}
-    metadata = Keyword.get(options, :metadata)
+      catch_error(TestPlug.call(conn, []))
+      assert_received {:report, {%TestException{}, options}}
+      metadata = Keyword.get(options, :metadata)
 
-    assert get_in(metadata, [:request,:query_string]) == "hello=computer"
-  end
+      assert get_in(metadata, [:request, :query_string]) == "hello=computer"
+    end
 
-  test "allows modifying bugsnag report options before it's sent" do
-    defmodule TestErrorReportBuilder do
-      @behaviour Plugsnag.ErrorReportBuilder
+    test "allows modifying bugsnag report options before it's sent" do
+      defmodule TestErrorReportBuilder do
+        @behaviour Plugsnag.ErrorReportBuilder
 
-      def build_error_report(error_report, conn) do
-        user_info =  %{
-          id: conn |> get_req_header("x-user-id") |> List.first
-        }
+        def build_error_report(error_report, conn) do
+          user_info = %{
+            id: conn |> get_req_header("x-user-id") |> List.first()
+          }
 
-        %{error_report | user: user_info}
+          %{error_report | user: user_info}
+        end
       end
-    end
 
-    defmodule TestPlugsnagCallbackPlug do
-      use Plugsnag, error_report_builder: TestErrorReportBuilder
-      use ErrorRaisingPlug
-    end
+      defmodule TestPlugsnagCallbackPlug do
+        use Plugsnag, error_report_builder: TestErrorReportBuilder
+        use ErrorRaisingPlug
+      end
 
-    conn = conn(:get, "/")
+      conn = conn(:get, "/")
 
-    conn =
-      conn
-      |> put_req_header("x-user-id", "abc123")
+      conn =
+        conn
+        |> put_req_header("x-user-id", "abc123")
 
-    catch_error TestPlugsnagCallbackPlug.call(conn, [])
-    assert_received {:report, {%TestException{}, options}}
+      catch_error(TestPlugsnagCallbackPlug.call(conn, []))
+      assert_received {:report, {%TestException{}, options}}
 
-    assert Keyword.get(options, :user) == %{
-     id: "abc123"
-    }
+      assert Keyword.get(options, :user) == %{
+               id: "abc123"
+             }
+    end
   end
 
+  describe "4xx exception" do
+    test "Raising an error on failure" do
+      conn = conn(:get, "/")
+
+      assert_raise Plug.Conn.WrapperError, "** (PlugsnagTest.NotFoundException) not found", fn ->
+        NotFoundPlug.call(conn, [])
+      end
+
+      refute_receive {:report, {%NotFoundException{}, _}}, 100
+    end
+  end
 end