Merge pull request #31 from CityBaseInc/compilation-error-if-missing-…

…json-encoder

Warnings and errors if missing JSON encoder

CHANGELOG.md

@@ -1,6 +1,6 @@
-# Changelog for v0.x
+# Changelog
 
-## IN PROGRESS
+## v2.2.0 (2024-05-10)
 
 ### Enhancements
 
@@ -9,6 +9,18 @@
 * New config option: `:production_aliases` can be set to a list of names that
   should be translated to `"production"` for `notice.context.environment`.  See
   README for more details.
+* New documentation for config option: `:json_encoder` is documented in the
+  README.
+* New JSON encoder protections:
+    * If the JSON encoder module does not exist at compile time, the library
+      will compile with an error.
+    * If the JSON encoder module does not exist when `Airbrake.Worker` is
+      started, the process will not start.
+    * If the JSON encoder module _does_ exist but does not define `encode!/1`
+      when a report is made, a warning will be output to stderr and a _very_
+      simple Airbrake notice about the missing `encode!/1` function _will_ be
+      sent.  Previously, the `Airbrake.Worker` would crash and take the app down
+      with it without sending any Airbrake notices.
 
 ## v2.1.0 (??????????)
 

integration_test_apps/jason_only_app/config/config.exs

@@ -0,0 +1,4 @@
+import Config
+
+config :airbrake_client,
+  json_encoder: Jason

integration_test_apps/jason_only_app/mix.exs

@@ -20,7 +20,7 @@ defmodule JasonOnlyApp.MixProject do
   defp deps do
     [
       {:airbrake_client, ">= 0.0.0", path: "../.."},
-      {:jason, ">= 1.0.0", optional: true}
+      {:jason, ">= 1.0.0"}
     ]
   end
 end

integration_test_apps/jason_only_app/test/jason_only_app_test.exs

@@ -39,7 +39,7 @@ defmodule JasonOnlyAppTest do
                "notifier" => %{
                  "name" => "Airbrake Client",
                  "url" => "https://github.com/CityBaseInc/airbrake_client",
-                 "version" => "2.1.0"
+                 "version" => "2.2.0"
                },
                "params" => nil,
                "session" => nil
@@ -88,7 +88,7 @@ defmodule JasonOnlyAppTest do
                "notifier" => %{
                  "name" => "Airbrake Client",
                  "url" => "https://github.com/CityBaseInc/airbrake_client",
-                 "version" => "2.1.0"
+                 "version" => "2.2.0"
                },
                "params" => %{"foo" => 55},
                "session" => %{"foo" => 555}

integration_test_apps/poison_only_app/test/poison_only_app_test.exs

@@ -40,7 +40,7 @@ defmodule PoisonOnlyAppTest do
                "notifier" => %{
                  "name" => "Airbrake Client",
                  "url" => "https://github.com/CityBaseInc/airbrake_client",
-                 "version" => "2.1.0"
+                 "version" => "2.2.0"
                },
                "params" => nil,
                "session" => nil
@@ -89,7 +89,7 @@ defmodule PoisonOnlyAppTest do
                "notifier" => %{
                  "name" => "Airbrake Client",
                  "url" => "https://github.com/CityBaseInc/airbrake_client",
-                 "version" => "2.1.0"
+                 "version" => "2.2.0"
                },
                "params" => %{"foo" => 55},
                "session" => %{"foo" => 555}

lib/airbrake/payload/json_encoder.ex

@@ -0,0 +1,15 @@
+defmodule Airbrake.JSONEncoder do
+  @moduledoc false
+
+  @json_encoder Application.compile_env(:airbrake_client, :json_encoder, Poison)
+
+  def encoder do
+    @json_encoder
+  end
+
+  defmacro encode!(payload) do
+    quote do
+      unquote(@json_encoder).encode!(unquote(payload))
+    end
+  end
+end

lib/airbrake/worker.ex

@@ -2,13 +2,15 @@ defmodule Airbrake.Worker do
   @moduledoc false
   use GenServer
 
+  require Airbrake.JSONEncoder
+
+  alias Airbrake.{Config, Payload}
+
   defmodule State do
     @moduledoc false
     defstruct refs: %{}, last_exception: nil
   end
 
-  alias Airbrake.Config
-
   @name __MODULE__
   @request_headers [{"Content-Type", "application/json"}]
   @default_host "https://api.airbrake.io"
