Update SendGrid Adapter to return ok/error tuple (#572)

What changed?
==============

We update the `SendGridAdapter` to abide by the new Adapter behaviour.
The adapter now returns an `{:ok, email}` or `{:error, error}`.

As part of this work, we create the `build_api_error` function to
replace the `raise_api_error` function.

Note on implementation with throw/catch
---------------------------------------

Building SendGrid's body requires a lot of deeply nested data
manipulation. Nested deep within some of the branching logic was some
code that raised errors.

Rather than returning an error tuple at a really low level and have to
bubble up the error all the way til it's returned, we opted for a
simpler approach: throwing and catching the errors.

We throw and catch `{:error, error}` when we used to `raise error`.

It's possible we could do something different in the future, like
bubbling up the errors and returning them without having to resort to
`throw` and `catch`, but right now we want to minimize the number of
changes for these `raise`s deep in the call stack.

Raising (not returning) configuration errors
--------------------------------

While working on this, an important question came up. Should we raise or
return an error because of a missing API key? Currently we raise.

I think it's okay to raise an error in that case because it's a
configuration issue. It's not the same type of error that happens when
you can't build an email, deliver it, or even get a response that isn't
200. That error is a configuration issue and so the sooner we notify the
user of Bamboo, the better.

We can also think about it this way: we want to return an ok/error tuple
because we people want to handle email building or delivery issues.
Having an API key missing isn't that kind of issue. It means the library
is configured incorrectly, so it seems good to raise an error.

lib/bamboo/adapters/send_grid_adapter.ex

@@ -47,19 +47,26 @@ defmodule Bamboo.SendGridAdapter do
 
   def deliver(email, config) do
     api_key = get_key(config)
-    body = email |> to_sendgrid_body(config) |> Bamboo.json_library().encode!()
-    url = [base_uri(), @send_message_path]
 
-    case :hackney.post(url, headers(api_key), body, AdapterHelper.hackney_opts(config)) do
-      {:ok, status, _headers, response} when status > 299 ->
-        filtered_params = body |> Bamboo.json_library().decode!() |> Map.put("key", "[FILTERED]")
-        raise_api_error(@service_name, response, filtered_params)
+    try do
+      body = email |> to_sendgrid_body(config) |> Bamboo.json_library().encode!()
+      url = [base_uri(), @send_message_path]
 
-      {:ok, status, headers, response} ->
-        %{status_code: status, headers: headers, body: response}
+      case :hackney.post(url, headers(api_key), body, AdapterHelper.hackney_opts(config)) do
+        {:ok, status, _headers, response} when status > 299 ->
+          filtered_params =
+            body |> Bamboo.json_library().decode!() |> Map.put("key", "[FILTERED]")
 
-      {:error, reason} ->
-        raise_api_error(inspect(reason))
+          {:error, build_api_error(@service_name, response, filtered_params)}
+
+        {:ok, status, headers, response} ->
+          {:ok, %{status_code: status, headers: headers, body: response}}
+
+        {:error, reason} ->
+          {:error, build_api_error(inspect(reason))}
+      end
+    catch
+      :throw, {:error, _} = error -> error
     end
   end
 
@@ -168,7 +175,7 @@ defmodule Bamboo.SendGridAdapter do
   end
 
   defp build_personalization(_personalization) do
-    raise "Each personalization requires a 'to' field"
+    throw({:error, "Each personalization requires a 'to' field"})
   end
 
   defp map_put_if(map_out, map_in, key, mapper \\ & &1) do
@@ -381,7 +388,7 @@ defmodule Bamboo.SendGridAdapter do
   defp cast_time(unix_timestamp) when is_integer(unix_timestamp), do: unix_timestamp
 
   defp cast_time(_other) do
-    raise "expected 'send_at' time parameter to be a DateTime or unix timestamp"
+    throw({:error, "expected 'send_at' time parameter to be a DateTime or unix timestamp"})
   end
 
   defp cast_addresses(addresses, type) when is_list(addresses) do
@@ -410,7 +417,7 @@ defmodule Bamboo.SendGridAdapter do
     case {Map.get(address, :name, Map.get(address, "name")),
           Map.get(address, :email, Map.get(address, "email"))} do
       {_name, nil} ->
-        raise "Must specify at least an 'email' field in map #{inspect(address)}"
+        throw({:error, "Must specify at least an 'email' field in map #{inspect(address)}"})
 
       {nil, address} ->
         %{email: address}

test/lib/bamboo/adapters/send_grid_adapter_test.exs

@@ -122,6 +122,12 @@ defmodule Bamboo.SendGridAdapterTest do
     end
   end
 
+  test "deliver/2 returns an {:ok, response}" do
+    {:ok, response} = new_email() |> SendGridAdapter.deliver(@config)
+
+    assert %{status_code: 200, headers: _, body: _} = response
+  end
+
   test "deliver/2 sends the to the right url" do
     new_email() |> SendGridAdapter.deliver(@config)
 
@@ -565,10 +571,9 @@ defmodule Bamboo.SendGridAdapterTest do
       |> Email.put_header("Reply-To", "reply@foo.com")
       |> Bamboo.SendGridHelper.add_personalizations([%{subject: "This will fail"}])
 
-    assert_raise RuntimeError, ~r/'to' field/, fn ->
-      email
-      |> SendGridAdapter.deliver(@config)
-    end
+    {:error, msg} = email |> SendGridAdapter.deliver(@config)
+
+    assert msg =~ ~r/'to' field/
   end
 
   test "deliver/2 personalization send_at field must be either DateTime or epoch timestamp" do
@@ -577,10 +582,9 @@ defmodule Bamboo.SendGridAdapterTest do
       |> Email.put_header("Reply-To", "reply@foo.com")
       |> Bamboo.SendGridHelper.add_personalizations([%{to: "foo@bar.com", send_at: "now"}])
 
-    assert_raise RuntimeError, ~r/'send_at' time/, fn ->
-      email
-      |> SendGridAdapter.deliver(@config)
-    end
+    {:error, msg} = email |> SendGridAdapter.deliver(@config)
+
+    assert msg =~ ~r/'send_at' time/
   end
 
   test "deliver/2 correctly formats email addresses in personalizations" do
@@ -619,10 +623,9 @@ defmodule Bamboo.SendGridAdapterTest do
       |> Email.put_header("Reply-To", "reply@foo.com")
       |> Bamboo.SendGridHelper.add_personalizations([%{to: %{"name" => "Lou"}, send_at: "now"}])
 
-    assert_raise RuntimeError, ~r/'email' field/, fn ->
-      email
-      |> SendGridAdapter.deliver(@config)
-    end
+    {:error, msg} = email |> SendGridAdapter.deliver(@config)
+
+    assert msg =~ ~r/'email' field/
   end
 
   test "deliver/2 will set sandbox mode correctly" do
@@ -645,20 +648,20 @@ defmodule Bamboo.SendGridAdapterTest do
     assert params["mail_settings"]["bypass_list_management"]["enable"] == true
   end
 
-  test "raises if the response is not a success" do
+  test "returns an error if the response is not a success" do
     email = new_email(from: "INVALID_EMAIL")
 
-    assert_raise Bamboo.ApiError, fn ->
-      email |> SendGridAdapter.deliver(@config)
-    end
+    {:error, error} = email |> SendGridAdapter.deliver(@config)
+
+    assert %Bamboo.ApiError{} = error
   end
 
   test "removes api key from error output" do
     email = new_email(from: "INVALID_EMAIL")
 
-    assert_raise Bamboo.ApiError, ~r/"key" => "\[FILTERED\]"/, fn ->
-      email |> SendGridAdapter.deliver(@config)
-    end
+    {:error, error} = email |> SendGridAdapter.deliver(@config)
+
+    assert error.message =~ ~r/"key" => "\[FILTERED\]"/
   end
 
   defp new_email(attrs \\ []) do