Merge pull request #138 from pow-auth/improve-errors

Improve errors

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## v0.2.8 (TBA)
+
+- More expressive errors now including the whole HTTP response where applicable
+
 ## v0.2.7 (2023-09-12)
 
 * `Assent.Strategy.Strava` added

lib/assent.ex

@@ -6,74 +6,81 @@ defmodule Assent do
   end
 
   defmodule CallbackCSRFError do
-    defexception [:message]
+    defexception [:key]
 
-    @spec new(binary()) :: %__MODULE__{}
-    def new(key) do
-      %__MODULE__{message: "CSRF detected with param key #{inspect key}"}
+    def message(exception) do
+      "CSRF detected with param key #{inspect exception.key}"
     end
   end
 
   defmodule MissingParamError do
-    defexception [:message, :params]
-
-    @spec new(binary(), map()) :: %__MODULE__{}
-    def new(key, params) do
-      %__MODULE__{
-        message: "Expected #{inspect key} to exist in params, but only found the following keys: #{inspect Map.keys(params)}",
-        params: params
-      }
+    defexception [:expected_key, :params]
+
+    def message(exception) do
+      expected_key = inspect exception.expected_key
+      params = inspect Map.keys(exception.params)
+
+      "Expected #{expected_key} in params, got: #{params}"
     end
   end
 
   defmodule RequestError do
-    defexception [:message, :error]
+    defexception [:message, :response]
+
+    alias Assent.HTTPAdapter.HTTPResponse
+
+    def message(exception) do
+      """
+      #{exception.message}
+
+      #{HTTPResponse.format(exception.response)}
+      """
+    end
+  end
+
+  defmodule InvalidResponseError do
+    defexception [:response]
 
     alias Assent.HTTPAdapter.HTTPResponse
 
-    @spec unexpected(HTTPResponse.t()) :: %__MODULE__{}
-    def unexpected(response) do
-      %__MODULE__{
-        message:
-          """
-          An unexpected success response was received:
-
-          #{inspect response.body}
-          """,
-        error: :unexpected_response
-      }
+    def message(exception) do
+      """
+      An invalid response was received.
+
+      #{HTTPResponse.format(exception.response)}
+      """
     end
+  end
+
+  defmodule UnexpectedResponseError do
+    defexception [:response]
 