@@ -72,7 +74,15 @@ defmodule Airbrake.Worker do
     [type: inspect(exception.__struct__), message: Exception.message(exception)]
   end
 
-  def init(state), do: {:ok, state}
+  def init(state) do
+    json_encoder = Airbrake.JSONEncoder.encoder()
+
+    if Code.ensure_loaded?(json_encoder) do
+      {:ok, state}
+    else
+      {:stop, "JSON encoder #{inspect(json_encoder)} is missing"}
+    end
+  end
 
   def handle_cast({:report, exception, stacktrace, options}, %{last_exception: {exception, details}} = state) do
     enhanced_options =
@@ -109,9 +119,9 @@ defmodule Airbrake.Worker do
   defp send_report(exception, stacktrace, options) do
     unless ignore?(exception) do
       enhanced_options = build_options(options)
-      payload = Airbrake.Payload.new(exception, stacktrace, enhanced_options)
-      json_encoder = Application.get_env(:airbrake_client, :json_encoder, Poison)
-      @http_adapter.post(notify_url(), json_encoder.encode!(payload), @request_headers)
+      payload = Payload.new(exception, stacktrace, enhanced_options)
+      json_payload = encode_payload(payload)
+      @http_adapter.post(notify_url(), json_payload, @request_headers)
     end
   end
 
@@ -128,6 +138,14 @@ defmodule Airbrake.Worker do
     end
   end
 
+  defp encode_payload(%Payload{} = payload) do
+    Airbrake.JSONEncoder.encode!(payload)
+  rescue
+    UndefinedFunctionError ->
+      IO.warn("JSON encoder does not have an encode!/1 function")
+      "{\"errors\":[{\"message\":\"JSON encoder does not have an encode!/1 function\"}]}"
+  end
+
   defp get_stacktrace do
     {:current_stacktrace, stacktrace} = Process.info(self(), :current_stacktrace)
     stacktrace

mix.exs

@@ -4,7 +4,7 @@ defmodule Airbrake.Mixfile do
   def project do
     [
       app: :airbrake_client,
-      version: "2.1.0",
+      version: "2.2.0",
       elixir: "~> 1.12",
       elixirc_paths: elixirc_paths(Mix.env()),
       package: package(),

test/airbrake/payload_test.exs

@@ -48,7 +48,7 @@ defmodule Airbrake.PayloadTest do
                notifier: %{
                  name: "Airbrake Client",
                  url: "https://github.com/CityBaseInc/airbrake_client",
-                 version: "2.1.0"
+                 version: "2.2.0"
                }
              } = Payload.new(exception, stacktrace)
     end
@@ -105,7 +105,7 @@ defmodule Airbrake.PayloadTest do
                notifier: %{
                  name: "Airbrake Client",
                  url: "https://github.com/CityBaseInc/airbrake_client",
-                 version: "2.1.0"
+                 version: "2.2.0"
                }
              } = Payload.new(@exception, @stacktrace)
     end
@@ -198,7 +198,7 @@ defmodule Airbrake.PayloadTest do
                "notifier" => %{
                  "name" => "Airbrake Client",
                  "url" => "https://github.com/CityBaseInc/airbrake_client",
-                 "version" => "2.1.0"
+                 "version" => "2.2.0"
                },
                "params" => nil,
                "session" => nil
@@ -242,7 +242,7 @@ defmodule Airbrake.PayloadTest do
                "notifier" => %{
                  "name" => "Airbrake Client",
                  "url" => "https://github.com/CityBaseInc/airbrake_client",
-                 "version" => "2.1.0"
+                 "version" => "2.2.0"
                },
                "params" => %{"foo" => 55},
                "session" => %{"foo" => 555}
@@ -281,7 +281,7 @@ defmodule Airbrake.PayloadTest do
                "notifier" => %{
                  "name" => "Airbrake Client",
                  "url" => "https://github.com/CityBaseInc/airbrake_client",
-                 "version" => "2.1.0"
+                 "version" => "2.2.0"
                },
                "params" => nil,
                "session" => nil
@@ -325,7 +325,7 @@ defmodule Airbrake.PayloadTest do
                "notifier" => %{
                  "name" => "Airbrake Client",
                  "url" => "https://github.com/CityBaseInc/airbrake_client",
-                 "version" => "2.1.0"
+                 "version" => "2.2.0"
                },
                "params" => %{"foo" => 55},
                "session" => %{"foo" => 555}