From 4bc74aeee01503c0b1e331d3a4cc5885ebf197e8 Mon Sep 17 00:00:00 2001
From: Jason Draper <jason@drapergeek.com>
Date: Tue, 28 Feb 2017 11:18:33 -0500
Subject: [PATCH] Set up basic error handling for set

If there is a problem storing the session, this will retry 5 times and then raise an error with (hopefully) useful information for the developer to look into.
---
 lib/redbird/plug/session/redis.ex | 43 ++++++++++++++++++++++++++-----
 mix.exs                           |  1 +
 mix.lock                          |  2 ++
 test/redbird_test.exs             | 12 +++++++++
 4 files changed, 51 insertions(+), 7 deletions(-)

diff --git a/lib/redbird/plug/session/redis.ex b/lib/redbird/plug/session/redis.ex
index 4f2cde0..5e9a584 100644
--- a/lib/redbird/plug/session/redis.ex
+++ b/lib/redbird/plug/session/redis.ex
@@ -25,15 +25,27 @@ defmodule Plug.Session.REDIS do
     put(conn, add_namespace(generate_random_key()), data, init_options)
   end
   def put(_conn, namespaced_key, data, init_options) do
-    setex(%{
-      key: namespaced_key,
-      value: data,
-      seconds: session_expiration(init_options)
-    })
-    namespaced_key
+    set_key_with_retries(
+      namespaced_key,
+      data,
+      session_expiration(init_options),
+      1
+    )
   end
 
-  def delete(_conn, redis_key, _kinit_options) do
+  defp set_key_with_retries(key, data, seconds, counter) do
+    case setex(%{key: key, value: data, seconds: seconds}) do
+      :ok -> key
+      response ->
+        if counter > 5 do
+          Redbird.RedisError.raise(error: response, key: key)
+        else
+          set_key_with_retries(key, data, seconds, counter + 1)
+        end
+    end
+  end
+
+  def delete(_conn, redis_key, _init_options) do
     del(redis_key)
     :ok
   end
@@ -61,3 +73,20 @@ defmodule Plug.Session.REDIS do
     end
   end
 end
+
+defmodule Redbird.RedisError do
+  defexception [:message]
+
+  def raise([error: error, key: key]) do
+    message = "#{base_message} Redis Error: #{error} key: #{key}"
+    raise __MODULE__, message
+  end
+
+  def exception(message) do
+    %__MODULE__{message: message}
+  end
+
+  defp base_message do
+    "Redbird was unable to store the session in redis."
+  end
+end
diff --git a/mix.exs b/mix.exs
index 1b4b5aa..9f06e21 100644
--- a/mix.exs
+++ b/mix.exs
@@ -28,6 +28,7 @@ defmodule Redbird.Mixfile do
   defp deps do
     [
       {:ex_doc, "~> 0.13", only: :dev},
+      {:mock, "~> 0.2.0", only: :test},
       {:exredis, "~> 0.2"},
       {:plug, "~> 1.1"},
     ]
diff --git a/mix.lock b/mix.lock
index d06cb72..76df7c8 100644
--- a/mix.lock
+++ b/mix.lock
@@ -2,5 +2,7 @@
   "eredis": {:hex, :eredis, "1.0.8", "ab4fda1c4ba7fbe6c19c26c249dc13da916d762502c4b4fa2df401a8d51c5364", [:rebar], []},
   "ex_doc": {:hex, :ex_doc, "0.14.5", "c0433c8117e948404d93ca69411dd575ec6be39b47802e81ca8d91017a0cf83c", [:mix], [{:earmark, "~> 1.0", [hex: :earmark, optional: false]}]},
   "exredis": {:hex, :exredis, "0.2.5", "34d02fa9ef32174cbd790b166746f91b3dbc131ce37a2d9a2451e8bba164b423", [:mix], [{:eredis, ">= 1.0.8", [hex: :eredis, optional: false]}]},
+  "meck": {:hex, :meck, "0.8.4", "59ca1cd971372aa223138efcf9b29475bde299e1953046a0c727184790ab1520", [:make, :rebar], []},
   "mime": {:hex, :mime, "1.0.1", "05c393850524767d13a53627df71beeebb016205eb43bfbd92d14d24ec7a1b51", [:mix], []},
+  "mock": {:hex, :mock, "0.2.1", "bfdba786903e77f9c18772dee472d020ceb8ef000783e737725a4c8f54ad28ec", [:mix], [{:meck, "~> 0.8.2", [hex: :meck, optional: false]}]},
   "plug": {:hex, :plug, "1.3.0", "6e2b01afc5db3fd011ca4a16efd9cb424528c157c30a44a0186bcc92c7b2e8f3", [:mix], [{:cowboy, "~> 1.0.1 or ~> 1.1", [hex: :cowboy, optional: true]}, {:mime, "~> 1.0", [hex: :mime, optional: false]}]}}
diff --git a/test/redbird_test.exs b/test/redbird_test.exs
index 4a79290..79abe00 100644
--- a/test/redbird_test.exs
+++ b/test/redbird_test.exs
@@ -2,6 +2,7 @@ defmodule RedbirdTest do
   use ExUnit.Case, async: true
   use Plug.Test
   alias Plug.Session.REDIS
+  import Mock
 
   @default_opts [
     store: :redis,
@@ -84,6 +85,17 @@ defmodule RedbirdTest do
 
       assert conn |> get_session(:foo) |> is_nil
     end
+
+    test "it throws an exception after multiple attempts to store and fail" do
+      with_mock Redbird.Redis, [setex: fn(_) -> "FAIL" end] do
+        assert_raise Redbird.RedisError, ~r/Redbird was unable to store the session in redis. Redis Error: FAIL/, fn ->
+          conn(:get, "/")
+          |> sign_conn
+          |> put_session(:foo, "bar")
+          |> send_resp(200, "")
+        end
+      end
+    end
   end
 
   describe "delete" do