-    @spec invalid(HTTPResponse.t()) :: %__MODULE__{}
-    def invalid(response) do
-      %__MODULE__{
-        message:
-          """
-          Server responded with status: #{response.status}
+    alias Assent.HTTPAdapter.HTTPResponse
 
-          Headers:#{Enum.reduce(response.headers, "", fn {k, v}, acc -> acc <> "\n#{k}: #{v}" end)}
+    def message(exception) do
+      """
+      An unexpected response was received.
 
-          Body:
-          #{inspect response.body}
-          """,
-        error: :invalid_server_response
-      }
+      #{HTTPResponse.format(exception.response)}
+      """
     end
+  end
+
+  defmodule ServerUnreachableError do
+    defexception [:http_adapter, :request_url, :reason]
+
+    def message(exception) do
+      [url | _rest] = String.split(exception.request_url, "?", parts: 2)
 
-    @spec unreachable(atom(), binary(), term()) :: %__MODULE__{}
-    def unreachable(adapter, url, error) do
-      %__MODULE__{
-        message:
-          """
-          Server was unreachable with #{inspect adapter}.
+      """
+      The server was unreachable.
 
-          Failed with the following error:
-          #{inspect error}
+      HTTP Adapter: #{inspect exception.http_adapter}
+      Request URL: #{url}
 
-          URL: #{url}
-          """,
-        error: :unreachable
-      }
+      Reason:
+      #{inspect exception.reason}
+      """
     end
   end
 

lib/assent/config.ex

@@ -6,7 +6,11 @@ defmodule Assent.Config do
   defmodule MissingKeyError do
     @type t :: %__MODULE__{}
 
-    defexception [:message]
+    defexception [:key]
+
+    def message(exception) do
+      "Key #{inspect exception.key} not found in config"
+    end
   end
 
   @type t :: Keyword.t()
@@ -18,7 +22,7 @@ defmodule Assent.Config do
   def fetch(config, key) do
     case Keyword.fetch(config, key) do
       {:ok, value} -> {:ok, value}
-      :error       -> {:error, MissingKeyError.exception("Key `:#{key}` not found in config")}
+      :error       -> {:error, MissingKeyError.exception(key: key)}
     end
   end
 

lib/assent/http_adapter.ex

@@ -21,12 +21,31 @@ defmodule Assent.HTTPAdapter do
 
     @type header :: {binary(), binary()}
     @type t      :: %__MODULE__{
+      http_adapter: atom(),
+      request_url: binary(),
       status: integer(),
       headers: [header()],
       body: binary() | term()
     }
 
-    defstruct status: 200, headers: [], body: ""
+    defstruct http_adapter: nil, request_url: nil, status: 200, headers: [], body: ""
+
+    def format(response) do
+      [request_url | _rest] = String.split(response.request_url, "?", parts: 2)
+
+      """
+      HTTP Adapter: #{inspect response.http_adapter}
+      Request URL: #{request_url}
+
+      Response status: #{response.status}
+
+      Response headers:
+      #{Enum.reduce(response.headers, "", fn {k, v}, acc -> acc <> "\n#{k}: #{v}" end)}
+
+      Response body:
+      #{inspect response.body}
+      """
+    end
   end
 
   @type method :: :get | :post

lib/assent/jwt_adapter/assent_jwt.ex

@@ -29,7 +29,7 @@ defmodule Assent.JWTAdapter.AssentJWT do
 
     case encode_json_base64(header, opts) do
       {:ok, encoded_header} -> {:ok, encoded_header}
-      {:error, error}       -> {:error, %Error{message: "Failed to encode header", reason: error, data: header}}
+      {:error, error}       -> {:error, Error.exception(message: "Failed to encode header", reason: error, data: header)}
     end
   end
 
@@ -43,7 +43,7 @@ defmodule Assent.JWTAdapter.AssentJWT do
   defp encode_claims(claims, opts) do
     case encode_json_base64(claims, opts) do
       {:ok, encoded_claims} -> {:ok, encoded_claims}
-      {:error, error}       -> {:error, %Error{message: "Failed to encode claims", reason: error, data: claims}}
+      {:error, error}       -> {:error, Error.exception(message: "Failed to encode claims", reason: error, data: claims)}
     end
   end
 
@@ -52,7 +52,7 @@ defmodule Assent.JWTAdapter.AssentJWT do
 
     case sign_message(message, alg, secret_or_private_key) do
       {:ok, signature} -> {:ok, "#{message}.#{Base.url_encode64(signature, padding: false)}"}
-      {:error, error} -> {:error, %Error{message: "Failed to sign JWT", reason: error, data: {message, alg}}}
+      {:error, error} -> {:error, Error.exception(message: "Failed to sign JWT", reason: error, data: {message, alg})}
     end
   end
 
@@ -139,7 +139,7 @@ defmodule Assent.JWTAdapter.AssentJWT do
   defp split(token) do
     case String.split(token, ".") do
       [header, claims, signature] -> {:ok, %{header: header, claims: claims, signature: signature}}
-      parts                     -> {:error, %Error{message: "JWT must have exactly three parts", reason: :invalid_format, data: parts}}
+      parts                     -> {:error, Error.exception(message: "JWT must have exactly three parts", reason: :invalid_format, data: parts)}
     end
   end
 
@@ -150,7 +150,7 @@ defmodule Assent.JWTAdapter.AssentJWT do
          {:ok, alg}          <- fetch_alg(header) do
       {:ok, alg, header}
     else
-      {:error, error} -> {:error, %Error{message: "Failed to decode header", reason: error, data: header}}
+      {:error, error} -> {:error, Error.exception(message: "Failed to decode header", reason: error, data: header)}
     end
   end
 
@@ -177,14 +177,14 @@ defmodule Assent.JWTAdapter.AssentJWT do
          {:ok, claims}       <- decode_json(claims, json_library) do
       {:ok, claims}
     else
-      {:error, error} -> {:error, %Error{message: "Failed to decode claims", reason: error, data: claims}}
+      {:error, error} -> {:error, Error.exception(message: "Failed to decode claims", reason: error, data: claims)}
     end
   end
 
   defp decode_signature(signature) do
     case decode_base64_url(signature) do
       {:ok, signature} -> {:ok, signature}
-      {:error, error} -> {:error, %Error{message: "Failed to decode signature", reason: error, data: signature}}
+      {:error, error} -> {:error, Error.exception(message: "Failed to decode signature", reason: error, data: signature)}
     end
   end
 
@@ -193,7 +193,7 @@ defmodule Assent.JWTAdapter.AssentJWT do
 
     case verify_message(message, signature, alg, secret_or_public_key) do
       {:ok, verified} -> {:ok, verified}
-      {:error, error} -> {:error, %Error{message: "Failed to verify signature", reason: error, data: {message, signature, alg}}}
+      {:error, error} -> {:error, Error.exception(message: "Failed to verify signature", reason: error, data: {message, signature, alg})}
     end
   end
 

lib/assent/strategies/oauth.ex

@@ -46,7 +46,7 @@ defmodule Assent.Strategy.OAuth do
   @behaviour Assent.Strategy
 
   alias Assent.Strategy, as: Helpers
-  alias Assent.{Config, HTTPAdapter.HTTPResponse, JWTAdapter, MissingParamError, RequestError}
+  alias Assent.{Config, HTTPAdapter.HTTPResponse, JWTAdapter, MissingParamError, InvalidResponseError, RequestError, UnexpectedResponseError}
 
   @doc """
   Generate authorization URL for request phase.
@@ -244,8 +244,8 @@ defmodule Assent.Strategy.OAuth do
   defp process_token_response({:ok, %HTTPResponse{status: 200, body: %{"oauth_token" => _, "oauth_token_secret" => _} = token}}), do: {:ok, token}
   defp process_token_response(any), do: process_response(any)
 
-  defp process_response({:ok, %HTTPResponse{} = response}), do: {:error, RequestError.unexpected(response)}
-  defp process_response({:error, %HTTPResponse{} = response}), do: {:error, RequestError.invalid(response)}
+  defp process_response({:ok, %HTTPResponse{} = response}), do: {:error, UnexpectedResponseError.exception(response: response)}
+  defp process_response({:error, %HTTPResponse{} = response}), do: {:error, InvalidResponseError.exception(response: response)}
   defp process_response({:error, error}), do: {:error, error}
 
   defp build_authorize_url({:ok, token}, config) do
@@ -298,10 +298,10 @@ defmodule Assent.Strategy.OAuth do
   end
 
   defp fetch_oauth_token(%{"oauth_token" => code}), do: {:ok, code}
-  defp fetch_oauth_token(params), do: {:error, MissingParamError.new("oauth_token", params)}
+  defp fetch_oauth_token(params), do: {:error, MissingParamError.exception(expected_key: "oauth_token", params: params)}
 
   defp fetch_oauth_verifier(%{"oauth_verifier" => code}), do: {:ok, code}
-  defp fetch_oauth_verifier(params), do: {:error, MissingParamError.new("oauth_verifier", params)}
+  defp fetch_oauth_verifier(params), do: {:error, MissingParamError.exception(expected_key: "oauth_verifier", params: params)}
 
   defp get_access_token(config, oauth_token, oauth_verifier) do
     with {:ok, site} <- Config.fetch(config, :site) do
@@ -340,6 +340,6 @@ defmodule Assent.Strategy.OAuth do
   end
 
   defp process_user_response({:ok, %HTTPResponse{status: 200, body: user}}), do: {:ok, user}
-  defp process_user_response({:error, %HTTPResponse{status: 401}}), do: {:error, %RequestError{message: "Unauthorized token"}}
+  defp process_user_response({:error, %HTTPResponse{status: 401} = response}), do: {:error, RequestError.exception(message: "Unauthorized token", response: response)}
   defp process_user_response(any), do: process_response(any)
 end

lib/assent/strategies/oauth2.ex

@@ -65,7 +65,7 @@ defmodule Assent.Strategy.OAuth2 do
   @behaviour Assent.Strategy
 
   alias Assent.Strategy, as: Helpers
-  alias Assent.{CallbackCSRFError, CallbackError, Config, HTTPAdapter.HTTPResponse, JWTAdapter, MissingParamError, RequestError}
+  alias Assent.{CallbackCSRFError, CallbackError, Config, HTTPAdapter.HTTPResponse, JWTAdapter, MissingParamError, InvalidResponseError, RequestError, UnexpectedResponseError}
 
   @doc """
   Generate authorization URL for request phase.
@@ -144,21 +144,21 @@ defmodule Assent.Strategy.OAuth2 do
     error     = params["error"]
     error_uri = params["error_uri"]
 
-    {:error, %CallbackError{message: message, error: error, error_uri: error_uri}}
+    {:error, CallbackError.exception(message: message, error: error, error_uri: error_uri)}
   end
   defp check_error_params(_params), do: :ok
 
   defp fetch_code_param(%{"code" => code}), do: {:ok, code}
-  defp fetch_code_param(params), do: {:error, MissingParamError.new("code", params)}
+  defp fetch_code_param(params), do: {:error, MissingParamError.exception(expected_key: "code", params: params)}
 
   defp maybe_check_state(%{state: stored_state}, %{"state" => provided_state}) do
     case Assent.constant_time_compare(stored_state, provided_state) do
       true -> :ok
-      false -> {:error, CallbackCSRFError.new("state")}
+      false -> {:error, CallbackCSRFError.exception(key: "state")}
     end
   end
   defp maybe_check_state(%{state: _state}, params) do
-    {:error, MissingParamError.new("state", params)}
+    {:error, MissingParamError.exception(expected_key: "state", params: params)}
   end
   defp maybe_check_state(_session_params, _params), do: :ok
 
@@ -262,8 +262,8 @@ defmodule Assent.Strategy.OAuth2 do
   defp process_access_token_response({:ok, %HTTPResponse{status: status, body: %{"access_token" => _} = token}}) when status in [200, 201], do: {:ok, token}
   defp process_access_token_response(any), do: process_response(any)
 
-  defp process_response({:ok, %HTTPResponse{} = response}), do: {:error, RequestError.unexpected(response)}
-  defp process_response({:error, %HTTPResponse{} = response}), do: {:error, RequestError.invalid(response)}
+  defp process_response({:ok, %HTTPResponse{} = response}), do: {:error, UnexpectedResponseError.exception(response: response)}
+  defp process_response({:error, %HTTPResponse{} = response}), do: {:error, InvalidResponseError.exception(response: response)}
   defp process_response({:error, error}), do: {:error, error}
 
   defp fetch_user_with_strategy(config, token, strategy) do
@@ -350,6 +350,6 @@ defmodule Assent.Strategy.OAuth2 do
   end
 
   defp process_user_response({:ok, %HTTPResponse{status: 200, body: user}}), do: {:ok, user}
-  defp process_user_response({:error, %HTTPResponse{status: 401}}), do: {:error, %RequestError{message: "Unauthorized token"}}
+  defp process_user_response({:error, %HTTPResponse{status: 401} = response}), do: {:error, RequestError.exception(message: "Unauthorized token", response: response)}
   defp process_user_response(any), do: process_response(any)
 end

lib/assent/strategies/oidc.ex

@@ -67,7 +67,7 @@ defmodule Assent.Strategy.OIDC do
   @behaviour Assent.Strategy
 
   alias Assent.Strategy, as: Helpers
-  alias Assent.{Config, HTTPAdapter.HTTPResponse, RequestError, Strategy.OAuth2}
+  alias Assent.{Config, HTTPAdapter.HTTPResponse, RequestError, UnexpectedResponseError, InvalidResponseError, Strategy.OAuth2}
 
   @doc """
   Generates an authorization URL for request phase.
@@ -119,8 +119,8 @@ defmodule Assent.Strategy.OIDC do
   defp process_openid_configuration_response({:ok, %HTTPResponse{status: 200, body: configuration}}), do: {:ok, configuration}
   defp process_openid_configuration_response(any), do: process_response(any)
 
-  defp process_response({:ok, %HTTPResponse{} = response}), do: {:error, RequestError.unexpected(response)}
-  defp process_response({:error, %HTTPResponse{} = response}), do: {:error, RequestError.invalid(response)}
+  defp process_response({:ok, %HTTPResponse{} = response}), do: {:error, UnexpectedResponseError.exception(response: response)}
+  defp process_response({:error, %HTTPResponse{} = response}), do: {:error, InvalidResponseError.exception(response: response)}
   defp process_response({:error, error}), do: {:error, error}
 
   defp fetch_from_openid_config(config, key) do
@@ -439,7 +439,7 @@ defmodule Assent.Strategy.OIDC do
       _any                                         -> {:ok, body}
     end
   end
-  defp process_userinfo_response({:error, %HTTPResponse{status: 401}}, _openid_config, _config), do: {:error, %RequestError{message: "Unauthorized token"}}
+  defp process_userinfo_response({:error, %HTTPResponse{status: 401} = response}, _openid_config, _config), do: {:error, RequestError.exception(message: "Unauthorized token", response: response)}
   defp process_userinfo_response(any, _openid_config, _config), do: process_response(any)
 
   defp process_jwt(body, openid_config, config) do

lib/assent/strategies/twitter.ex

@@ -33,7 +33,7 @@ defmodule Assent.Strategy.Twitter do
   @impl true
   def callback(config, params) do
     case Map.has_key?(params, "denied") do
-      true  -> {:error, %CallbackError{message: "The user denied the authorization request"}}
+      true  -> {:error, CallbackError.exception(message: "The user denied the authorization request")}
       false -> Base.callback(config, params, __MODULE__)
     end
   end