Merge pull request #21 from PagerDuty/COL-1250-improve-response-handling

[COL-1250] update response handling

lib/mixduty.ex

@@ -2,6 +2,7 @@ defmodule Mixduty do
   require Logger
   alias Mixduty.Client
   alias Jason, as: JSON
+  alias Mixduty.Response
 
   def get(path, client, params \\ [], options \\ []) do
     url = endpoint() <> path <> "?" <> URI.encode_query(params)
@@ -28,41 +29,13 @@ defmodule Mixduty do
 
   def raw_request(method, url, %Client{headers: headers}, body, options) do
     HTTPoison.request(method, url, body, headers, options)
-    |> handle_response()
+    |> Response.new()
   end
 
   def raw_request(_method, _url, _client, _body, _options) do
     {:error, "Client is incorrectly configured, initialize client with correct auth token"}
   end
 
-  def handle_response({:ok, %HTTPoison.Response{status_code: code, body: resp_body}})
-      when code in 200..299 do
-    case resp_body do
-      "" -> {:ok, resp_body} |> parse_json(code)
-      _ -> JSON.decode(resp_body) |> parse_json(code)
-    end
-  end
-
-  def handle_response({:ok, %HTTPoison.Response{status_code: code, body: resp_body}})
-      when code in 400..599 do
-    {:error, resp_body}
-  end
-
-  def handle_response({:error, %HTTPoison.Error{reason: reason}}) do
-    {:error, reason}
-  end
-
-  def parse_json({:ok, parsed_response}, code) do
-    %{
-      status: code,
-      data: parsed_response
-    }
-  end
-
-  def parse_json({:error, err}, _) do
-    {:error, "Could not parse response", err}
-  end
-
   defp endpoint do
     Application.get_env(:mixduty, :base_url, "https://api.pagerduty.com/")
   end

lib/mixduty/error.ex

@@ -0,0 +1,16 @@
+defmodule Mixduty.Error do
+  @enforce_keys [:message]
+  @derive {Inspect, except: [:request]}
+
+  defstruct [
+    :message,
+    :cause,
+    :status_code
+  ]
+
+  @type t :: %__MODULE__{
+          message: String.t(),
+          cause: HTTPoison.Response.t() | HTTPoison.Error.t() | String.t(),
+          status_code: integer()
+        }
+end

lib/mixduty/response.ex

@@ -0,0 +1,146 @@
+defmodule Mixduty.Response do
+  require Logger
+  alias Mixduty.Error
+
+  alias Jason, as: JSON
+
+  @enforce_keys [:body, :headers, :status_code, :request, :request_url]
+
+  defstruct [
+    :body,
+    :headers,
+    :status_code,
+    :request,
+    :request_url
+  ]
+
+  @type headers ::
+          [{atom(), binary()}]
+          | [{binary(), binary()}]
+          | %{required(binary()) => binary()}
+          | any()
+
+  @type t :: %__MODULE__{
+          body: Map.t() | String.t(),
+          headers: headers(),
+          status_code: integer(),
+          request: HTTPoison.Request.t(),
+          request_url: binary() | any()
+        }
+
+  def new({:ok, %HTTPoison.Response{status_code: code, body: body} = resp})
+      when code in 200..299 and body in [nil, ""] do
+    resp
+    |> Map.from_struct()
+    |> Map.put(:body, %{})
+    |> put_into(__MODULE__)
+  end
+
+  def new({:ok, %HTTPoison.Response{status_code: code, body: body} = resp})
+      when code in 200..299 do
+    case json_decode(body) do
+      {:ok, decoded_body} ->
+        resp
+        |> Map.from_struct()
+        |> Map.put(:body, decoded_body)
+        |> put_into(__MODULE__)
+
+      {:error, error} ->
+        {:error, %Error{message: "JSON parse error: #{error}", status_code: code, cause: resp}}
+    end
+  end
+
+  def new({:ok, %HTTPoison.Response{headers: headers, status_code: code} = resp})
+      when code in 400..599 do
+    {:error,
+     %Error{
+       message: reason_from_status_header_or_code(headers, code),
+       status_code: code,
+       cause: resp
+     }}
+  end
+
+  def new({:ok, %HTTPoison.Response{status_code: code} = resp}) do
+    {:error, %Error{message: "Unhandled status code: #{code}", status_code: code, cause: resp}}
+  end
+
+  def new({:error, %HTTPoison.Error{reason: reason} = error}) do
+    {:error, %Error{message: "HTTP request failed. Reason: #{inspect(reason)}", cause: error}}
+  end
+
+  def put_into(response, struct_name_or_map, opts \\ [])
+
+  def put_into({:ok, %__MODULE__{body: body}}, %{} = map, opts),
+    do:
+      {:ok,
+       Map.merge(
+         map,
+         body
+         |> extract_from_container(opts[:container])
+         |> Morphix.atomorphiform!()
+       )}
+
+  def put_into({:ok, %__MODULE__{body: body}}, struct_name, opts),
+    do:
+      {:ok,
+       struct(
+         struct_name,
+         body
+         |> extract_from_container(opts[:container])
+         |> Morphix.atomorphiform!()
+       )}
+
+  def put_into(%{} = map, struct_name, _opts), do: {:ok, struct(struct_name, map)}
+
+  def put_into(error, _module, _opts), do: error
+
+  def put_all_into(response, struct_name_or_map, opts \\ [])
+
+  def put_all_into({:ok, %__MODULE__{body: body}}, %{} = map, opts) do
+    {
+      :ok,
+      body
+      |> extract_from_container(opts[:container])
+      |> Enum.map(fn item -> Map.merge(map, Morphix.atomorphiform!(item)) end)
+    }
+  end
+
+  def put_all_into({:ok, %__MODULE__{body: body}}, struct_name, opts) do
+    {
+      :ok,
+      body
+      |> extract_from_container(opts[:container])
+      |> Enum.map(fn item -> struct(struct_name, Morphix.atomorphiform!(item)) end)
+    }
+  end
+
+  def put_all_into(error, _module, _opts), do: error
+
+  ## Private
+
+  # Recursively trims whitespaces before proceeding
+  # to the next function clause matching
+  defp json_decode(" " <> str), do: json_decode(str)
+
+  defp json_decode(str)
+       when is_binary(str)
+       when str != "" do
+    case JSON.decode(str) do
+      {:ok, data} -> {:ok, data}
+      {:error, error} -> {:error, JSON.DecodeError.message(error)}
+    end
+  end
+
+  defp json_decode(_), do: nil
+
+  defp reason_from_status_header_or_code(headers, code) do
+    case Enum.find(headers, &match?({"Status", _}, &1)) do
+      {"Status", status} -> status
+      nil -> Plug.Conn.Status.reason_phrase(code)
+    end
+  end
+
+  defp extract_from_container(body, nil = _container_name), do: body
+
+  defp extract_from_container(body, container_name), do: get_in(body, [container_name])
+end

