[Fix] ErrorStorage agent calls (#98)

* improve agent.get call

* refactor reset error count

* test reset return valule

* fix error storage specs

* fixes from review

* rename ErrorStorage.reset/2 to ErrorStorage.reset_stats/2

* rename ErrorStorage.get_error_stats/1 to ErrorStorage.get_stats/1

Author: joaquinco

lib/boom_notifier/error_storage.ex

@@ -37,19 +37,17 @@ defmodule BoomNotifier.ErrorStorage do
     %{key: error_hash_key} = error_info
     timestamp = error_info.timestamp || DateTime.utc_now()
 
-    default_error_storage_info = %__MODULE__{
-      accumulated_occurrences: 1,
-      first_occurrence: timestamp,
-      last_occurrence: timestamp,
-      __max_storage_capacity__: 1
-    }
+    initial_error_storage =
+      timestamp
+      |> build_error_storage()
+      |> Map.put(:accumulated_occurrences, 1)
 
     Agent.update(
       :boom_notifier,
       &Map.update(
         &1,
         error_hash_key,
-        default_error_storage_info,
+        initial_error_storage,
         fn error_storage_item ->
           error_storage_item
           |> Map.update!(:accumulated_occurrences, fn current -> current + 1 end)
@@ -65,12 +63,11 @@ defmodule BoomNotifier.ErrorStorage do
   @doc """
   Given an error info, it returns the aggregated info stored in the agent.
   """
-  @spec get_error_stats(ErrorInfo.t()) :: %__MODULE__{}
-  def get_error_stats(error_info) do
+  @spec get_stats(ErrorInfo.t()) :: __MODULE__.t() | nil
+  def get_stats(error_info) do
     %{key: error_hash_key} = error_info
 
-    Agent.get(:boom_notifier, fn state -> state end)
-    |> Map.get(error_hash_key)
+    Agent.get(:boom_notifier, &Map.get(&1, error_hash_key))
   end
 
   @doc """
@@ -81,53 +78,55 @@ defmodule BoomNotifier.ErrorStorage do
   """
   @spec send_notification?(ErrorInfo.t()) :: boolean()
   def send_notification?(error_info) do
-    %{key: error_hash_key} = error_info
-
-    error_storage_item =
-      Agent.get(:boom_notifier, fn state -> state end)
-      |> Map.get(error_hash_key)
-
-    do_send_notification?(error_storage_item)
+    error_info
+    |> get_stats()
+    |> do_send_notification?()
   end
 
   @doc """
   Reset the accumulated_occurrences for the given error info to zero. It also
   increments the max storage capacity based on the notification strategy.
+
+  Returns error storage entry before reset
   """
-  @spec reset_accumulated_errors(error_strategy, ErrorInfo.t()) :: :ok
-  def reset_accumulated_errors(:exponential, error_info) do
-    %{key: error_hash_key} = error_info
+  @spec reset_stats(ErrorInfo.t()) :: __MODULE__.t()
+  @spec reset_stats(ErrorInfo.t(), count_strategy :: :exponential | any()) :: __MODULE__.t()
+  def reset_stats(error_info), do: reset_stats(error_info, nil)
 
-    Agent.update(
-      :boom_notifier,
-      &Map.update!(&1, error_hash_key, fn error_storage_item ->
-        clear_values(error_storage_item)
-        |> Map.update!(:__max_storage_capacity__, fn current -> current * 2 end)
-      end)
-    )
+  def reset_stats(error_info, :exponential) do
+    reset_state(error_info, fn value -> value * 2 end)
   end
 
-  def reset_accumulated_errors([exponential: [limit: limit]], error_info) do
-    %{key: error_hash_key} = error_info
+  def reset_stats(error_info, exponential: [limit: limit]) do
+    reset_state(error_info, fn value -> min(value * 2, limit) end)
+  end
 
-    Agent.update(
-      :boom_notifier,
-      &Map.update!(&1, error_hash_key, fn error_storage_item ->
-        clear_values(error_storage_item)
-        |> Map.update!(:__max_storage_capacity__, fn current -> min(current * 2, limit) end)
-      end)
-    )
+  def reset_stats(error_info, _) do
+    reset_state(error_info, fn _ -> 1 end)
   end
 
-  def reset_accumulated_errors(:always, error_info) do
+  defp reset_state(error_info, limit_updater_function) do
     %{key: error_hash_key} = error_info
 
-    Agent.update(
+    Agent.get_and_update(
       :boom_notifier,
-      &Map.update!(&1, error_hash_key, fn error_storage_item ->
-        clear_values(error_storage_item)
-        |> Map.replace!(:__max_storage_capacity__, 1)
-      end)
+      fn state ->
+        error_storage_item = Map.get(state, error_hash_key)
+
+        state =
+          Map.update(
+            state,
+            error_hash_key,
+            build_error_storage(),
+            fn error_storage_item ->
+              error_storage_item
+              |> clear_values()
+              |> Map.update!(:__max_storage_capacity__, limit_updater_function)
+            end
+          )
+
+        {error_storage_item, state}
+      end
     )
   end
 
@@ -138,7 +137,7 @@ defmodule BoomNotifier.ErrorStorage do
     |> Map.replace!(:last_occurrence, nil)
   end
 
-  @spec do_send_notification?(ErrorInfo.t() | nil) :: boolean()
+  @spec do_send_notification?(nil | __MODULE__.t()) :: boolean()
   defp do_send_notification?(nil), do: false
 
   defp do_send_notification?(error_storage_item) do
@@ -147,4 +146,13 @@ defmodule BoomNotifier.ErrorStorage do
 
     accumulated_occurrences >= max_storage_capacity
   end
+
+  defp build_error_storage(timestamp \\ nil) do
+    %__MODULE__{
+      accumulated_occurrences: 0,
+      first_occurrence: timestamp,
+      last_occurrence: timestamp,
+      __max_storage_capacity__: 1
+    }
+  end
 end

lib/boom_notifier/notification_sender.ex

@@ -30,13 +30,12 @@ defmodule BoomNotifier.NotificationSender do
 
   def notify_all(settings, error_info) do
     notification_trigger = Keyword.get(settings, :notification_trigger, :always)
-    occurrences = Map.put(error_info, :occurrences, ErrorStorage.get_error_stats(error_info))
-
-    ErrorStorage.reset_accumulated_errors(notification_trigger, error_info)
+    occurrences = ErrorStorage.reset_stats(error_info, notification_trigger)
+    error_info = Map.put(error_info, :occurrences, occurrences)
 
     BoomNotifier.walkthrough_notifiers(
       settings,
-      fn notifier, options -> notify(notifier, occurrences, options) end
+      fn notifier, options -> notify(notifier, error_info, options) end
     )
   end
 

test/example_app/mix.lock

@@ -14,7 +14,7 @@
   "idna": {:hex, :idna, "6.1.1", "8a63070e9f7d0c62eb9d9fcb360a7de382448200fbbd1b106cc96d3d8099df8d", [:rebar3], [{:unicode_util_compat, "~>0.7.0", [hex: :unicode_util_compat, repo: "hexpm", optional: false]}], "hexpm", "92376eb7894412ed19ac475e4a86f7b413c1b9fbb5bd16dccd57934157944cea"},
   "jason": {:hex, :jason, "1.4.1", "af1504e35f629ddcdd6addb3513c3853991f694921b1b9368b0bd32beb9f1b63", [:mix], [{:decimal, "~> 1.0 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "fbb01ecdfd565b56261302f7e1fcc27c4fb8f32d56eab74db621fc154604a7a1"},
   "metrics": {:hex, :metrics, "1.0.1", "25f094dea2cda98213cecc3aeff09e940299d950904393b2a29d191c346a8486", [:rebar3], [], "hexpm", "69b09adddc4f74a40716ae54d140f93beb0fb8978d8636eaded0c31b6f099f16"},
-  "mime": {:hex, :mime, "2.0.5", "dc34c8efd439abe6ae0343edbb8556f4d63f178594894720607772a041b04b02", [:mix], [], "hexpm", "da0d64a365c45bc9935cc5c8a7fc5e49a0e0f9932a761c55d6c52b142780a05c"},
+  "mime": {:hex, :mime, "2.0.6", "8f18486773d9b15f95f4f4f1e39b710045fa1de891fada4516559967276e4dc2", [:mix], [], "hexpm", "c9945363a6b26d747389aac3643f8e0e09d30499a138ad64fe8fd1d13d9b153e"},
   "mimerl": {:hex, :mimerl, "1.2.0", "67e2d3f571088d5cfd3e550c383094b47159f3eee8ffa08e64106cdf5e981be3", [:rebar3], [], "hexpm", "f278585650aa581986264638ebf698f8bb19df297f66ad91b18910dfc6e19323"},
   "parse_trans": {:hex, :parse_trans, "3.4.1", "6e6aa8167cb44cc8f39441d05193be6e6f4e7c2946cb2759f015f8c56b76e5ff", [:rebar3], [], "hexpm", "620a406ce75dada827b82e453c19cf06776be266f5a67cff34e1ef2cbb60e49a"},
   "phoenix": {:hex, :phoenix, "1.6.6", "281c8ce8dccc9f60607346b72cdfc597c3dde134dd9df28dff08282f0b751754", [:mix], [{:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:phoenix_pubsub, "~> 2.0", [hex: :phoenix_pubsub, repo: "hexpm", optional: false]}, {:phoenix_view, "~> 1.0", [hex: :phoenix_view, repo: "hexpm", optional: false]}, {:plug, "~> 1.10", [hex: :plug, repo: "hexpm", optional: false]}, {:plug_cowboy, "~> 2.2", [hex: :plug_cowboy, repo: "hexpm", optional: true]}, {:plug_crypto, "~> 1.2", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "807bd646e64cd9dc83db016199715faba72758e6db1de0707eef0a2da4924364"},
@@ -23,13 +23,13 @@
   "phoenix_live_view": {:hex, :phoenix_live_view, "0.17.7", "05a42377075868a678d446361effba80cefef19ab98941c01a7a4c7560b29121", [:mix], [{:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:phoenix, "~> 1.5.9 or ~> 1.6.0", [hex: :phoenix, repo: "hexpm", optional: false]}, {:phoenix_html, "~> 3.1", [hex: :phoenix_html, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4.2 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "25eaf41028eb351b90d4f69671874643a09944098fefd0d01d442f40a6091b6f"},
   "phoenix_pubsub": {:hex, :phoenix_pubsub, "2.0.0", "a1ae76717bb168cdeb10ec9d92d1480fec99e3080f011402c0a2d68d47395ffb", [:mix], [], "hexpm", "c52d948c4f261577b9c6fa804be91884b381a7f8f18450c5045975435350f771"},
   "phoenix_view": {:hex, :phoenix_view, "1.1.2", "1b82764a065fb41051637872c7bd07ed2fdb6f5c3bd89684d4dca6e10115c95a", [:mix], [{:phoenix_html, "~> 2.14.2 or ~> 3.0", [hex: :phoenix_html, repo: "hexpm", optional: true]}], "hexpm", "7ae90ad27b09091266f6adbb61e1d2516a7c3d7062c6789d46a7554ec40f3a56"},
-  "plug": {:hex, :plug, "1.13.4", "addb6e125347226e3b11489e23d22a60f7ab74786befb86c14f94fb5f23ca9a4", [:mix], [{:mime, "~> 1.0 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:plug_crypto, "~> 1.1.1 or ~> 1.2", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4.3 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "06114c1f2a334212fe3ae567dbb3b1d29fd492c1a09783d52f3d489c1a6f4cf2"},
+  "plug": {:hex, :plug, "1.16.1", "40c74619c12f82736d2214557dedec2e9762029b2438d6d175c5074c933edc9d", [:mix], [{:mime, "~> 1.0 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:plug_crypto, "~> 1.1.1 or ~> 1.2 or ~> 2.0", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4.3 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "a13ff6b9006b03d7e33874945b2755253841b238c34071ed85b0e86057f8cddc"},
   "plug_cowboy": {:hex, :plug_cowboy, "2.5.2", "62894ccd601cf9597e2c23911ff12798a8a18d237e9739f58a6b04e4988899fe", [:mix], [{:cowboy, "~> 2.7", [hex: :cowboy, repo: "hexpm", optional: false]}, {:cowboy_telemetry, "~> 0.3", [hex: :cowboy_telemetry, repo: "hexpm", optional: false]}, {:plug, "~> 1.7", [hex: :plug, repo: "hexpm", optional: false]}], "hexpm", "ea6e87f774c8608d60c8d34022a7d073bd7680a0a013f049fc62bf35efea1044"},
-  "plug_crypto": {:hex, :plug_crypto, "1.2.2", "05654514ac717ff3a1843204b424477d9e60c143406aa94daf2274fdd280794d", [:mix], [], "hexpm", "87631c7ad914a5a445f0a3809f99b079113ae4ed4b867348dd9eec288cecb6db"},
+  "plug_crypto": {:hex, :plug_crypto, "1.2.5", "918772575e48e81e455818229bf719d4ab4181fcbf7f85b68a35620f78d89ced", [:mix], [], "hexpm", "26549a1d6345e2172eb1c233866756ae44a9609bd33ee6f99147ab3fd87fd842"},
   "ranch": {:hex, :ranch, "1.8.0", "8c7a100a139fd57f17327b6413e4167ac559fbc04ca7448e9be9057311597a1d", [:make, :rebar3], [], "hexpm", "49fbcfd3682fab1f5d109351b61257676da1a2fdbe295904176d5e521a2ddfe5"},
   "ssl_verify_fun": {:hex, :ssl_verify_fun, "1.1.7", "354c321cf377240c7b8716899e182ce4890c5938111a1296add3ec74cf1715df", [:make, :mix, :rebar3], [], "hexpm", "fe4c190e8f37401d30167c8c405eda19469f34577987c76dde613e838bbc67f8"},
   "swoosh": {:hex, :swoosh, "1.6.3", "598d3f07641004bedb3eede40057760ae18be1073cff72f079ca1e1fc9cd97b9", [:mix], [{:cowboy, "~> 1.1 or ~> 2.4", [hex: :cowboy, repo: "hexpm", optional: true]}, {:finch, "~> 0.6", [hex: :finch, repo: "hexpm", optional: true]}, {:gen_smtp, "~> 0.13 or ~> 1.0", [hex: :gen_smtp, repo: "hexpm", optional: true]}, {:hackney, "~> 1.9", [hex: :hackney, repo: "hexpm", optional: true]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}, {:mail, "~> 0.2", [hex: :mail, repo: "hexpm", optional: true]}, {:mime, "~> 1.1 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:plug_cowboy, ">= 1.0.0", [hex: :plug_cowboy, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.4.2 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "81ff9d7c7c4005a57465a7eb712edd71db51829aef94c8a34c30c5b9e9964adf"},
-  "telemetry": {:hex, :telemetry, "1.2.1", "68fdfe8d8f05a8428483a97d7aab2f268aaff24b49e0f599faa091f1d4e7f61c", [:rebar3], [], "hexpm", "dad9ce9d8effc621708f99eac538ef1cbe05d6a874dd741de2e689c47feafed5"},
+  "telemetry": {:hex, :telemetry, "1.3.0", "fedebbae410d715cf8e7062c96a1ef32ec22e764197f70cda73d82778d61e7a2", [:rebar3], [], "hexpm", "7015fc8919dbe63764f4b4b87a95b7c0996bd539e0d499be6ec9d7f3875b79e6"},
   "telemetry_metrics": {:hex, :telemetry_metrics, "0.6.1", "315d9163a1d4660aedc3fee73f33f1d355dcc76c5c3ab3d59e76e3edf80eef1f", [:mix], [{:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "7be9e0871c41732c233be71e4be11b96e56177bf15dde64a8ac9ce72ac9834c6"},
   "telemetry_poller": {:hex, :telemetry_poller, "1.0.0", "db91bb424e07f2bb6e73926fcafbfcbcb295f0193e0a00e825e589a0a47e8453", [:rebar3], [{:telemetry, "~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "b3a24eafd66c3f42da30fc3ca7dda1e9d546c12250a2d60d7b81d264fbec4f6e"},
   "tesla": {:hex, :tesla, "1.7.0", "a62dda2f80d4f8a925eb7b8c5b78c461e0eb996672719fe1a63b26321a5f8b4e", [:mix], [{:castore, "~> 0.1 or ~> 1.0", [hex: :castore, repo: "hexpm", optional: true]}, {:exjsx, ">= 3.0.0", [hex: :exjsx, repo: "hexpm", optional: true]}, {:finch, "~> 0.13", [hex: :finch, repo: "hexpm", optional: true]}, {:fuse, "~> 2.4", [hex: :fuse, repo: "hexpm", optional: true]}, {:gun, "~> 1.3", [hex: :gun, repo: "hexpm", optional: true]}, {:hackney, "~> 1.6", [hex: :hackney, repo: "hexpm", optional: true]}, {:ibrowse, "4.4.0", [hex: :ibrowse, repo: "hexpm", optional: true]}, {:jason, ">= 1.0.0", [hex: :jason, repo: "hexpm", optional: true]}, {:mime, "~> 1.0 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:mint, "~> 1.0", [hex: :mint, repo: "hexpm", optional: true]}, {:msgpax, "~> 2.3", [hex: :msgpax, repo: "hexpm", optional: true]}, {:poison, ">= 1.0.0", [hex: :poison, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: true]}], "hexpm", "2e64f01ebfdb026209b47bc651a0e65203fcff4ae79c11efb73c4852b00dc313"},