mix.exs

@@ -4,7 +4,7 @@ defmodule Mixduty.MixProject do
   def project do
     [
       app: :mixduty,
-      version: "0.1.1",
+      version: "0.2.0",
       elixir: "~> 1.11",
       start_permanent: Mix.env() == :prod,
       description: description(),
@@ -40,7 +40,9 @@ defmodule Mixduty.MixProject do
     [
       {:httpoison, "~> 1.6"},
       {:jason, "~> 1.2.2"},
-      {:ex_doc, "~> 0.24", only: :dev, runtime: false}
+      {:ex_doc, "~> 0.24", only: :dev, runtime: false},
+      {:plug_cowboy, "~> 1.0"},
+      {:morphix, "~> 0.8.0"}
     ]
   end
 end

test/mixduty_test.exs

@@ -96,16 +96,21 @@ defmodule MixdutyTest do
     test "parses a successful response" do
       response = {:ok, %HTTPoison.Response{status_code: 201, body: @success_json}}
 
-      assert %{
-               status: 201,
-               data: %{"incident" => %{"summary" => "[#1234] The server is on fire."}}
-             } = Mixduty.handle_response(response)
+      assert {:ok,
+              %Mixduty.Response{
+                body: %{"incident" => %{"summary" => "[#1234] The server is on fire."}},
+                status_code: 201
+              }} = Mixduty.Response.new(response)
     end
 
     test "parses an empty response" do
       response = {:ok, %HTTPoison.Response{status_code: 201, body: ""}}
 
-      assert %{status: 201, data: ""} = Mixduty.handle_response(response)
+      assert {:ok,
+              %Mixduty.Response{
+                body: %{},
+                status_code: 201
+              }} = Mixduty.Response.new(response)
     end
 
     test "errors on failure to parse JSON" do
@@ -118,13 +123,19 @@ defmodule MixdutyTest do
            """
          }}
 
-      assert {:error, "Could not parse response", _} = Mixduty.handle_response(response)
+      assert {:error,
+              %Mixduty.Error{
+                message: "JSON parse error: unexpected byte at position 16: 0x72 ('r')",
+                status_code: 201,
+                cause: _
+              }} = Mixduty.Response.new(response)
     end
 
     test "passes through server errors" do
       response = {:ok, %HTTPoison.Response{status_code: 400, body: "Bad Request"}}
 
-      assert {:error, "Bad Request"} = Mixduty.handle_response(response)
+      assert {:error, %Mixduty.Error{message: "Bad Request", status_code: 400, cause: _}} =
+               Mixduty.Response.new(response)
     end
   end
 end